On Sun, Apr 17, 2022 at 08:40:21PM +0300, Amir Goldstein wrote:
> For example:
> _supported_fs ^xfs
>
> There is no need to specify "generic" when using a block list
> "all other fs are supported" is implied.
>
> Converted some generic tests that open code this condition without
> emitting a meaningful reason.
>
> More generic test like generic/186,187 could use a block list, but
> were not converted because they emit some meaningful reason:
> _notrun "Can't fragment free space on btrfs."
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
At first, I'd like to take more time to hear discussion from others. I'll
leave personal review point to each patch, welcome more opinions :)
...
This feature is good to me. But I'd strongly recommend that don't *abuse*
it, except we have a definite reason to count a fs out entirely.
The best way to _notrun a case is by detecting a specified feature/behavior
is supported or not. Judge by kernel/package version is worse. Skipping a
fs entirely without proper reason is the worst.
The generic cases trys to open for all known/unknown filesystems at first,
so before a fs maintainer/devel clearly make sure they don't/no way to accept
this test, we'd better to give them chance to have a try, not count them out
from beginning.
Thanks,
Zorro
> common/rc | 30 ++++++++++++++++++++----------
> tests/generic/500 | 3 +--
> tests/generic/631 | 3 +--
> tests/generic/679 | 3 +--
> 4 files changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 17629801..1d37dcbd 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1572,19 +1572,29 @@ _fail()
>
> # tests whether $FSTYP is one of the supported filesystems for a test
> #
> -_supported_fs()
> +_check_supported_fs()
> {
> - local f
> + local res=1
> + local f
>
> - for f
> - do
> - if [ "$f" = "$FSTYP" -o "$f" = "generic" ]
> - then
> - return
> - fi
> - done
> + for f; do
> + # ^FS means black listed fs
> + if [ "$f" = "^$FSTYP" ]; then
> + return 1
> + elif [ "$f" = "generic" ] || [[ "$f" == "^"* ]]; then
> + # ^FS implies "generic ^FS"
> + res=0
> + elif [ "$f" = "$FSTYP" ]; then
> + return 0
> + fi
> + done
> + return $res
> +}
>
> - _notrun "not suitable for this filesystem type: $FSTYP"
> +_supported_fs()
> +{
> + _check_supported_fs $* || \
> + _notrun "not suitable for this filesystem type: $FSTYP"
> }
>
> # check if a FS on a device is mounted
> diff --git a/tests/generic/500 b/tests/generic/500
> index 0be59934..bc84d219 100755
> --- a/tests/generic/500
> +++ b/tests/generic/500
> @@ -35,7 +35,6 @@ _cleanup()
> . ./common/dmthin
>
> # real QA test starts here
> -_supported_fs generic
> _require_scratch_nocheck
> _require_dm_target thin-pool
>
> @@ -43,7 +42,7 @@ _require_dm_target thin-pool
> # and since we've filled the thinp device it'll return EIO, which will make
> # btrfs flip read only, making it fail this test when it just won't work right
> # for us in the first place.
> -test $FSTYP == "btrfs" && _notrun "btrfs doesn't work that way lol"
> +_supported_fs ^btrfs
>
> # Require underlying device support discard
> _scratch_mkfs >>$seqres.full 2>&1
> diff --git a/tests/generic/631 b/tests/generic/631
> index 4996bce7..219f7a05 100755
> --- a/tests/generic/631
> +++ b/tests/generic/631
> @@ -36,10 +36,9 @@ _cleanup()
> . ./common/attr
>
> # real QA test starts here
> -_supported_fs generic
> _require_scratch
> _require_attrs trusted
> -test "$FSTYP" = "overlay" && _notrun "Test does not apply to overlayfs."
> +_supported_fs ^overlay
> _require_extra_fs overlay
>
> _scratch_mkfs >> $seqres.full
> diff --git a/tests/generic/679 b/tests/generic/679
> index 440f0c08..c32d42b9 100755
> --- a/tests/generic/679
> +++ b/tests/generic/679
> @@ -17,7 +17,6 @@ _begin_fstest auto quick prealloc
>
> # real QA test starts here
>
> -_supported_fs generic
> _require_scratch
> _require_xfs_io_command "falloc"
> _require_xfs_io_command "fiemap"
> @@ -26,7 +25,7 @@ _require_xfs_io_command "fiemap"
> #
> # https://lore.kernel.org/linux-btrfs/20220315164011.GF8241@magnolia/
> #
> -[ $FSTYP == "xfs" ] && _notrun "test not valid for xfs"
> +_supported_fs ^xfs
>
> rm -f $seqres.full
>
> --
> 2.35.1
>
