Fix unintentional skipping of tests

cd11d001fe86 ("Support skipping tests from test{,_device}()") breaks a
good handful of tests.

For example, block/005 uses _test_dev_is_rotational to check if the
device is rotational and uses the result to size up the fio run. As a
side-effect, _test_dev_is_rotational also sets SKIP_REASON, which (since
commit cd11d001fe86) causes the test to print out a "[not run]" even
through the test actually ran successfully.

Fix this by renaming the existing helpers to _require_foo (e.g. a
_require_test_dev_is_rotational) and add the non-_require variant where
needed.

Fixes: cd11d001fe86 ("Support skipping tests from test{,_device}()")
Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
[Omar: simplify new _test_dev helpers]
Signed-off-by: Omar Sandoval <osandov@fb.com>

diff --git a/check b/check
index 398eca05e3a4ce33c3cdcb0a8e295903f7d52a0b..dc89d13e3e2e4a8235dfb3d3c0672d6c463ac28d 100755 (executable)
--- a/check
+++ b/check
@@ -421,21 +421,8 @@ _call_test() {
 }
 
 _test_dev_is_zoned() {
-       if [[ ! -f "${TEST_DEV_SYSFS}/queue/zoned" ]] ||
-             grep -q none "${TEST_DEV_SYSFS}/queue/zoned"; then
-               SKIP_REASON="${TEST_DEV} is not a zoned block device"
-               return 1
-       fi
-       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
+       [[ -e "${TEST_DEV_SYSFS}/queue/zoned" &&
+          $(cat "${TEST_DEV_SYSFS}/queue/zoned") != none ]]
 }
 
 _run_test() {
@@ -497,7 +484,7 @@ _run_test() {
                        local unset_skip_reason=0
                        if [[ ! -v SKIP_REASON ]]; then
                                unset_skip_reason=1
-                               if (( !CAN_BE_ZONED )) && ! _test_dev_is_not_zoned; then
+                               if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then
                                        SKIP_REASON="${TEST_DEV} is a zoned block device"
                                elif declare -fF device_requires >/dev/null; then
                                        device_requires

diff --git a/common/iopoll b/common/iopoll
index 80a5f99b08ca1c813b74623e4d5d08ac590aaa48..dfdd2cf6f08f0b54c19fb612942e7596f161d714 100644 (file)
--- a/common/iopoll
+++ b/common/iopoll
@@ -17,7 +17,7 @@ _have_fio_with_poll() {
        return 0
 }
 
-_test_dev_supports_io_poll() {
+_require_test_dev_supports_io_poll() {
        local old_io_poll
        if ! old_io_poll="$(cat "${TEST_DEV_SYSFS}/queue/io_poll" 2>/dev/null)"; then
                SKIP_REASON="kernel does not support polling"
@@ -30,7 +30,7 @@ _test_dev_supports_io_poll() {
        return 0
 }
 
-_test_dev_supports_io_poll_delay() {
+_require_test_dev_supports_io_poll_delay() {
        local old_io_poll_delay
        if ! old_io_poll_delay="$(cat "${TEST_DEV_SYSFS}/queue/io_poll_delay" 2>/dev/null)"; then
                SKIP_REASON="kernel does not support hybrid polling"

diff --git a/common/rc b/common/rc
index 1893dda2b2f77828fb0b970204df52f1b9372372..7f02103dc7863e84b832a111f7b7ea799e717ded 100644 (file)
--- a/common/rc
+++ b/common/rc
@@ -181,17 +181,25 @@ _have_tracepoint() {
        return 0
 }
 
-_test_dev_can_discard() {
-       if [[ $(cat "${TEST_DEV_SYSFS}/queue/discard_max_bytes") -eq 0 ]]; then
-               SKIP_REASON="$TEST_DEV does not support discard"
+_test_dev_is_rotational() {
+       [[ $(cat "${TEST_DEV_SYSFS}/queue/rotational") -ne 0 ]]
+}
+
+_require_test_dev_is_rotational() {
+       if ! _test_dev_is_rotational; then
+               SKIP_REASON="$TEST_DEV is not rotational"
                return 1
        fi
        return 0
 }
 
-_test_dev_is_rotational() {
-       if [[ $(cat "${TEST_DEV_SYSFS}/queue/rotational") -eq 0 ]]; then
-               SKIP_REASON="$TEST_DEV is not rotational"
+_test_dev_can_discard() {
+       [[ $(cat "${TEST_DEV_SYSFS}/queue/discard_max_bytes") -gt 0 ]]
+}
+
+_require_test_dev_can_discard() {
+       if ! _test_dev_can_discard; then
+               SKIP_REASON="$TEST_DEV does not support discard"
                return 1
        fi
        return 0
@@ -214,7 +222,7 @@ _test_dev_queue_set() {
        echo "$2" >"${TEST_DEV_SYSFS}/queue/$1"
 }
 
-_test_dev_is_pci() {
+_require_test_dev_is_pci() {
        if ! readlink -f "$TEST_DEV_SYSFS/device" | grep -q pci; then
                # nvme needs some special casing
                if readlink -f "$TEST_DEV_SYSFS/device" | grep -q nvme; then
@@ -247,7 +255,7 @@ _get_pci_parent_from_blkdev() {
                tail -2 | head -1
 }
 
-_test_dev_in_hotplug_slot() {
+_require_test_dev_in_hotplug_slot() {
        local parent
        parent="$(_get_pci_parent_from_blkdev)"
 
@@ -261,7 +269,11 @@ _test_dev_in_hotplug_slot() {
 }
 
 _test_dev_is_partition() {
-       if [[ -z ${TEST_DEV_PART_SYSFS} ]]; then
+       [[ -n ${TEST_DEV_PART_SYSFS} ]]
+}
+
+_require_test_dev_is_partition() {
+       if ! _test_dev_is_partition; then
                SKIP_REASON="${TEST_DEV} is not a partition device"
                return 1
        fi

diff --git a/new b/new
index 31973ed1add2dca8322fe9abf1dea24c5fea79e0..73f0faa8fa969f41769c256dd3b94677ee60bce0 100755 (executable)
--- a/new
+++ b/new
@@ -85,10 +85,10 @@ group_requires() {
 #
 # 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() sets \$SKIP_REASON, all
-# tests in this group will be skipped on that device.
+# _require_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
+#      _require_test_dev_is_foo && _require_test_dev_supports_bar
 # }
 
 # TODO: define any helpers that are specific to this group.
@@ -171,10 +171,10 @@ 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() sets \$SKIP_REASON, the test will
-# be skipped on that device.
+# _require_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
+#      _require_test_dev_is_foo && _require_test_dev_supports_bar
 # }
 
 # TODO: define the test. The output of this function (stdout and stderr) will

diff --git a/tests/block/003 b/tests/block/003
index 6696d371d7e5d6b71fd0ac3fb26c329eb615cca2..2af9b89ec3e5bce6cd38ab5c0d4f0dde80d9b269 100755 (executable)
--- a/tests/block/003
+++ b/tests/block/003
@@ -14,7 +14,7 @@ requires() {
 }
 
 device_requires() {
-       _test_dev_can_discard
+       _require_test_dev_can_discard
 }
 
 test_device() {

diff --git a/tests/block/007 b/tests/block/007
index f03935084ce6a6e2929cb545790b5a2919e685d3..a8fe9985ed004e296e3c7ea43fd7d14b079cc171 100755 (executable)
--- a/tests/block/007
+++ b/tests/block/007
@@ -15,7 +15,8 @@ requires() {
 }
 
 device_requires() {
-       _test_dev_supports_io_poll && _test_dev_supports_io_poll_delay
+       _require_test_dev_supports_io_poll &&
+               _require_test_dev_supports_io_poll_delay
 }
 
 run_fio_job() {

diff --git a/tests/block/011 b/tests/block/011
index c3432a63e274fb85ce5c2cc1492026a7593f834d..4f331b4a75220edb5092340a3f0b50ead67118f3 100755 (executable)
--- a/tests/block/011
+++ b/tests/block/011
@@ -15,7 +15,7 @@ requires() {
 }
 
 device_requires() {
-       _test_dev_is_pci
+       _require_test_dev_is_pci
 }
 
 test_device() {

diff --git a/tests/block/019 b/tests/block/019
index 7cd26bd512bc1b703c2177352c4f0b15982b330d..113a3d6e89866b0c47a0c47a914fef1ec6c4ec83 100755 (executable)
--- a/tests/block/019
+++ b/tests/block/019
@@ -14,7 +14,7 @@ requires() {
 }
 
 device_requires() {
-       _test_dev_is_pci && _test_dev_in_hotplug_slot
+       _require_test_dev_is_pci && _require_test_dev_in_hotplug_slot
 }
 
 test_device() {

diff --git a/tests/nvme/032 b/tests/nvme/032
index a91a473ac5df54ad7fb6d1678ea4cb71ff0f04e5..ce45657951a1aec880541b0337933b56d2d2ab43 100755 (executable)
--- a/tests/nvme/032
+++ b/tests/nvme/032
@@ -19,7 +19,7 @@ requires() {
 }
 
 device_requires() {
-       _test_dev_is_nvme
+       _require_test_dev_is_nvme
 }
 
 test_device() {

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 40f0413d32d2fe2b3a3c2de11d4c1049369cc2ac..6ffa971b4308c7467f243d77ca552bd1785a0210 100644 (file)
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -11,12 +11,12 @@ group_requires() {
 }
 
 group_device_requires() {
-       _test_dev_is_nvme
+       _require_test_dev_is_nvme
 }
 
 NVMET_CFS="/sys/kernel/config/nvmet/"
 
-_test_dev_is_nvme() {
+_require_test_dev_is_nvme() {
        if ! readlink -f "$TEST_DEV_SYSFS/device" | grep -q nvme; then
                SKIP_REASON="$TEST_DEV is not a NVMe device"
                return 1

diff --git a/tests/scsi/006 b/tests/scsi/006
index f220f61e3c1e0be309dadead612c62879922498a..05ed6520d6004476349e13effba3dc273323ad99 100755 (executable)
--- a/tests/scsi/006
+++ b/tests/scsi/006
@@ -12,7 +12,7 @@ DESCRIPTION="toggle SCSI cache type"
 QUICK=1
 
 device_requires() {
-       _test_dev_is_scsi_disk
+       _require_test_dev_is_scsi_disk
 }
 
 test_device() {

diff --git a/tests/scsi/rc b/tests/scsi/rc
index 2a192fd0f9690d54d078b206917ba47cd8e78b80..1477cecc5593f4c673b730eb38a838734d6c263a 100644 (file)
--- a/tests/scsi/rc
+++ b/tests/scsi/rc
@@ -11,14 +11,14 @@ group_requires() {
 }
 
 group_device_requires() {
-       _test_dev_is_scsi
+       _require_test_dev_is_scsi
 }
 
 _have_scsi_generic() {
        _have_modules sg
 }
 
-_test_dev_is_scsi() {
+_require_test_dev_is_scsi() {
        if [[ ! -d ${TEST_DEV_SYSFS}/device/scsi_device ]]; then
                SKIP_REASON="$TEST_DEV is not a SCSI device"
                return 1
@@ -26,7 +26,7 @@ _test_dev_is_scsi() {
        return 0
 }
 
-_test_dev_is_scsi_disk() {
+_require_test_dev_is_scsi_disk() {
        if [[ ! -d ${TEST_DEV_SYSFS}/device/scsi_disk ]]; then
                SKIP_REASON="$TEST_DEV is not a SCSI disk"
                return 1

diff --git a/tests/zbd/007 b/tests/zbd/007
index b4dcbd89f17958beea34d919cc7a31545bf35ab0..2376b3aedaa05749e30144f1e2e6f65a273138af 100755 (executable)
--- a/tests/zbd/007
+++ b/tests/zbd/007
@@ -18,7 +18,7 @@ requires() {
 }
 
 device_requires() {
-       _test_dev_is_logical
+       _require_test_dev_is_logical
 }
 
 # Select test target zones. Pick up the first sequential required zones. If

diff --git a/tests/zbd/rc b/tests/zbd/rc
index 9c1dc5210b1a80570676917d6730deb5c53e65b2..312469375ba4586fe8c5b1d373623a865188dc0a 100644 (file)
--- a/tests/zbd/rc
+++ b/tests/zbd/rc
@@ -18,7 +18,10 @@ group_requires() {
 }
 
 group_device_requires() {
-       _test_dev_is_zoned
+       if ! _test_dev_is_zoned; then
+               SKIP_REASON="${TEST_DEV} is not a zoned block device"
+               return
+       fi
 }
 
 _fallback_null_blk_zoned() {
@@ -253,14 +256,10 @@ _find_two_contiguous_seq_zones() {
 }
 
 _test_dev_is_dm() {
-       if [[ ! -r "${TEST_DEV_SYSFS}/dm/name" ]]; then
-               SKIP_REASON="$TEST_DEV is not device-mapper"
-               return 1
-       fi
-       return 0
+       [[ -r "${TEST_DEV_SYSFS}/dm/name" ]]
 }
 
-_test_dev_is_logical() {
+_require_test_dev_is_logical() {
        if ! _test_dev_is_partition && ! _test_dev_is_dm; then
                SKIP_REASON="$TEST_DEV is not a logical device"
                return 1
@@ -274,11 +273,9 @@ _test_dev_has_dm_map() {
 
        dm_name=$(<"${TEST_DEV_SYSFS}/dm/name")
        if ! dmsetup status "${dm_name}" | grep -qe "${target_type}"; then
-               SKIP_REASON="$TEST_DEV does not have ${target_type} map"
                return 1
        fi
        if dmsetup status "${dm_name}" | grep -v "${target_type}"; then
-               SKIP_REASON="$TEST_DEV has map other than ${target_type}"
                return 1
        fi
        return 0