Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Make redis test suite tcl version agnostic #877

Closed
wants to merge 2 commits into from

5 participants

@jbergstroem

Just ran the test suite against tcl 8.6.0, which worked nicely. The test suite currently uses tclsh8.5 everywhere, so I made calls to tclsh instead. Since there's currently a 8.5 requirement, I added a check for that.

@djc

+1.

@bwright

Seems like a great commit! +1

@jbergstroem

I haven't checked around much, but there might be issues with tcl installs where tclsh isn't a symlink to the proper version. I'll have a look at freebsd, openbsd and smartos tomorrow.

@antirez
Owner

Thanks, seems like a good idea if it works well everywhere.

Probably it's worth to catch "package require Tcl 8.5" and in case of errors properly format an error message where the user is told that to run the Redis test at least Tcl 8.5 is required.

Cheers

@jbergstroem

@antirez: On catching tcl 8.5, I thought the error tcl gave you was pretty clear (never mind version numbers):

# make test
cd src && make test
make[1]: Entering directory `/home/jbergstroem/redis/src'
/usr/bin/tclsh
version conflict for package "Tcl": have 8.6.0, need 8.7
    while executing
"package require Tcl 8.7"
    (file "tests/test_helper.tcl" line 5)
make[1]: *** [test] Error 1

As for tclsh over tcl<version>, the results are pretty disappointing:

OS tcl version tclsh
smartos 8.4 YES
openbsd 5.2 8.5 NO
ubuntu 12.10 8.5 YES
freebsd 9.1 8.5 NO

There doesn't seem to be a good way to carry a patch that uses tclsh. We probably want to use which/et al from shell and perhaps communicating to tcl with ENV. If others agree, I'll whip something together.

@jbergstroem

Being too impatient, I went ahead and did above changes. Overall, I think its a better approach.

@antirez: I still think the tclsh version feedback is good as-is. Let me know if I should give it some more love. The shell script will also give further feedback before entering tcl.

See #906 for new branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  runtest
@@ -1,5 +1,5 @@
#!/bin/sh
-TCL=tclsh8.5
+TCL=tclsh
which $TCL
if [ "$?" != "0" ]
then
View
2  tests/integration/replication-4.tcl
@@ -1,5 +1,5 @@
proc start_bg_complex_data {host port db ops} {
- exec tclsh8.5 tests/helpers/bg_complex_data.tcl $host $port $db $ops &
+ exec tclsh tests/helpers/bg_complex_data.tcl $host $port $db $ops &
}
proc stop_bg_complex_data {handle} {
View
2  tests/integration/replication.tcl
@@ -78,7 +78,7 @@ start_server {tags {"repl"}} {
}
proc start_write_load {host port seconds} {
- exec tclsh8.5 tests/helpers/gen_write_load.tcl $host $port $seconds &
+ exec tclsh tests/helpers/gen_write_load.tcl $host $port $seconds &
}
proc stop_write_load {handle} {
View
4 tests/test_helper.tcl
@@ -2,6 +2,8 @@
# This softare is released under the BSD License. See the COPYING file for
# more information.
+package require Tcl 8.5
+
set tcl_precision 17
source tests/support/redis.tcl
source tests/support/server.tcl
@@ -189,7 +191,7 @@ proc test_server_main {} {
set start_port [expr {$::port+100}]
for {set j 0} {$j < $::numclients} {incr j} {
set start_port [find_available_port $start_port]
- set p [exec tclsh8.5 [info script] {*}$::argv \
+ set p [exec tclsh [info script] {*}$::argv \
--client $port --port $start_port &]
lappend ::clients_pids $p
incr start_port 10
Something went wrong with that request. Please try again.