Add cross-version test framework (and a simple test) (#1371)

This includes a way to run two versions of the server from the TCL test
framework. It's a preparation to add more cross-version tests. The
runtest script accepts a new parameter

    ./runtest --other-server-path path/to/valkey-server

and a new tag "needs:other-server" for test cases and start_server.
Tests with this tag are automatically skipped if `--other-server-path`
is not provided.

This PR adds it in a CI job with Valkey 7.2.7 by downloading a binary
release.

Fixes #76

---------

Signed-off-by: Viktor Söderqvist <[email protected]>

.github/workflows/ci.yml

@@ -18,10 +18,15 @@ jobs:
         # Fail build if there are warnings
         # build with TLS just for compilation coverage
         run: make -j4 all-with-unit-tests SERVER_CFLAGS='-Werror' BUILD_TLS=yes USE_FAST_FLOAT=yes
+      - name: install old server for compatibility testing
+        run: |
+          cd tests/tmp
+          wget https://download.valkey.io/releases/valkey-7.2.7-noble-x86_64.tar.gz
+          tar -xvf valkey-7.2.7-noble-x86_64.tar.gz
       - name: test
         run: |
           sudo apt-get install tcl8.6 tclx
-          ./runtest --verbose --tags -slow --dump-logs
+          ./runtest --verbose --tags -slow --dump-logs --other-server-path tests/tmp/valkey-7.2.7-noble-x86_64/bin/valkey-server
       - name: module api test
         run: CFLAGS='-Werror' ./runtest-moduleapi --verbose --dump-logs
       - name: validate commands.def up to date

tests/README.md

@@ -24,16 +24,17 @@ There are additional runtime options that can further adjust the test suite to
 match different external server configurations. All options are listed by
 `./runtest --help`. The following table is just a subset of the options:
 
-| Option               | Impact                                                   |
-| -------------------- | -------------------------------------------------------- |
-| `--singledb`         | Only use database 0, don't assume others are supported. |
-| `--ignore-encoding`  | Skip all checks for specific encoding.  |
-| `--ignore-digest`    | Skip key value digest validations. |
-| `--cluster-mode`     | Run in strict Valkey Cluster compatibility mode. |
-| `--large-memory`     | Enables tests that consume more than 100MB |
-| `--tls`              | Run tests with TLS. See below. |
-| `--tls-module`       | Run tests with TLS, when TLS support is built as a module. |
-| `--help`             | Displays the full set of options. |
+| Option                     | Impact                                                   |
+| -------------------------- | -------------------------------------------------------- |
+| `--singledb`               | Only use database 0, don't assume others are supported. |
+| `--ignore-encoding`        | Skip all checks for specific encoding.  |
+| `--ignore-digest`          | Skip key value digest validations. |
+| `--cluster-mode`           | Run in strict Valkey Cluster compatibility mode. |
+| `--large-memory`           | Enables tests that consume more than 100MB |
+| `--tls`                    | Run tests with TLS. See below. |
+| `--tls-module`             | Run tests with TLS, when TLS support is built as a module. |
+| `--other-server-path PATH` | Run compatibility tests with an other server executable. |
+| `--help`                   | Displays the full set of options. |
 
 Running with TLS requires the following preparations:
 
@@ -86,18 +87,22 @@ Tags can be applied in different context levels:
 The following compatibility and capability tags are currently used:
 
 | Tag                       | Indicates |
-| ---------------------     | --------- |
+| ------------------------- | --------- |
 | `external:skip`           | Not compatible with external servers. |
+| `cluster`                 | Uses cluster with multiple nodes. |
 | `cluster:skip`            | Not compatible with `--cluster-mode`. |
 | `large-memory`            | Test that requires more than 100MB |
+| `tls`                     | Uses TLS. |
 | `tls:skip`                | Not compatible with `--tls`. |
-| `needs:repl`              | Uses replication and needs to be able to `SYNC` from server. |
+| `ipv6`                    | Uses IPv6. |
+| `needs:repl`, `repl`      | Uses replication and needs to be able to `SYNC` from server. |
 | `needs:debug`             | Uses the `DEBUG` command or other debugging focused commands (like `OBJECT REFCOUNT`). |
 | `needs:pfdebug`           | Uses the `PFDEBUG` command. |
 | `needs:config-maxmemory`  | Uses `CONFIG SET` to manipulate memory limit, eviction policies, etc. |
 | `needs:config-resetstat`  | Uses `CONFIG RESETSTAT` to reset statistics. |
 | `needs:reset`             | Uses `RESET` to reset client connections. |
 | `needs:save`              | Uses `SAVE` or `BGSAVE` to create an RDB file. |
+| `needs:other-server`      | Requires `--other-server-path`. |
 
 When using an external server (`--host` and `--port`), filtering using the
 `external:skip` tags is done automatically.

tests/integration/cross-version-replication.tcl

@@ -0,0 +1,34 @@
+# Test replication from an older version primary.
+#
+# Use minimal.conf to make sure we don't use any configs not supported on the old version.
+
+proc server_name_and_version {} {
+    set server_name [s server_name]
+    if {$server_name eq {}} {
+        set server_name redis
+    }
+    set server_version [s "${server_name}_version"]
+    return "$server_name $server_version"
+}
+
+start_server {tags {"repl needs:other-server external:skip"} start-other-server 1 config "minimal.conf"} {
+    set primary_name_and_version [server_name_and_version]
+    r set foo bar
+
+    start_server {} {
+        test "Start replication from $primary_name_and_version" {
+            r replicaof [srv -1 host] [srv -1 port]
+            wait_for_sync r
+            # The key has been transferred.
+            assert_equal bar [r get foo]
+            assert_equal up [s master_link_status]
+        }
+
+        test "Replicate a SET command from $primary_name_and_version" {
+            r -1 set baz quux
+            wait_for_ofs_sync [srv 0 client] [srv -1 client]
+            set reply [r get baz]
+            assert_equal $reply quux
+        }
+    }
+}

tests/support/server.tcl

@@ -2,9 +2,9 @@ set ::global_overrides {}
 set ::tags {}
 set ::valgrind_errors {}
 
-proc start_server_error {config_file error} {
+proc start_server_error {executable config_file error} {
     set err {}
-    append err "Can't start the Valkey server\n"
+    append err "Can't start $executable\n"
     append err "CONFIGURATION:\n"
     append err [exec cat $config_file]
     append err "\nERROR:\n"
@@ -216,6 +216,11 @@ proc tags_acceptable {tags err_return} {
         return 0
     }
 
+    if {$::other_server_path eq {} && [lsearch $tags "needs:other-server"] >= 0} {
+        set err "Other server path not provided"
+        return 0
+    }
+
     if {$::external && [lsearch $tags "external:skip"] >= 0} {
         set err "Not supported on external server"
         return 0
@@ -289,8 +294,8 @@ proc create_server_config_file {filename config config_lines} {
     close $fp
 }
 
-proc spawn_server {config_file stdout stderr args} {
-    set cmd [list src/valkey-server $config_file]
+proc spawn_server {executable config_file stdout stderr args} {
+    set cmd [list $executable $config_file]
     set args {*}$args
     if {[llength $args] > 0} {
         lappend cmd {*}$args
@@ -319,7 +324,7 @@ proc spawn_server {config_file stdout stderr args} {
 }
 
 # Wait for actual startup, return 1 if port is busy, 0 otherwise
-proc wait_server_started {config_file stdout stderr pid} {
+proc wait_server_started {executable config_file stdout stderr pid} {
     set checkperiod 100; # Milliseconds
     set maxiter [expr {120*1000/$checkperiod}] ; # Wait up to 2 minutes.
     set port_busy 0
@@ -330,7 +335,7 @@ proc wait_server_started {config_file stdout stderr pid} {
         after $checkperiod
         incr maxiter -1
         if {$maxiter == 0} {
-            start_server_error $config_file "No PID detected in log $stdout"
+            start_server_error $executable $config_file "No PID detected in log $stdout"
             puts "--- LOG CONTENT ---"
             puts [exec cat $stdout]
             puts "-------------------"
@@ -347,7 +352,7 @@ proc wait_server_started {config_file stdout stderr pid} {
         # Configuration errors are unexpected, but it's helpful to fail fast
         # to give the feedback to the test runner.
         if {[regexp {FATAL CONFIG FILE ERROR} [exec cat $stderr]]} {
-            start_server_error $config_file "Configuration issue prevented Valkey startup"
+            start_server_error $executable $config_file "Configuration issue prevented Valkey startup"
             break
         }
     }
@@ -441,13 +446,17 @@ proc start_server {options {code undefined}} {
     set args {}
     set keep_persistence false
     set config_lines {}
+    set start_other_server 0
 
     # Wait for the server to be ready and check for server liveness/client connectivity before starting the test.
     set wait_ready true
 
     # parse options
     foreach {option value} $options {
         switch $option {
+            "start-other-server" {
+                set start_other_server $value ; # boolean, 0 or 1
+            }
             "config" {
                 set baseconfig $value
             }
@@ -498,6 +507,15 @@ proc start_server {options {code undefined}} {
         return
     }
 
+    if {$start_other_server} {
+        set executable $::other_server_path
+        if {![file executable $executable]} {
+            error "File not found or not executable: $executable"
+        }
+    } else {
+        set executable "src/valkey-server"
+    }
+
     set data [split [exec cat "tests/assets/$baseconfig"] "\n"]
     set config {}
     if {$::tls} {
@@ -588,15 +606,15 @@ proc start_server {options {code undefined}} {
     set server_started 0
     while {$server_started == 0} {
         if {$::verbose} {
-            puts -nonewline "=== ($tags) Starting server ${::host}:${port} "
+            puts -nonewline "=== ($tags) Starting server on ${::host}:${port} "
         }
 
         send_data_packet $::test_server_fd "server-spawning" "port $port"
 
-        set pid [spawn_server $config_file $stdout $stderr $args]
+        set pid [spawn_server $executable $config_file $stdout $stderr $args]
 
         # check that the server actually started
-        set port_busy [wait_server_started $config_file $stdout $stderr $pid]
+        set port_busy [wait_server_started $executable $config_file $stdout $stderr $pid]
 
         # Sometimes we have to try a different port, even if we checked
         # for availability. Other test clients may grab the port before we
@@ -634,7 +652,7 @@ proc start_server {options {code undefined}} {
         if {!$serverisup} {
             set err {}
             append err [exec cat $stdout] "\n" [exec cat $stderr]
-            start_server_error $config_file $err
+            start_server_error $executable $config_file $err
             return
         }
         set server_started 1
@@ -647,6 +665,7 @@ proc start_server {options {code undefined}} {
     if {[dict exists $config $port_param]} { set port [dict get $config $port_param] }
 
     # setup config dict
+    dict set srv "executable" $executable
     dict set srv "config_file" $config_file
     dict set srv "config" $config
     dict set srv "pid" $pid
@@ -801,12 +820,13 @@ proc restart_server {level wait_ready rotate_logs {reconnect 1} {shutdown sigter
         close $fd
     }
 
+    set executable [dict get $srv "executable"]
     set config_file [dict get $srv "config_file"]
 
-    set pid [spawn_server $config_file $stdout $stderr {}]
+    set pid [spawn_server $executable $config_file $stdout $stderr {}]
 
     # check that the server actually started
-    wait_server_started $config_file $stdout $stderr $pid
+    wait_server_started $executable $config_file $stdout $stderr $pid
 
     # update the pid in the servers list
     dict set srv "pid" $pid

tests/test_helper.tcl

@@ -69,6 +69,7 @@ set ::single_tests {}
 set ::run_solo_tests {}
 set ::skip_till ""
 set ::external 0; # If "1" this means, we are running against external instance
+set ::other_server_path {}; # Used in upgrade and inter-version tests
 set ::file ""; # If set, runs only the tests in this comma separated list
 set ::curfile ""; # Hold the filename of the current suite
 set ::accurate 0; # If true runs fuzz tests with more iterations
@@ -568,40 +569,54 @@ proc send_data_packet {fd status data {elapsed 0}} {
 
 proc print_help_screen {} {
     puts [join {
-        "--cluster          Run the cluster tests, by default cluster tests run along with all tests."
-        "--moduleapi        Run the module API tests, this option should only be used in runtest-moduleapi which will build the test module."
+        # This is for terminal output, so assume default term width of 80 columns. -----|
+        "--cluster          Run the cluster tests, by default cluster tests run along"
+        "                   with all tests."
+        "--moduleapi        Run the module API tests, this option should only be used in"
+        "                   runtest-moduleapi which will build the test module."
         "--valgrind         Run the test over valgrind."
         "--durable          suppress test crashes and keep running"
         "--stack-logging    Enable OSX leaks/malloc stack logging."
         "--accurate         Run slow randomized tests for more iterations."
         "--quiet            Don't show individual tests."
-        "--single <unit>    Just execute the specified unit (see next option). This option can be repeated."
+        "--single <unit>    Just execute the specified unit (see next option). This"
+        "                   option can be repeated."
         "--verbose          Increases verbosity."
         "--list-tests       List all the available test units."
-        "--only <test>      Just execute the specified test by test name or tests that match <test> regexp (if <test> starts with '/'). This option can be repeated."
+        "--only <test>      Just execute the specified test by test name or tests that"
+        "                   match <test> regexp (if <test> starts with '/'). This option"
+        "                   can be repeated."
         "--skip-till <unit> Skip all units until (and including) the specified one."
         "--skipunit <unit>  Skip one unit."
         "--clients <num>    Number of test clients (default 16)."
         "--timeout <sec>    Test timeout in seconds (default 20 min)."
         "--force-failure    Force the execution of a test that always fails."
         "--config <k> <v>   Extra config file argument."
-        "--skipfile <file>  Name of a file containing test names or regexp patterns (if <test> starts with '/') that should be skipped (one per line). This option can be repeated."
-        "--skiptest <test>  Test name or regexp pattern (if <test> starts with '/') to skip. This option can be repeated."
-        "--tags <tags>      Run only tests having specified tags or not having '-' prefixed tags."
+        "--skipfile <file>  Name of a file containing test names or regexp patterns (if"
+        "                   <test> starts with '/') that should be skipped (one per"
+        "                   line). This option can be repeated."
+        "--skiptest <test>  Test name or regexp pattern (if <test> starts with '/') to"
+        "                   skip. This option can be repeated."
+        "--tags <tags>      Run only tests having specified tags or not having '-'"
+        "                   prefixed tags."
         "--dont-clean       Don't delete valkey log files after the run."
         "--dont-pre-clean   Don't delete existing valkey log files before the run."
         "--no-latency       Skip latency measurements and validation by some tests."
         "--fastfail         Exit immediately once the first test fails."
         "--stop             Blocks once the first test fails."
         "--loop             Execute the specified set of tests forever."
         "--loops <count>    Execute the specified set of tests several times."
-        "--wait-server      Wait after server is started (so that you can attach a debugger)."
+        "--wait-server      Wait after server is started (so that you can attach a"
+        "                   debugger)."
         "--dump-logs        Dump server log on test failure."
         "--io-threads       Run tests with IO threads."
         "--tls              Run tests in TLS mode."
         "--tls-module       Run tests in TLS mode with Valkey module."
         "--host <addr>      Run tests against an external host."
         "--port <port>      TCP port to use against external host."
+        "--other-server-path <path>"
+        "                   Path to another version of valkey-server, used for inter-"
+        "                   version compatibility testing."
         "--baseport <port>  Initial port number for spawned valkey servers."
         "--portcount <num>  Port range for spawned valkey servers."
         "--singledb         Use a single database, avoid SELECT."
@@ -676,6 +691,9 @@ for {set j 0} {$j < [llength $argv]} {incr j} {
     } elseif {$opt eq {--port}} {
         set ::port $arg
         incr j
+    } elseif {$opt eq {--other-server-path}} {
+        set ::other_server_path $arg
+        incr j
     } elseif {$opt eq {--baseport}} {
         set ::baseport $arg
         incr j