gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix valgrind issues with long double module test (#9709)

Browse Source 
The module test in reply.tcl was introduced by #8521 but didn't run until recently (see #9639)
and then it started failing with valgrind.
This is because valgrind uses 64 bit long double (unlike most other platforms that have at least 80 bits)
But besides valgrind, the tests where also incompatible with ARM32, which also uses 64 bit long doubles.

We now use appropriate value to avoid issues with either valgrind or ARM32

In all the double tests, i use 3.141, which is safe since since addReplyDouble uses
`%.17Lg` which is able to represent this value without adding any digits due to precision loss. 

In the long double, since we use `%.17Lf` in ld2string, it preserves 17 significant
digits, rather than 17 digit after the decimal point (like in `%.17Lg`).
So to make these similar, i use value lower than 1 (no digits left of
the period)

Lastly, we have the same issue with TCL (no long doubles) so we read
raw protocol in that test.

Note that the only error before this fix (in both valgrind and ARM32 is this:
```
*** [err]: RM_ReplyWithLongDouble: a float reply in tests/unit/moduleapi/reply.tcl
Expected '3.141' to be equal to '3.14100000000000001' (context: type eval line 2 cmd {assert_equal 3.141 [r rw.longdouble 3.141]} proc ::test)
```
so the changes to debug.c and scripting.tcl aren't really needed, but i consider them a cleanup
(i.e. scripting.c validated a different constant than the one that's sent to it from debug.c).

Another unrelated change is to add the RESP version to the repeated tests in reply.tcl
This commit is contained in:
Oran Agra 2021-11-01 13:41:35 +02:00 committed by GitHub
parent 155c291006
commit f1f3cceb50
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 29 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/debug.c
View File

@ -723,7 +723,7 @@ NULL
} else if (!strcasecmp(name,"integer")) {
addReplyLongLong(c,12345);
} else if (!strcasecmp(name,"double")) {
addReplyDouble(c,3.14159265359);
addReplyDouble(c,3.141);
} else if (!strcasecmp(name,"bignum")) {
addReplyBigNum(c,"1234567999999999999999999999999999999",37);
} else if (!strcasecmp(name,"null")) {

2
tests/modules/basics.c
View File

@ -285,7 +285,7 @@ int TestCallResp3Double(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
/* we compare strings, since comparing doubles directly can fail in various architectures, e.g. 32bit */
char got[30], expected[30];
sprintf(got, "%.17g", d);
sprintf(expected, "%.17g", 3.14159265359);
sprintf(expected, "%.17g", 3.141);
if (strcmp(got, expected) != 0) goto fail;
RedisModule_ReplyWithSimpleString(ctx,"OK");
return REDISMODULE_OK;

38
tests/unit/moduleapi/reply.tcl
View File

@ -7,7 +7,7 @@ start_server {tags {"modules"}} {
for {set proto 2} {$proto <= 3} {incr proto} {
r hello $proto
test {RM_ReplyWithString: an string reply} {
test "RESP$proto: RM_ReplyWithString: an string reply" {
# RedisString
set string [r rw.string "Redis"]
assert_equal "Redis" $string
@ -16,31 +16,41 @@ start_server {tags {"modules"}} {
assert_equal "A simple string" $string
}
test {RM_ReplyWithBigNumber: an string reply} {
test "RESP$proto: RM_ReplyWithBigNumber: an string reply" {
assert_equal "123456778901234567890" [r rw.bignumber "123456778901234567890"]
}
test {RM_ReplyWithInt: an integer reply} {
test "RESP$proto: RM_ReplyWithInt: an integer reply" {
assert_equal 42 [r rw.int 42]
}
test {RM_ReplyWithDouble: a float reply} {
test "RESP$proto: RM_ReplyWithDouble: a float reply" {
assert_equal 3.141 [r rw.double 3.141]
}
test {RM_ReplyWithLongDouble: a float reply} {
assert_equal 3.141 [r rw.longdouble 3.141]
set ld 0.00000000000000001
test "RESP$proto: RM_ReplyWithLongDouble: a float reply" {
if {$proto == 2} {
# here the response gets to TCL as a string
assert_equal $ld [r rw.longdouble $ld]
} else {
# TCL doesn't support long double and the test infra converts it to a
# normal double which causes precision loss. so we use readraw instead
r readraw 1
assert_equal ",$ld" [r rw.longdouble $ld]
r readraw 0
}
}
test {RM_ReplyWithVerbatimString: a string reply} {
test "RESP$proto: RM_ReplyWithVerbatimString: a string reply" {
assert_equal "bla\nbla\nbla" [r rw.verbatim "bla\nbla\nbla"]
}
test {RM_ReplyWithArray: an array reply} {
test "RESP$proto: RM_ReplyWithArray: an array reply" {
assert_equal {0 1 2 3 4} [r rw.array 5]
}
test {RM_ReplyWithMap: an map reply} {
test "RESP$proto: RM_ReplyWithMap: an map reply" {
set res [r rw.map 3]
if {$proto == 2} {
assert_equal {0 0 1 1.5 2 3} $res
@ -49,11 +59,11 @@ start_server {tags {"modules"}} {
}
}
test {RM_ReplyWithSet: an set reply} {
test "RESP$proto: RM_ReplyWithSet: an set reply" {
assert_equal {0 1 2} [r rw.set 3]
}
test {RM_ReplyWithAttribute: an set reply} {
test "RESP$proto: RM_ReplyWithAttribute: an set reply" {
if {$proto == 2} {
catch {[r rw.attribute 3]} e
assert_match "Attributes aren't supported by RESP 2" $e
@ -71,15 +81,15 @@ start_server {tags {"modules"}} {
}
}
test {RM_ReplyWithBool: a boolean reply} {
test "RESP$proto: RM_ReplyWithBool: a boolean reply" {
assert_equal {0 1} [r rw.bool]
}
test {RM_ReplyWithNull: a NULL reply} {
test "RESP$proto: RM_ReplyWithNull: a NULL reply" {
assert_equal {} [r rw.null]
}
test {RM_ReplyWithError: an error reply} {
test "RESP$proto: RM_ReplyWithError: an error reply" {
catch {r rw.error} e
assert_match "An error" $e
}

6
tests/unit/scripting.tcl
View File

@ -1028,10 +1028,10 @@ start_server {tags {"scripting resp3 needs:debug"}} {
set ret [r eval "redis.setresp($i);return redis.call('debug', 'protocol', 'double')" 0]
if {$client_proto == 2 || $i == 2} {
# if either Lua or the clien is RESP2 the reply will be RESP2
assert_equal $ret {$18}
assert_equal [r read] {3.1415926535900001}
assert_equal $ret {$5}
assert_equal [r read] {3.141}
} else {
assert_equal $ret {,3.1415926535900001}
assert_equal $ret {,3.141}
}
}