From: antirez Date: Tue, 14 Dec 2010 16:42:01 +0000 (+0100) Subject: Merge remote branch 'pietern/strrange' X-Git-Url: https://git.saurik.com/redis.git/commitdiff_plain/57997664eaba82ad263c61b9cdbe7dd26ec8d08d?hp=d8f160a848b2d10615918668eddbbb1fe415bd85 Merge remote branch 'pietern/strrange' --- diff --git a/deps/linenoise/linenoise.c b/deps/linenoise/linenoise.c index dd434136..bfed5ea8 100644 --- a/deps/linenoise/linenoise.c +++ b/deps/linenoise/linenoise.c @@ -279,7 +279,9 @@ static int completeLine(int fd, const char *prompt, char *buf, size_t buflen, si } void linenoiseClearScreen(void) { - write(STDIN_FILENO,"\x1b[H\x1b[2J",7); + if (write(STDIN_FILENO,"\x1b[H\x1b[2J",7) <= 0) { + /* nothing to do, just to avoid warning. */ + } } static int linenoisePrompt(int fd, char *buf, size_t buflen, const char *prompt) { diff --git a/src/debug.c b/src/debug.c index 9e97868d..fff8d727 100644 --- a/src/debug.c +++ b/src/debug.c @@ -121,7 +121,7 @@ void computeDatasetDigest(unsigned char *final) { } else if (o->type == REDIS_SET) { setTypeIterator *si = setTypeInitIterator(o); robj *ele; - while((ele = setTypeNext(si)) != NULL) { + while((ele = setTypeNextObject(si)) != NULL) { xorObjectDigest(digest,ele); decrRefCount(ele); } @@ -152,10 +152,10 @@ void computeDatasetDigest(unsigned char *final) { unsigned char eledigest[20]; memset(eledigest,0,20); - obj = hashTypeCurrent(hi,REDIS_HASH_KEY); + obj = hashTypeCurrentObject(hi,REDIS_HASH_KEY); mixObjectDigest(eledigest,obj); decrRefCount(obj); - obj = hashTypeCurrent(hi,REDIS_HASH_VALUE); + obj = hashTypeCurrentObject(hi,REDIS_HASH_VALUE); mixObjectDigest(eledigest,obj); decrRefCount(obj); xorDigest(digest,eledigest,20); diff --git a/src/redis.h b/src/redis.h index 927a4f1d..6e2112a0 100644 --- a/src/redis.h +++ b/src/redis.h @@ -813,8 +813,9 @@ int setTypeRemove(robj *subject, robj *value); int setTypeIsMember(robj *subject, robj *value); setTypeIterator *setTypeInitIterator(robj *subject); void setTypeReleaseIterator(setTypeIterator *si); -robj *setTypeNext(setTypeIterator *si); -int setTypeRandomElement(robj *setobj, robj **objele, long long *llele); +int setTypeNext(setTypeIterator *si, robj **objele, int64_t *llele); +robj *setTypeNextObject(setTypeIterator *si); +int setTypeRandomElement(robj *setobj, robj **objele, int64_t *llele); unsigned long setTypeSize(robj *subject); void setTypeConvert(robj *subject, int enc); @@ -822,7 +823,8 @@ void setTypeConvert(robj *subject, int enc); void convertToRealHash(robj *o); void hashTypeTryConversion(robj *subject, robj **argv, int start, int end); void hashTypeTryObjectEncoding(robj *subject, robj **o1, robj **o2); -robj *hashTypeGet(robj *o, robj *key); +int hashTypeGet(robj *o, robj *key, robj **objval, unsigned char **v, unsigned int *vlen); +robj *hashTypeGetObject(robj *o, robj *key); int hashTypeExists(robj *o, robj *key); int hashTypeSet(robj *o, robj *key, robj *value); int hashTypeDelete(robj *o, robj *key); @@ -830,7 +832,8 @@ unsigned long hashTypeLength(robj *o); hashTypeIterator *hashTypeInitIterator(robj *subject); void hashTypeReleaseIterator(hashTypeIterator *hi); int hashTypeNext(hashTypeIterator *hi); -robj *hashTypeCurrent(hashTypeIterator *hi, int what); +int hashTypeCurrent(hashTypeIterator *hi, int what, robj **objval, unsigned char **v, unsigned int *vlen); +robj *hashTypeCurrentObject(hashTypeIterator *hi, int what); robj *hashTypeLookupWriteOrCreate(redisClient *c, robj *key); /* Pub / Sub */ diff --git a/src/sort.c b/src/sort.c index 79f79010..a44a6d63 100644 --- a/src/sort.c +++ b/src/sort.c @@ -76,7 +76,7 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) { /* Retrieve value from hash by the field name. This operation * already increases the refcount of the returned object. */ initStaticStringObject(fieldobj,((char*)&fieldname)+(sizeof(struct sdshdr))); - o = hashTypeGet(o, &fieldobj); + o = hashTypeGetObject(o, &fieldobj); } else { if (o->type != REDIS_STRING) return NULL; @@ -222,7 +222,7 @@ void sortCommand(redisClient *c) { } else if (sortval->type == REDIS_SET) { setTypeIterator *si = setTypeInitIterator(sortval); robj *ele; - while((ele = setTypeNext(si)) != NULL) { + while((ele = setTypeNextObject(si)) != NULL) { vector[j].obj = ele; vector[j].u.score = 0; vector[j].u.cmpobj = NULL; diff --git a/src/t_hash.c b/src/t_hash.c index 071b7754..838f29dd 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -31,27 +31,56 @@ void hashTypeTryObjectEncoding(robj *subject, robj **o1, robj **o2) { } } -/* Get the value from a hash identified by key. Returns either a string - * object or NULL if the value cannot be found. The refcount of the object - * is always increased by 1 when the value was found. */ -robj *hashTypeGet(robj *o, robj *key) { - robj *value = NULL; +/* Get the value from a hash identified by key. + * + * If the string is found either REDIS_ENCODING_HT or REDIS_ENCODING_ZIPMAP + * is returned, and either **objval or **v and *vlen are set accordingly, + * so that objects in hash tables are returend as objects and pointers + * inside a zipmap are returned as such. + * + * If the object was not found -1 is returned. + * + * This function is copy on write friendly as there is no incr/decr + * of refcount needed if objects are accessed just for reading operations. */ +int hashTypeGet(robj *o, robj *key, robj **objval, unsigned char **v, + unsigned int *vlen) +{ if (o->encoding == REDIS_ENCODING_ZIPMAP) { - unsigned char *v; - unsigned int vlen; + int found; + key = getDecodedObject(key); - if (zipmapGet(o->ptr,key->ptr,sdslen(key->ptr),&v,&vlen)) { - value = createStringObject((char*)v,vlen); - } + found = zipmapGet(o->ptr,key->ptr,sdslen(key->ptr),v,vlen); decrRefCount(key); + if (!found) return -1; } else { dictEntry *de = dictFind(o->ptr,key); - if (de != NULL) { - value = dictGetEntryVal(de); - incrRefCount(value); - } + if (de == NULL) return -1; + *objval = dictGetEntryVal(de); + } + return o->encoding; +} + +/* Higher level function of hashTypeGet() that always returns a Redis + * object (either new or with refcount incremented), so that the caller + * can retain a reference or call decrRefCount after the usage. + * + * The lower level function can prevent copy on write so it is + * the preferred way of doing read operations. */ +robj *hashTypeGetObject(robj *o, robj *key) { + robj *objval; + unsigned char *v; + unsigned int vlen; + + int encoding = hashTypeGet(o,key,&objval,&v,&vlen); + switch(encoding) { + case REDIS_ENCODING_HT: + incrRefCount(objval); + return objval; + case REDIS_ENCODING_ZIPMAP: + objval = createStringObject((char*)v,vlen); + return objval; + default: return NULL; } - return value; } /* Test if the key exists in the given hash. Returns 1 if the key @@ -156,24 +185,50 @@ int hashTypeNext(hashTypeIterator *hi) { } /* Get key or value object at current iteration position. - * This increases the refcount of the field object by 1. */ -robj *hashTypeCurrent(hashTypeIterator *hi, int what) { - robj *o; + * The returned item differs with the hash object encoding: + * - When encoding is REDIS_ENCODING_HT, the objval pointer is populated + * with the original object. + * - When encoding is REDIS_ENCODING_ZIPMAP, a pointer to the string and + * its length is retunred populating the v and vlen pointers. + * This function is copy on write friendly as accessing objects in read only + * does not require writing to any memory page. + * + * The function returns the encoding of the object, so that the caller + * can underestand if the key or value was returned as object or C string. */ +int hashTypeCurrent(hashTypeIterator *hi, int what, robj **objval, unsigned char **v, unsigned int *vlen) { if (hi->encoding == REDIS_ENCODING_ZIPMAP) { if (what & REDIS_HASH_KEY) { - o = createStringObject((char*)hi->zk,hi->zklen); + *v = hi->zk; + *vlen = hi->zklen; } else { - o = createStringObject((char*)hi->zv,hi->zvlen); + *v = hi->zv; + *vlen = hi->zvlen; } } else { - if (what & REDIS_HASH_KEY) { - o = dictGetEntryKey(hi->de); - } else { - o = dictGetEntryVal(hi->de); - } - incrRefCount(o); + if (what & REDIS_HASH_KEY) + *objval = dictGetEntryKey(hi->de); + else + *objval = dictGetEntryVal(hi->de); + } + return hi->encoding; +} + +/* A non copy-on-write friendly but higher level version of hashTypeCurrent() + * that always returns an object with refcount incremented by one (or a new + * object), so it's up to the caller to decrRefCount() the object if no + * reference is retained. */ +robj *hashTypeCurrentObject(hashTypeIterator *hi, int what) { + robj *obj; + unsigned char *v; + unsigned int vlen; + int encoding = hashTypeCurrent(hi,what,&obj,&v,&vlen); + + if (encoding == REDIS_ENCODING_HT) { + incrRefCount(obj); + return obj; + } else { + return createStringObject((char*)v,vlen); } - return o; } robj *hashTypeLookupWriteOrCreate(redisClient *c, robj *key) { @@ -270,7 +325,7 @@ void hincrbyCommand(redisClient *c) { if (getLongLongFromObjectOrReply(c,c->argv[3],&incr,NULL) != REDIS_OK) return; if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return; - if ((current = hashTypeGet(o,c->argv[2])) != NULL) { + if ((current = hashTypeGetObject(o,c->argv[2])) != NULL) { if (getLongLongFromObjectOrReply(c,current,&value, "hash value is not an integer") != REDIS_OK) { decrRefCount(current); @@ -293,20 +348,29 @@ void hincrbyCommand(redisClient *c) { void hgetCommand(redisClient *c) { robj *o, *value; + unsigned char *v; + unsigned int vlen; + int encoding; + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL || checkType(c,o,REDIS_HASH)) return; - if ((value = hashTypeGet(o,c->argv[2])) != NULL) { - addReplyBulk(c,value); - decrRefCount(value); + if ((encoding = hashTypeGet(o,c->argv[2],&value,&v,&vlen)) != -1) { + if (encoding == REDIS_ENCODING_HT) + addReplyBulk(c,value); + else + addReplyBulkCBuffer(c,v,vlen); } else { addReply(c,shared.nullbulk); } } void hmgetCommand(redisClient *c) { - int i; + int i, encoding; robj *o, *value; + unsigned char *v; + unsigned int vlen; + o = lookupKeyRead(c->db,c->argv[1]); if (o != NULL && o->type != REDIS_HASH) { addReply(c,shared.wrongtypeerr); @@ -318,9 +382,12 @@ void hmgetCommand(redisClient *c) { * an empty hash. The reply should then be a series of NULLs. */ addReplyMultiBulkLen(c,c->argc-2); for (i = 2; i < c->argc; i++) { - if (o != NULL && (value = hashTypeGet(o,c->argv[i])) != NULL) { - addReplyBulk(c,value); - decrRefCount(value); + if (o != NULL && + (encoding = hashTypeGet(o,c->argv[i],&value,&v,&vlen)) != -1) { + if (encoding == REDIS_ENCODING_HT) + addReplyBulk(c,value); + else + addReplyBulkCBuffer(c,v,vlen); } else { addReply(c,shared.nullbulk); } @@ -351,7 +418,7 @@ void hlenCommand(redisClient *c) { } void genericHgetallCommand(redisClient *c, int flags) { - robj *o, *obj; + robj *o; unsigned long count = 0; hashTypeIterator *hi; void *replylen = NULL; @@ -362,16 +429,25 @@ void genericHgetallCommand(redisClient *c, int flags) { replylen = addDeferredMultiBulkLength(c); hi = hashTypeInitIterator(o); while (hashTypeNext(hi) != REDIS_ERR) { + robj *obj; + unsigned char *v; + unsigned int vlen; + int encoding; + if (flags & REDIS_HASH_KEY) { - obj = hashTypeCurrent(hi,REDIS_HASH_KEY); - addReplyBulk(c,obj); - decrRefCount(obj); + encoding = hashTypeCurrent(hi,REDIS_HASH_KEY,&obj,&v,&vlen); + if (encoding == REDIS_ENCODING_HT) + addReplyBulk(c,obj); + else + addReplyBulkCBuffer(c,v,vlen); count++; } if (flags & REDIS_HASH_VALUE) { - obj = hashTypeCurrent(hi,REDIS_HASH_VALUE); - addReplyBulk(c,obj); - decrRefCount(obj); + encoding = hashTypeCurrent(hi,REDIS_HASH_VALUE,&obj,&v,&vlen); + if (encoding == REDIS_ENCODING_HT) + addReplyBulk(c,obj); + else + addReplyBulkCBuffer(c,v,vlen); count++; } } diff --git a/src/t_set.c b/src/t_set.c index e15952c0..0b4128ad 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -101,39 +101,68 @@ void setTypeReleaseIterator(setTypeIterator *si) { } /* Move to the next entry in the set. Returns the object at the current - * position, or NULL when the end is reached. This object will have its - * refcount incremented, so the caller needs to take care of this. */ -robj *setTypeNext(setTypeIterator *si) { - robj *ret = NULL; + * position. + * + * Since set elements can be internally be stored as redis objects or + * simple arrays of integers, setTypeNext returns the encoding of the + * set object you are iterating, and will populate the appropriate pointer + * (eobj) or (llobj) accordingly. + * + * When there are no longer elements -1 is returned. + * Returned objects ref count is not incremented, so this function is + * copy on write friendly. */ +int setTypeNext(setTypeIterator *si, robj **objele, int64_t *llele) { if (si->encoding == REDIS_ENCODING_HT) { dictEntry *de = dictNext(si->di); - if (de != NULL) { - ret = dictGetEntryKey(de); - incrRefCount(ret); - } + if (de == NULL) return -1; + *objele = dictGetEntryKey(de); } else if (si->encoding == REDIS_ENCODING_INTSET) { - int64_t llval; - if (intsetGet(si->subject->ptr,si->ii++,&llval)) - ret = createStringObjectFromLongLong(llval); + if (!intsetGet(si->subject->ptr,si->ii++,llele)) + return -1; } - return ret; + return si->encoding; } +/* The not copy on write friendly version but easy to use version + * of setTypeNext() is setTypeNextObject(), returning new objects + * or incrementing the ref count of returned objects. So if you don't + * retain a pointer to this object you should call decrRefCount() against it. + * + * This function is the way to go for write operations where COW is not + * an issue as the result will be anyway of incrementing the ref count. */ +robj *setTypeNextObject(setTypeIterator *si) { + int64_t intele; + robj *objele; + int encoding; + + encoding = setTypeNext(si,&objele,&intele); + switch(encoding) { + case -1: return NULL; + case REDIS_ENCODING_INTSET: + return createStringObjectFromLongLong(intele); + case REDIS_ENCODING_HT: + incrRefCount(objele); + return objele; + default: + redisPanic("Unsupported encoding"); + } + return NULL; /* just to suppress warnings */ +} /* Return random element from a non empty set. - * The returned element can be a long long value if the set is encoded + * The returned element can be a int64_t value if the set is encoded * as an "intset" blob of integers, or a redis object if the set * is a regular set. * * The caller provides both pointers to be populated with the right * object. The return value of the function is the object->encoding * field of the object and is used by the caller to check if the - * long long pointer or the redis object pointere was populated. + * int64_t pointer or the redis object pointere was populated. * * When an object is returned (the set was a real set) the ref count * of the object is not incremented so this function can be considered - * copy-on-write friendly. */ -int setTypeRandomElement(robj *setobj, robj **objele, long long *llele) { + * copy on write friendly. */ +int setTypeRandomElement(robj *setobj, robj **objele, int64_t *llele) { if (setobj->encoding == REDIS_ENCODING_HT) { dictEntry *de = dictGetRandomKey(setobj->ptr); *objele = dictGetEntryKey(de); @@ -158,25 +187,30 @@ unsigned long setTypeSize(robj *subject) { /* Convert the set to specified encoding. The resulting dict (when converting * to a hashtable) is presized to hold the number of elements in the original * set. */ -void setTypeConvert(robj *subject, int enc) { +void setTypeConvert(robj *setobj, int enc) { setTypeIterator *si; - robj *element; - redisAssert(subject->type == REDIS_SET); + redisAssert(setobj->type == REDIS_SET && + setobj->encoding == REDIS_ENCODING_INTSET); if (enc == REDIS_ENCODING_HT) { + int64_t intele; dict *d = dictCreate(&setDictType,NULL); + robj *element; + /* Presize the dict to avoid rehashing */ - dictExpand(d,intsetLen(subject->ptr)); + dictExpand(d,intsetLen(setobj->ptr)); - /* setTypeGet returns a robj with incremented refcount */ - si = setTypeInitIterator(subject); - while ((element = setTypeNext(si)) != NULL) + /* To add the elements we extract integers and create redis objects */ + si = setTypeInitIterator(setobj); + while (setTypeNext(si,NULL,&intele) != -1) { + element = createStringObjectFromLongLong(intele); redisAssert(dictAdd(d,element,NULL) == DICT_OK); + } setTypeReleaseIterator(si); - subject->encoding = REDIS_ENCODING_HT; - zfree(subject->ptr); - subject->ptr = d; + setobj->encoding = REDIS_ENCODING_HT; + zfree(setobj->ptr); + setobj->ptr = d; } else { redisPanic("Unsupported set conversion"); } @@ -292,7 +326,7 @@ void scardCommand(redisClient *c) { void spopCommand(redisClient *c) { robj *set, *ele; - long long llele; + int64_t llele; int encoding; if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.nullbulk)) == NULL || @@ -313,7 +347,7 @@ void spopCommand(redisClient *c) { void srandmemberCommand(redisClient *c) { robj *set, *ele; - long long llele; + int64_t llele; int encoding; if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL || @@ -334,9 +368,11 @@ int qsortCompareSetsByCardinality(const void *s1, const void *s2) { void sinterGenericCommand(redisClient *c, robj **setkeys, unsigned long setnum, robj *dstkey) { robj **sets = zmalloc(sizeof(robj*)*setnum); setTypeIterator *si; - robj *ele, *dstset = NULL; + robj *eleobj, *dstset = NULL; + int64_t intobj; void *replylen = NULL; unsigned long j, cardinality = 0; + int encoding; for (j = 0; j < setnum; j++) { robj *setobj = dstkey ? @@ -382,20 +418,60 @@ void sinterGenericCommand(redisClient *c, robj **setkeys, unsigned long setnum, * the element against all the other sets, if at least one set does * not include the element it is discarded */ si = setTypeInitIterator(sets[0]); - while((ele = setTypeNext(si)) != NULL) { - for (j = 1; j < setnum; j++) - if (!setTypeIsMember(sets[j],ele)) break; + while((encoding = setTypeNext(si,&eleobj,&intobj)) != -1) { + for (j = 1; j < setnum; j++) { + if (encoding == REDIS_ENCODING_INTSET) { + /* intset with intset is simple... and fast */ + if (sets[j]->encoding == REDIS_ENCODING_INTSET && + !intsetFind((intset*)sets[j]->ptr,intobj)) + { + break; + /* in order to compare an integer with an object we + * have to use the generic function, creating an object + * for this */ + } else if (sets[j]->encoding == REDIS_ENCODING_HT) { + eleobj = createStringObjectFromLongLong(intobj); + if (!setTypeIsMember(sets[j],eleobj)) { + decrRefCount(eleobj); + break; + } + decrRefCount(eleobj); + } + } else if (encoding == REDIS_ENCODING_HT) { + /* Optimization... if the source object is integer + * encoded AND the target set is an intset, we can get + * a much faster path. */ + if (eleobj->encoding == REDIS_ENCODING_INT && + sets[j]->encoding == REDIS_ENCODING_INTSET && + !intsetFind((intset*)sets[j]->ptr,(long)eleobj->ptr)) + { + break; + /* else... object to object check is easy as we use the + * type agnostic API here. */ + } else if (!setTypeIsMember(sets[j],eleobj)) { + break; + } + } + } /* Only take action when all sets contain the member */ if (j == setnum) { if (!dstkey) { - addReplyBulk(c,ele); + if (encoding == REDIS_ENCODING_HT) + addReplyBulk(c,eleobj); + else + addReplyBulkLongLong(c,intobj); cardinality++; } else { - setTypeAdd(dstset,ele); + if (encoding == REDIS_ENCODING_INTSET) { + eleobj = createStringObjectFromLongLong(intobj); + setTypeAdd(dstset,eleobj); + decrRefCount(eleobj); + } else { + setTypeAdd(dstset,eleobj); + } } } - decrRefCount(ele); } setTypeReleaseIterator(si); @@ -463,7 +539,7 @@ void sunionDiffGenericCommand(redisClient *c, robj **setkeys, int setnum, robj * if (!sets[j]) continue; /* non existing keys are like empty sets */ si = setTypeInitIterator(sets[j]); - while((ele = setTypeNext(si)) != NULL) { + while((ele = setTypeNextObject(si)) != NULL) { if (op == REDIS_OP_UNION || j == 0) { if (setTypeAdd(dstset,ele)) { cardinality++; @@ -485,7 +561,7 @@ void sunionDiffGenericCommand(redisClient *c, robj **setkeys, int setnum, robj * if (!dstkey) { addReplyMultiBulkLen(c,cardinality); si = setTypeInitIterator(dstset); - while((ele = setTypeNext(si)) != NULL) { + while((ele = setTypeNextObject(si)) != NULL) { addReplyBulk(c,ele); decrRefCount(ele); } diff --git a/src/valgrind.sup b/src/valgrind.sup new file mode 100644 index 00000000..7ba75754 --- /dev/null +++ b/src/valgrind.sup @@ -0,0 +1,5 @@ +{ + + Memcheck:Cond + fun:lzf_compress +} diff --git a/tests/support/server.tcl b/tests/support/server.tcl index 1507088e..4f48d22d 100644 --- a/tests/support/server.tcl +++ b/tests/support/server.tcl @@ -83,9 +83,13 @@ proc ping_server {host port} { } close $fd } e]} { - puts -nonewline "." + if {$::verbose} { + puts -nonewline "." + } } else { - puts -nonewline "ok" + if {$::verbose} { + puts -nonewline "ok" + } } return $retval } @@ -171,7 +175,7 @@ proc start_server {options {code undefined}} { set stderr [format "%s/%s" [dict get $config "dir"] "stderr"] if {$::valgrind} { - exec valgrind src/redis-server $config_file > $stdout 2> $stderr & + exec valgrind --suppressions=src/valgrind.sup src/redis-server $config_file > $stdout 2> $stderr & } else { exec src/redis-server $config_file > $stdout 2> $stderr & } @@ -181,7 +185,10 @@ proc start_server {options {code undefined}} { set retrynum 20 set serverisup 0 - puts -nonewline "=== ($tags) Starting server ${::host}:${::port} " + if {$::verbose} { + puts -nonewline "=== ($tags) Starting server ${::host}:${::port} " + } + after 10 if {$code ne "undefined"} { while {[incr retrynum -1]} { @@ -196,7 +203,10 @@ proc start_server {options {code undefined}} { } else { set serverisup 1 } - puts {} + + if {$::verbose} { + puts "" + } if {!$serverisup} { error_and_quit $config_file [exec cat $stderr] @@ -246,41 +256,34 @@ proc start_server {options {code undefined}} { reconnect # execute provided block - set curnum $::testnum - if {![catch { uplevel 1 $code } err]} { - # zero exit status is good - unset err + set num_tests $::num_tests + if {[catch { uplevel 1 $code } error]} { + set backtrace $::errorInfo + + # Kill the server without checking for leaks + dict set srv "skipleaks" 1 + kill_server $srv + + # Print warnings from log + puts [format "\nLogged warnings (pid %d):" [dict get $srv "pid"]] + set warnings [warnings_from_file [dict get $srv "stdout"]] + if {[string length $warnings] > 0} { + puts "$warnings" + } else { + puts "(none)" + } + puts "" + + error $error $backtrace } - if {$curnum == $::testnum} { - # don't check for leaks when no tests were executed + # Don't do the leak check when no tests were run + if {$num_tests == $::num_tests} { dict set srv "skipleaks" 1 } # pop the server object set ::servers [lrange $::servers 0 end-1] - - # allow an exception to bubble up the call chain but still kill this - # server, because we want to reuse the ports when the tests are re-run - if {[info exists err]} { - if {$err eq "exception"} { - puts [format "Logged warnings (pid %d):" [dict get $srv "pid"]] - set warnings [warnings_from_file [dict get $srv "stdout"]] - if {[string length $warnings] > 0} { - puts "$warnings" - } else { - puts "(none)" - } - # kill this server without checking for leaks - dict set srv "skipleaks" 1 - kill_server $srv - error "exception" - } elseif {[string length $err] > 0} { - puts "Error executing the suite, aborting..." - puts $err - exit 1 - } - } set ::tags [lrange $::tags 0 end-[llength $tags]] kill_server $srv diff --git a/tests/support/test.tcl b/tests/support/test.tcl index e801e1f2..153ba1e3 100644 --- a/tests/support/test.tcl +++ b/tests/support/test.tcl @@ -1,25 +1,23 @@ -set ::passed 0 -set ::failed 0 -set ::testnum 0 +set ::num_tests 0 +set ::num_passed 0 +set ::num_failed 0 +set ::tests_failed {} proc assert {condition} { if {![uplevel 1 expr $condition]} { - puts "!! ERROR\nExpected '$value' to evaluate to true" - error "assertion" + error "assertion:Expected '$value' to be true" } } proc assert_match {pattern value} { if {![string match $pattern $value]} { - puts "!! ERROR\nExpected '$value' to match '$pattern'" - error "assertion" + error "assertion:Expected '$value' to match '$pattern'" } } proc assert_equal {expected value} { if {$expected ne $value} { - puts "!! ERROR\nExpected '$value' to be equal to '$expected'" - error "assertion" + error "assertion:Expected '$value' to be equal to '$expected'" } } @@ -27,8 +25,7 @@ proc assert_error {pattern code} { if {[catch {uplevel 1 $code} error]} { assert_match $pattern $error } else { - puts "!! ERROR\nExpected an error but nothing was catched" - error "assertion" + error "assertion:Expected an error but nothing was catched" } } @@ -47,7 +44,7 @@ proc assert_type {type key} { assert_equal $type [r type $key] } -proc test {name code {okpattern notspecified}} { +proc test {name code {okpattern undefined}} { # abort if tagged with a tag to deny foreach tag $::denytags { if {[lsearch $::tags $tag] >= 0} { @@ -69,30 +66,62 @@ proc test {name code {okpattern notspecified}} { } } - incr ::testnum - puts -nonewline [format "#%03d %-68s " $::testnum $name] - flush stdout + incr ::num_tests + set details {} + lappend details $::curfile + lappend details $::tags + lappend details $name + + if {$::verbose} { + puts -nonewline [format "#%03d %-68s " $::num_tests $name] + flush stdout + } + if {[catch {set retval [uplevel 1 $code]} error]} { - if {$error eq "assertion"} { - incr ::failed + if {[string match "assertion:*" $error]} { + set msg [string range $error 10 end] + lappend details $msg + lappend ::tests_failed $details + + incr ::num_failed + if {$::verbose} { + puts "FAILED" + puts "$msg\n" + } else { + puts -nonewline "F" + } } else { - puts "EXCEPTION" - puts "\nCaught error: $error" - error "exception" + # Re-raise, let handler up the stack take care of this. + error $error $::errorInfo } } else { - if {$okpattern eq "notspecified" || $okpattern eq $retval || [string match $okpattern $retval]} { - puts "PASSED" - incr ::passed + if {$okpattern eq "undefined" || $okpattern eq $retval || [string match $okpattern $retval]} { + incr ::num_passed + if {$::verbose} { + puts "PASSED" + } else { + puts -nonewline "." + } } else { - puts "!! ERROR expected\n'$okpattern'\nbut got\n'$retval'" - incr ::failed + set msg "Expected '$okpattern' to equal or match '$retval'" + lappend details $msg + lappend ::tests_failed $details + + incr ::num_failed + if {$::verbose} { + puts "FAILED" + puts "$msg\n" + } else { + puts -nonewline "F" + } } } + flush stdout + if {$::traceleaks} { set output [exec leaks redis-server] if {![string match {*0 leaks*} $output]} { - puts "--------- Test $::testnum LEAKED! --------" + puts "--- Test \"$name\" leaked! ---" puts $output exit 1 } diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 93cb750f..a39a2134 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -52,8 +52,10 @@ proc status {r property} { proc waitForBgsave r { while 1 { if {[status r bgsave_in_progress] eq 1} { - puts -nonewline "\nWaiting for background save to finish... " - flush stdout + if {$::verbose} { + puts -nonewline "\nWaiting for background save to finish... " + flush stdout + } after 1000 } else { break @@ -64,8 +66,10 @@ proc waitForBgsave r { proc waitForBgrewriteaof r { while 1 { if {[status r bgrewriteaof_in_progress] eq 1} { - puts -nonewline "\nWaiting for background AOF rewrite to finish... " - flush stdout + if {$::verbose} { + puts -nonewline "\nWaiting for background AOF rewrite to finish... " + flush stdout + } after 1000 } else { break diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index 4c207f64..2b7a8957 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -13,13 +13,17 @@ set ::host 127.0.0.1 set ::port 16379 set ::traceleaks 0 set ::valgrind 0 +set ::verbose 0 set ::denytags {} set ::allowtags {} set ::external 0; # If "1" this means, we are running against external instance set ::file ""; # If set, runs only the tests in this comma separated list +set ::curfile ""; # Hold the filename of the current suite proc execute_tests name { - source "tests/$name.tcl" + set path "tests/$name.tcl" + set ::curfile $path + source $path } # Setup a list to hold a stack of server configs. When calls to start_server @@ -147,9 +151,27 @@ proc main {} { } cleanup - puts "\n[expr $::passed+$::failed] tests, $::passed passed, $::failed failed" - if {$::failed > 0} { - puts "\n*** WARNING!!! $::failed FAILED TESTS ***\n" + puts "\n[expr $::num_tests] tests, $::num_passed passed, $::num_failed failed\n" + if {$::num_failed > 0} { + set curheader "" + puts "Failures:" + foreach {test} $::tests_failed { + set header [lindex $test 0] + append header " (" + append header [join [lindex $test 1] ","] + append header ")" + + if {$curheader ne $header} { + set curheader $header + puts "\n$curheader:" + } + + set name [lindex $test 2] + set msg [lindex $test 3] + puts "- $name: $msg" + } + + puts "" exit 1 } } @@ -167,6 +189,8 @@ for {set j 0} {$j < [llength $argv]} {incr j} { } } incr j + } elseif {$opt eq {--valgrind}} { + set ::valgrind 1 } elseif {$opt eq {--file}} { set ::file $arg incr j @@ -177,6 +201,8 @@ for {set j 0} {$j < [llength $argv]} {incr j} { } elseif {$opt eq {--port}} { set ::port $arg incr j + } elseif {$opt eq {--verbose}} { + set ::verbose 1 } else { puts "Wrong argument: $opt" exit 1 @@ -187,7 +213,7 @@ if {[catch { main } err]} { if {[string length $err] > 0} { # only display error when not generated by the test suite if {$err ne "exception"} { - puts $err + puts $::errorInfo } exit 1 } diff --git a/tests/unit/sort.tcl b/tests/unit/sort.tcl index 41558522..3a4c855f 100644 --- a/tests/unit/sort.tcl +++ b/tests/unit/sort.tcl @@ -144,8 +144,10 @@ start_server { set sorted [r sort tosort BY weight_* LIMIT 0 10] } set elapsed [expr [clock clicks -milliseconds]-$start] - puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " - flush stdout + if {$::verbose} { + puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " + flush stdout + } } test "SORT speed, $num element list BY hash field, 100 times" { @@ -154,8 +156,10 @@ start_server { set sorted [r sort tosort BY wobj_*->weight LIMIT 0 10] } set elapsed [expr [clock clicks -milliseconds]-$start] - puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " - flush stdout + if {$::verbose} { + puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " + flush stdout + } } test "SORT speed, $num element list directly, 100 times" { @@ -164,8 +168,10 @@ start_server { set sorted [r sort tosort LIMIT 0 10] } set elapsed [expr [clock clicks -milliseconds]-$start] - puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " - flush stdout + if {$::verbose} { + puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " + flush stdout + } } test "SORT speed, $num element list BY , 100 times" { @@ -174,8 +180,10 @@ start_server { set sorted [r sort tosort BY nokey LIMIT 0 10] } set elapsed [expr [clock clicks -milliseconds]-$start] - puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " - flush stdout + if {$::verbose} { + puts -nonewline "\n Average time to sort: [expr double($elapsed)/100] milliseconds " + flush stdout + } } } }