Re: [PATCH blktests v3] Fix unintentional skipping of tests

[Omar Sandoval <osandov@osandov.com>]

On Wed, Apr 22, 2020 at 09:44:36AM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> 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}()")
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>

Thanks! I'll apply this assuming it looks good after a full test run. A
couple of comments below, but I fixed those up.

> ---
> 
> Changes since v2
> ~~~~~~~~~~~~~~~~
> * Fix missing _test_dev -> _require_test_dev in block/003 (Shinichiro)
> * Revert change in block/004 (Shinichiro)
> 
>  check           | 10 ++++------
>  common/iopoll   |  4 ++--
>  common/rc       | 35 ++++++++++++++++++++++++++++-------
>  new             | 12 ++++++------
>  tests/block/003 |  2 +-
>  tests/block/007 |  3 ++-
>  tests/block/011 |  2 +-
>  tests/block/019 |  2 +-
>  tests/nvme/032  |  2 +-
>  tests/nvme/rc   |  4 ++--
>  tests/scsi/006  |  2 +-
>  tests/scsi/rc   |  6 +++---
>  tests/zbd/007   |  2 +-
>  tests/zbd/rc    | 11 +++++++++--
>  14 files changed, 62 insertions(+), 35 deletions(-)
> 
> diff --git a/check b/check
> index 398eca05e3a4..84ec086c408b 100755
> --- a/check
> +++ b/check
> @@ -423,18 +423,16 @@ _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
>  }

This can be simplified to:

_test_dev_is_zoned() {
	[[ -e "${TEST_DEV_SYSFS}/queue/zoned" &&
	    $(cat "${TEST_DEV_SYSFS}/queue/zoned") != none ]]
}

A few of the other _test_dev helpers can be cleaned up in the same way.