Skip tests based on SKIP_REASON, not return value

Currently, {,group_,device_}requires are required to both set
SKIP_REASON and return non-zero. This is somewhat redundant, and will be
inconsistent with skipping from test{,_device} that is about to be
implemented. Let's ignore the return value and always skip the test if
SKIP_REASON is set. The _have_foo and _test_dev_foo helpers should still
return so that they can be chained together.

diff --git a/check b/check
index 5153dcd8645467f722993eddd16a5d87aa308077..5aeedd2ce79f7d0ee442d8aa4426450d25b7c479 100755 (executable)
--- a/check
+++ b/check
@@ -423,6 +423,15 @@ _test_dev_is_zoned() {
        return 0
 }
 
+_test_dev_is_not_zoned() {
+       if _test_dev_is_zoned; then
+               SKIP_REASON="${TEST_DEV} is a zoned block device"
+               return 1
+       fi
+       unset SKIP_REASON
+       return 0
+}
+
 _run_test() {
        TEST_NAME="$1"
        CAN_BE_ZONED=0
@@ -435,9 +444,12 @@ _run_test() {
        . "tests/${TEST_NAME}"
 
        if declare -fF test >/dev/null; then
-               if declare -fF requires >/dev/null && ! requires; then
-                       _output_notrun "$TEST_NAME"
-                       return 0
+               if declare -fF requires >/dev/null; then
+                       requires
+                       if [[ -v SKIP_REASON ]]; then
+                               _output_notrun "$TEST_NAME"
+                               return 0
+                       fi
                fi
 
                RESULTS_DIR="$OUTPUT/nodev"
@@ -471,24 +483,30 @@ _run_test() {
                        return 0
                fi
 
-               if declare -fF requires >/dev/null && ! requires; then
-                       _output_notrun "$TEST_NAME"
-                       return 0
+               if declare -fF requires >/dev/null; then
+                       requires
+                       if [[ -v SKIP_REASON ]]; then
+                               _output_notrun "$TEST_NAME"
+                               return 0
+                       fi
                fi
 
                local ret=0
                for TEST_DEV in "${TEST_DEVS[@]}"; do
                        TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}"
                        TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}"
-                       if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then
-                               SKIP_REASON="${TEST_DEV} is a zoned block device"
+                       if (( !CAN_BE_ZONED )) && ! _test_dev_is_not_zoned; then
                                _output_notrun "$TEST_NAME => $(basename "$TEST_DEV")"
+                               unset SKIP_REASON
                                continue
                        fi
-                       unset SKIP_REASON
-                       if declare -fF device_requires >/dev/null && ! device_requires; then
-                               _output_notrun "$TEST_NAME => $(basename "$TEST_DEV")"
-                               continue
+                       if declare -fF device_requires >/dev/null; then
+                               device_requires
+                               if [[ -v SKIP_REASON ]]; then
+                                       _output_notrun "$TEST_NAME => $(basename "$TEST_DEV")"
+                                       unset SKIP_REASON
+                                       continue
+                               fi
                        fi
                        RESULTS_DIR="$OUTPUT/$(basename "$TEST_DEV")"
                        if ! _call_test test_device; then
@@ -514,9 +532,12 @@ _run_group() {
        # shellcheck disable=SC1090
        . "tests/${group}/rc"
 
-       if declare -fF group_requires >/dev/null && ! group_requires; then
-               _output_notrun "${group}/***"
-               return 0
+       if declare -fF group_requires >/dev/null; then
+               group_requires
+               if [[ -v SKIP_REASON ]]; then
+                       _output_notrun "${group}/***"
+                       return 0
+               fi
        fi
 
        if declare -fF group_device_requires >/dev/null; then
@@ -526,9 +547,11 @@ _run_group() {
                        TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}"
                        # shellcheck disable=SC2034
                        TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}"
-                       if ! group_device_requires; then
+                       group_device_requires
+                       if [[ -v SKIP_REASON ]]; then
                                _output_notrun "${group}/*** => $(basename "$TEST_DEV")"
                                unset TEST_DEVS["$i"]
+                               unset SKIP_REASON
                        fi
                done
                # Fix the array indices.

diff --git a/common/rc b/common/rc
index 63b29be7ba74c0bdfab6e4b5c9ae77965dacc026..1893dda2b2f77828fb0b970204df52f1b9372372 100644 (file)
--- a/common/rc
+++ b/common/rc
@@ -139,7 +139,7 @@ _have_kernel_option() {
        for f in /proc/config.gz /boot/config-$(uname -r); do
                [ -e "$f" ] || continue
                if zgrep -q "^CONFIG_${opt}=[my]$" "$f"; then
-                       SKIP_REASON=""
+                       unset SKIP_REASON
                        return 0
                else
                        SKIP_REASON="kernel option $opt has not been enabled"

diff --git a/new b/new
index 5f6e1c01a273cf214a5b46329f53de742b374aaf..9a2d25523e09405eb2e22cdd57e18318cf02c96d 100755 (executable)
--- a/new
+++ b/new
@@ -63,32 +63,30 @@ if [[ ! -e tests/${group} ]]; then
 # . common/foo
 
 # TODO: if this test group has any extra requirements, it should define a
-# group_requires() function. If tests in this group can be run,
-# group_requires() should return 0. Otherwise, it should return non-zero and
-# set the \$SKIP_REASON variable.
+# group_requires() function. If tests in this group cannot be run,
+# group_requires() should set the \$SKIP_REASON variable.
 #
 # Usually, group_requires() just needs to check that any necessary programs and
 # kernel features are available using the _have_foo helpers. If
-# group_requires() returns non-zero, all tests in this group will be skipped.
+# group_requires() sets \$SKIP_REASON, all tests in this group will be skipped.
 group_requires() {
        _have_root
 }
 
 # TODO: if this test group has extra requirements for what devices it can be
 # run on, it should define a group_device_requires() function. If tests in this
-# group can be run on the test device, it should return zero. Otherwise, it
-# should return non-zero and set the \$SKIP_REASON variable. \$TEST_DEV is the
-# full path of the block device (e.g., /dev/nvme0n1 or /dev/sda1), and
-# \$TEST_DEV_SYSFS is the sysfs path of the disk (not the partition, e.g.,
-# /sys/block/nvme0n1 or /sys/block/sda). If the target device is a partition
-# device, \$TEST_DEV_PART_SYSFS is the sysfs path of the partition device
-# (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1). Otherwise,
-# \$TEST_DEV_PART_SYSFS is an empty string.
+# group cannot be run on the test device, it should set the \$SKIP_REASON
+# variable. \$TEST_DEV is the full path of the block device (e.g., /dev/nvme0n1
+# or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the disk (not the
+# partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If the target device
+# is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs path of the
+# partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1).
+# Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
 #
 # Usually, group_device_requires() just needs to check that the test device is
 # the right type of hardware or supports any necessary features using the
-# _test_dev_foo helpers. If group_device_requires() returns non-zero, all tests
-# in this group will be skipped on that device.
+# _test_dev_foo helpers. If group_device_requires() sets \$SKIP_REASON, all
+# tests in this group will be skipped on that device.
 # group_device_requires() {
 #      _test_dev_is_foo && _test_dev_supports_bar
 # }
@@ -153,21 +151,19 @@ DESCRIPTION=""
 # CAN_BE_ZONED=1
 
 # TODO: if this test has any extra requirements, it should define a requires()
-# function. If the test can be run, requires() should return 0. Otherwise, it
-# should return non-zero and set the \$SKIP_REASON variable. Usually,
-# requires() just needs to check that any necessary programs and kernel
-# features are available using the _have_foo helpers. If requires() returns
-# non-zero, the test is skipped.
+# function. If the test cannot be run, requires() should set the \$SKIP_REASON
+# variable. Usually, requires() just needs to check that any necessary programs
+# and kernel features are available using the _have_foo helpers. If requires()
+# sets \$SKIP_REASON, the test is skipped.
 # requires() {
 #      _have_foo
 # }
 
 # TODO: if this test has extra requirements for what devices it can be run on,
-# it should define a device_requires() function. If this test can be run on the
-# test device, it should return zero. Otherwise, it should return non-zero and
-# set the \$SKIP_REASON variable. \$TEST_DEV is the full path of the block
-# device (e.g., /dev/nvme0n1 or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs
-# path of the disk (not the partition, e.g., /sys/block/nvme0n1 or
+# it should define a device_requires() function. If this test cannot be run on
+# the test device, it should set \$SKIP_REASON. \$TEST_DEV is the full path of
+# the block device (e.g., /dev/nvme0n1 or /dev/sda1), and \$TEST_DEV_SYSFS is
+# the sysfs path of the disk (not the partition, e.g., /sys/block/nvme0n1 or
 # /sys/block/sda). If the target device is a partition device,
 # \$TEST_DEV_PART_SYSFS is the sysfs path of the partition device (e.g.,
 # /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1). Otherwise,
@@ -175,7 +171,7 @@ DESCRIPTION=""
 #
 # Usually, device_requires() just needs to check that the test device is the
 # right type of hardware or supports any necessary features using the
-# _test_dev_foo helpers. If device_requires() returns non-zero, the test will
+# _test_dev_foo helpers. If device_requires() sets \$SKIP_REASON, the test will
 # be skipped on that device.
 # device_requires() {
 #      _test_dev_is_foo && _test_dev_supports_bar

diff --git a/tests/meta/007 b/tests/meta/007
index 7978190b378b525edb5317d8e7518765f4cb459d..ae57a67e96314e37bf2db87e0f39675578be3617 100755 (executable)
--- a/tests/meta/007
+++ b/tests/meta/007
@@ -10,7 +10,6 @@ DESCRIPTION="skip in requires()"
 
 requires() {
        SKIP_REASON="(╯°□°)╯︵ ┻━┻"
-       return 1
 }
 
 test() {

diff --git a/tests/meta/008 b/tests/meta/008
index 0305b564c3a01376a045370cbe3bc1d600701d19..6d7b032ad784dfa3e1fa53b9b40febb685772ec9 100755 (executable)
--- a/tests/meta/008
+++ b/tests/meta/008
@@ -10,7 +10,6 @@ DESCRIPTION="skip in device_requires()"
 
 device_requires() {
        SKIP_REASON="(╯°□°)╯︵ $TEST_DEV ┻━┻"
-       return 1
 }
 
 test_device() {

diff --git a/tests/meta/rc b/tests/meta/rc
index 093edd17419f2f2ba4f65880cf220bd5b85cc5fe..121c2951b2baa3a795ca7dda60aaf85b5b05e147 100644 (file)
--- a/tests/meta/rc
+++ b/tests/meta/rc
@@ -9,17 +9,13 @@
 group_requires() {
        if [[ "${META_REQUIRES_SKIP:-}" ]]; then
                SKIP_REASON="META_REQUIRES_SKIP was set"
-               return 1
        fi
-       return 0
 }
 
 group_device_requires() {
        if [[ "${META_DEVICE_REQUIRES_SKIP:-}" ]]; then
                SKIP_REASON="META_DEVICE_REQUIRES_SKIP was set"
-               return 1
        fi
-       return 0
 }
 
 _have_writeable_kmsg() {

diff --git a/tests/nvmeof-mp/rc b/tests/nvmeof-mp/rc
index 278843a1270d8968960345b40db670282df7c206..1fd63144592103924dcf87d0e1b473843be96428 100755 (executable)
--- a/tests/nvmeof-mp/rc
+++ b/tests/nvmeof-mp/rc
@@ -19,10 +19,10 @@ group_requires() {
        # option.
        if _have_kernel_option NVME_MULTIPATH; then
                SKIP_REASON="CONFIG_NVME_MULTIPATH has been set in .config"
-               return 1
+               return
        fi
 
-       _have_configfs || return $?
+       _have_configfs || return
        required_modules=(
                dm_multipath
                dm_queue_length
@@ -38,30 +38,30 @@ group_requires() {
                scsi_dh_emc
                scsi_dh_rdac
        )
-       _have_modules "${required_modules[@]}" || return 1
+       _have_modules "${required_modules[@]}" || return
 
        for p in mkfs.ext4 mkfs.xfs multipath multipathd pidof; do
-               _have_program "$p" || return $?
+               _have_program "$p" || return
        done
 
-       _multipathd_version_ge 0.7.0 || return $?
+       _multipathd_version_ge 0.7.0 || return
        
-       _have_root || return $?
+       _have_root || return
 
-       _have_kernel_option DM_UEVENT || return $?
+       _have_kernel_option DM_UEVENT || return
 
        # shellcheck disable=SC2043
        for name in multipathd; do
                if pidof "$name" >/dev/null; then
                        SKIP_REASON="$name must be stopped before the nvmeof-mp tests are run"
-                       return 1
+                       return
                fi
        done
        if [ -e /etc/multipath.conf ] &&
            ! diff -q /etc/multipath.conf tests/nvmeof-mp/multipath.conf >&/dev/null
        then
                SKIP_REASON="/etc/multipath.conf already exists"
-               return 1
+               return
        fi
 }
 

diff --git a/tests/srp/015 b/tests/srp/015
index e9769dc8a6a0ee6f6798f575e5cb0c8ba29edf08..e03b204796a502d6266cf43833a6606020cae689 100755 (executable)
--- a/tests/srp/015
+++ b/tests/srp/015
@@ -8,10 +8,9 @@ DESCRIPTION="File I/O on top of multipath concurrently with logout and login (mq
 TIMED=1
 
 requires() {
-       _have_modules siw
-       _have_kver 5 5
-       # See also commit 4336c5821a7b ("rdma: add 'link add/delete' commands").
-       _have_iproute2 190404
+       # See also iproute commit 4336c5821a7b ("rdma: add 'link add/delete'
+       # commands").
+       _have_modules siw && _have_kver 5 5 && _have_iproute2 190404
 }
 
 test_disconnect_repeatedly() {

diff --git a/tests/srp/rc b/tests/srp/rc
index 2738c8f6a4e5eb8c0e3de359c7a3ab8dd975f442..07fa5e3a5f266cd221276ef0f81bab96e19e8e47 100755 (executable)
--- a/tests/srp/rc
+++ b/tests/srp/rc
@@ -29,10 +29,10 @@ is_lio_configured() {
 group_requires() {
        local m name p required_modules
 
-       _have_configfs || return $?
+       _have_configfs || return
        if is_lio_configured; then
                SKIP_REASON="LIO must be unloaded before the SRP tests are run"
-               return 1
+               return
        fi
        required_modules=(
                dm_multipath
@@ -56,31 +56,31 @@ group_requires() {
                target_core_iblock
                target_core_mod
        )
-       _have_modules "${required_modules[@]}" || return 1
+       _have_modules "${required_modules[@]}" || return
 
        for p in mkfs.ext4 mkfs.xfs multipath multipathd pidof sg_reset; do
-               _have_program "$p" || return $?
+               _have_program "$p" || return
        done
 
-       _multipathd_version_ge 0.7.0 || return $?
+       _multipathd_version_ge 0.7.0 || return
 
-       _have_root || return $?
+       _have_root || return
 
-       _have_src_program discontiguous-io || return $?
+       _have_src_program discontiguous-io || return
 
-       _have_kernel_option DM_UEVENT || return $?
+       _have_kernel_option DM_UEVENT || return
 
        for name in srp_daemon multipathd; do
                if pidof "$name" >/dev/null; then
                        SKIP_REASON="$name must be stopped before the SRP tests are run"
-                       return 1
+                       return
                fi
        done
        if [ -e /etc/multipath.conf ] &&
            ! diff -q /etc/multipath.conf tests/srp/multipath.conf >&/dev/null
        then
                SKIP_REASON="/etc/multipath.conf already exists"
-               return 1
+               return
        fi
 }
 

diff --git a/tests/zbd/rc b/tests/zbd/rc
index 5dd2779ae48d87caa137a6854aeb45536ac4fbb5..9c1dc5210b1a80570676917d6730deb5c53e65b2 100644 (file)
--- a/tests/zbd/rc
+++ b/tests/zbd/rc
@@ -12,11 +12,9 @@
 #
 
 group_requires() {
-       _have_root || return $?
-       _have_program blkzone || return $?
-       _have_program dd || return $?
-       _have_kernel_option BLK_DEV_ZONED || return $?
-       _have_modules null_blk && _have_module_param null_blk zoned
+       _have_root && _have_program blkzone && _have_program dd &&
+               _have_kernel_option BLK_DEV_ZONED && _have_modules null_blk &&
+               _have_module_param null_blk zoned
 }
 
 group_device_requires() {