test infra - improve test skipping ability

- skip full units
- skip a single test (not just a list of tests)
- when skipping tag, skip spinning up servers, not just the tests
- skip tags when running against an external server too
- allow using multiple tags (split them)

(cherry picked from commit 5c61f1a6ed876186b944e79f903354cd81077bb6)

tests/support/server.tcl

@@ -160,7 +160,19 @@ proc server_is_up {host port retrynum} {
 
 # doesn't really belong here, but highly coupled to code in start_server
 proc tags {tags code} {
+    # If we 'tags' contain multiple tags, quoted and seperated by spaces,
+    # we want to get rid of the quotes in order to have a proper list
+    set tags [string map { \" "" } $tags]
     set ::tags [concat $::tags $tags]
+    # We skip unwanted tags
+    foreach tag $::denytags {
+        if {[lsearch $::tags $tag] >= 0} {
+            incr ::num_aborted
+            send_data_packet $::test_server_fd ignore "Tag: $tag"
+            set ::tags [lrange $::tags 0 end-[llength $tags]]
+            return
+        }
+    }
     uplevel 1 $code
     set ::tags [lrange $::tags 0 end-[llength $tags]]
 }
@@ -226,24 +238,6 @@ proc wait_server_started {config_file stdout pid} {
 }
 
 proc start_server {options {code undefined}} {
-    # If we are running against an external server, we just push the
-    # host/port pair in the stack the first time
-    if {$::external} {
-        if {[llength $::servers] == 0} {
-            set srv {}
-            dict set srv "host" $::host
-            dict set srv "port" $::port
-            set client [redis $::host $::port 0 $::tls]
-            dict set srv "client" $client
-            $client select 9
-
-            # append the server to the stack
-            lappend ::servers $srv
-        }
-        uplevel 1 $code
-        return
-    }
-
     # setup defaults
     set baseconfig "default.conf"
     set overrides {}
@@ -260,8 +254,10 @@ proc start_server {options {code undefined}} {
                 set overrides $value
             }
             "tags" {
-                set tags $value
+                # If we 'tags' contain multiple tags, quoted and seperated by spaces,
-                set ::tags [concat $::tags $value]
+                # we want to get rid of the quotes in order to have a proper list
+                set tags [string map { \" "" } $value]
+                set ::tags [concat $::tags $tags]
             }
             "keep_persistence" {
                 set keep_persistence $value
@@ -272,6 +268,39 @@ proc start_server {options {code undefined}} {
         }
     }
 
+    # We skip unwanted tags
+    foreach tag $::denytags {
+        if {[lsearch $::tags $tag] >= 0} {
+            incr ::num_aborted
+            send_data_packet $::test_server_fd ignore "Tag: $tag"
+            set ::tags [lrange $::tags 0 end-[llength $tags]]
+            return
+        }
+    }
+
+    # If we are running against an external server, we just push the
+    # host/port pair in the stack the first time
+    if {$::external} {
+        if {[llength $::servers] == 0} {
+            set srv {}
+            dict set srv "host" $::host
+            dict set srv "port" $::port
+            set client [redis $::host $::port 0 $::tls]
+            dict set srv "client" $client
+            $client select 9
+
+            set config {}
+            dict set config "port" $::port
+            dict set srv "config" $config
+
+            # append the server to the stack
+            lappend ::servers $srv
+        }
+        uplevel 1 $code
+        set ::tags [lrange $::tags 0 end-[llength $tags]]
+        return
+    }
+
     set data [split [exec cat "tests/assets/$baseconfig"] "\n"]
     set config {}
     if {$::tls} {

tests/support/test.tcl

@@ -99,16 +99,7 @@ proc wait_for_condition {maxtries delay e _else_ elsescript} {
     }
 }
 
-proc test {name code {okpattern undefined}} {
+proc test {name code {okpattern undefined} {options undefined}} {
-    # abort if tagged with a tag to deny
-    foreach tag $::denytags {
-        if {[lsearch $::tags $tag] >= 0} {
-            incr ::num_aborted
-            send_data_packet $::test_server_fd ignore $name
-            return
-        }
-    }
-
     # abort if test name in skiptests
     if {[lsearch $::skiptests $name] >= 0} {
         incr ::num_skipped

tests/test_helper.tcl

@@ -85,6 +85,7 @@ set ::verbose 0
 set ::quiet 0
 set ::denytags {}
 set ::skiptests {}
+set ::skipunits {}
 set ::allowtags {}
 set ::only_tests {}
 set ::single_tests {}
@@ -423,6 +424,12 @@ proc lpop {listVar {count 1}} {
     set ele
 }
 
+proc lremove {listVar value} {
+    upvar 1 $listVar var
+    set idx [lsearch -exact $var $value]
+    set var [lreplace $var $idx $idx]
+}
+
 # A new client is idle. Remove it from the list of active clients and
 # if there are still test units to run, launch them.
 proc signal_idle_client fd {
@@ -521,11 +528,13 @@ proc print_help_screen {} {
         "--list-tests       List all the available test units."
         "--only <test>      Just execute the specified test by test name. 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 10 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 that should be skipped (one per line)."
+        "--skiptest <name>  Name of a file containing test names that should be skipped (one per line)."
         "--dont-clean       Don't delete redis log files after the run."
         "--stop             Blocks once the first test fails."
         "--loop             Execute the specified set of tests forever."
@@ -563,6 +572,9 @@ for {set j 0} {$j < [llength $argv]} {incr j} {
         set file_data [read $fp]
         close $fp
         set ::skiptests [split $file_data "\n"]
+    } elseif {$opt eq {--skiptest}} {
+        lappend ::skiptests $arg
+        incr j
     } elseif {$opt eq {--valgrind}} {
         set ::valgrind 1
     } elseif {$opt eq {--stack-logging}} {
@@ -601,6 +613,9 @@ for {set j 0} {$j < [llength $argv]} {incr j} {
     } elseif {$opt eq {--only}} {
         lappend ::only_tests $arg
         incr j
+    } elseif {$opt eq {--skipunit}} {
+        lappend ::skipunits $arg
+        incr j
     } elseif {$opt eq {--skip-till}} {
         set ::skip_till $arg
         incr j
@@ -638,13 +653,23 @@ for {set j 0} {$j < [llength $argv]} {incr j} {
     }
 }
 
-# If --skil-till option was given, we populate the list of single tests
+set filtered_tests {}
+
+# Set the filtered tests to be the short list (single_tests) if exists.
+# Otherwise, we start filtering all_tests
+if {[llength $::single_tests] > 0} {
+    set filtered_tests $::single_tests
+} else {
+    set filtered_tests $::all_tests
+}
+
+# If --skip-till option was given, we populate the list of single tests
 # to run with everything *after* the specified unit.
 if {$::skip_till != ""} {
     set skipping 1
     foreach t $::all_tests {
-        if {$skipping == 0} {
+        if {$skipping == 1} {
-            lappend ::single_tests $t
+            lremove filtered_tests $t
         }
         if {$t == $::skip_till} {
             set skipping 0
@@ -656,10 +681,20 @@ if {$::skip_till != ""} {
     }
 }
 
+# If --skipunits option was given, we populate the list of single tests
+# to run with everything *not* in the skipunits list.
+if {[llength $::skipunits] > 0} {
+    foreach t $::all_tests {
+        if {[lsearch $::skipunits $t] != -1} {
+            lremove filtered_tests $t
+        }
+    }
+}
+
 # Override the list of tests with the specific tests we want to run
-# in case there was some filter, that is --single or --skip-till options.
+# in case there was some filter, that is --single, -skipunit or --skip-till options.
-if {[llength $::single_tests] > 0} {
+if {[llength $filtered_tests] < [llength $::all_tests]} {
-    set ::all_tests $::single_tests
+    set ::all_tests $filtered_tests
 }
 
 proc attach_to_replication_stream {} {