Prevent tests setting singledb from breaking other tests (#2056)

Some Tcl test suites set the global variable `::singledb`. If they don't
restore the value afterwards, they can affect other test suites, which
leads to problems that are hard to track down.

This change introduces a test tag 'singledb' that has the same effect,
but it is local to the scope of the tag (a tags block, start_server or a
single test case). The tests should use the tag instead of setting the
global.

Additional change: Use ECHO instead of PING in `valkey_deferring_client`
and `valkey_client` procs when singledb is set, to avoid failing when
the server is loading. (SELECT and ECHO are allowed while loading, but
not PING.)

---------

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>

Author: zuiderkwast

tests/README.md

@@ -103,6 +103,7 @@ The following compatibility and capability tags are currently used:
 | `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`. |
+| `singledb`                | Test runs as if `--singledb` was given. |
 
 When using an external server (`--host` and `--port`), filtering using the
 `external:skip` tags is done automatically.

tests/integration/aof.tcl

@@ -674,11 +674,7 @@ tags {"aof external:skip"} {
     }
 }
 
-# make sure the test infra won't use SELECT
-set old_singledb $::singledb
-set ::singledb 1
-
-tags {"aof cluster external:skip"} {
+tags {"aof cluster external:skip singledb"} {
     test {Test cluster slots / cluster shards in aof won't crash} {
         create_aof $aof_dirpath $aof_file {
             append_to_aof [formatCommand cluster slots]
@@ -735,5 +731,3 @@ tags {"aof cluster external:skip"} {
         clean_aof_persistence $aof_dirpath
     }
 }
-
-set ::singledb $old_singledb

tests/support/server.tcl

@@ -447,6 +447,7 @@ proc start_server {options {code undefined}} {
     set keep_persistence false
     set config_lines {}
     set start_other_server 0
+    set old_singledb $::singledb
 
     # Wait for the server to be ready and check for server liveness/client connectivity before starting the test.
     set wait_ready true
@@ -494,15 +495,22 @@ proc start_server {options {code undefined}} {
     if {![tags_acceptable $::tags err]} {
         incr ::num_aborted
         send_data_packet $::test_server_fd ignore $err
+        set ::singledb $old_singledb
         set ::tags [lrange $::tags 0 end-[llength $tags]]
         return
     }
 
+    # Tags that override global options
+    if {[lsearch $::tags singledb] >= 0} {
+        set ::singledb 1
+    }
+
     # If we are running against an external server, we just push the
     # host/port pair in the stack the first time
     if {$::external} {
         run_external_server_test $code $overrides
 
+        set ::singledb $old_singledb
         set ::tags [lrange $::tags 0 end-[llength $tags]]
         return
     }
@@ -773,13 +781,15 @@ proc start_server {options {code undefined}} {
         # pop the server object
         set ::servers [lrange $::servers 0 end-1]
 
+        set ::singledb $old_singledb
         set ::tags [lrange $::tags 0 end-[llength $tags]]
         kill_server $srv
         if {!$keep_persistence} {
             clean_persistence $srv
         }
         set _ ""
     } else {
+        set ::singledb $old_singledb
         set ::tags [lrange $::tags 0 end-[llength $tags]]
         set _ $srv
     }

tests/support/test.tcl

@@ -210,13 +210,17 @@ proc test {name code {okpattern undefined} {tags {}}} {
         return
     }
 
+    set old_singledb $::singledb
     set tags [concat $::tags $tags]
     if {![tags_acceptable $tags err]} {
         incr ::num_aborted
         send_data_packet $::test_server_fd ignore "$name: $err"
         return
     }
 
+    if {[lsearch $tags singledb] >= 0} {
+        set ::singledb 1
+    }
     incr ::num_tests
     set details {}
     lappend details "$name in $::curfile"
@@ -300,5 +304,6 @@ proc test {name code {okpattern undefined} {tags {}}} {
             send_data_packet $::test_server_fd err "Detected a memory leak in test '$name': $output"
         }
     }
+    set ::singledb $old_singledb
     set ::cur_test $prev_test
 }

tests/test_helper.tcl

@@ -213,7 +213,8 @@ proc valkey_deferring_client {args} {
         $client read
     } else {
         # For timing/symmetry with the above select
-        $client ping
+        # that doesn't return error while loading.
+        $client echo goodday
         $client read
     }
     return $client
@@ -232,7 +233,7 @@ proc valkey_client {args} {
     # select the right db and read the response (OK), or at least ping
     # the server if we're in a singledb mode.
     if {$::singledb} {
-        $client ping
+        $client echo hey
     } else {
         $client select 9
     }

tests/unit/cluster/base.tcl

@@ -1,10 +1,6 @@
 # Check the basic monitoring and failover capabilities.
 
-# make sure the test infra won't use SELECT
-set old_singledb $::singledb
-set ::singledb 1
-
-tags {tls:skip external:skip cluster} {
+tags {tls:skip external:skip cluster singledb} {
 
 set base_conf [list cluster-enabled yes]
 start_multiple_servers 5 [list overrides $base_conf] {
@@ -122,5 +118,3 @@ test "CLUSTER SLAVES and CLUSTER REPLICAS with zero replicas" {
 } ;# stop servers
 
 } ;# tags
-
-set ::singledb $old_singledb

tests/unit/cluster/cli.tcl

@@ -2,12 +2,8 @@
 
 source tests/support/cli.tcl
 
-# make sure the test infra won't use SELECT
-set old_singledb $::singledb
-set ::singledb 1
-
 # cluster creation is complicated with TLS, and the current tests don't really need that coverage
-tags {tls:skip external:skip cluster} {
+tags {tls:skip external:skip cluster singledb} {
 
 set base_conf [list cluster-enabled yes cluster-node-timeout 1000]
 start_multiple_servers 3 [list overrides $base_conf] {
@@ -535,5 +531,3 @@ start_multiple_servers 3 [list overrides $base_conf] {
 }
 
 }
-
-set ::singledb $old_singledb

tests/unit/cluster/cluster-multiple-meets.tcl

@@ -1,8 +1,4 @@
-# make sure the test infra won't use SELECT
-set old_singledb $::singledb
-set ::singledb 1
-
-tags {tls:skip external:skip cluster} {
+tags {tls:skip external:skip cluster singledb} {
     set base_conf [list cluster-enabled yes]
     start_multiple_servers 2 [list overrides $base_conf] {
         test "Cluster nodes are reachable" {
@@ -86,6 +82,3 @@ tags {tls:skip external:skip cluster} {
         }
     } ;# stop servers
 } ;# tags
-
-set ::singledb $old_singledb
-

tests/unit/cluster/cluster-reliable-meet.tcl

@@ -1,8 +1,4 @@
-# make sure the test infra won't use SELECT
-set old_singledb $::singledb
-set ::singledb 1
-
-tags {tls:skip external:skip cluster} {
+tags {tls:skip external:skip cluster singledb} {
     set CLUSTER_PACKET_TYPE_PING 0
     set CLUSTER_PACKET_TYPE_PONG 1
     set CLUSTER_PACKET_TYPE_MEET 2
@@ -76,8 +72,6 @@ tags {tls:skip external:skip cluster} {
     } ;# stop servers
 } ;# tags
 
-set ::singledb $old_singledb
-
 proc cluster_get_first_node_in_handshake id {
     set nodes [get_cluster_nodes $id]
     foreach n $nodes {

tests/unit/cluster/cross-version-cluster.tcl

@@ -2,11 +2,7 @@
 #
 # Use minimal.conf to make sure we don't use any configs not supported on the old version.
 
-# make sure the test infra won't use SELECT
-set old_singledb $::singledb
-set ::singledb 1
-
-tags {external:skip needs:other-server cluster} {
+tags {external:skip needs:other-server cluster singledb} {
     # To run this test use the `--other-server-path` parameter and pass in a compatible server path supporting
     # SendClusterMessage module API.
     #
@@ -32,5 +28,3 @@ tags {external:skip needs:other-server cluster} {
         }
     }
 }
-
-set ::singledb $old_singledb