Re: [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing

Marcos Paulo de Souza
2019-12-18T00:41:55Z

On Wed, 2019-12-18 at 08:30 +0800, Qu Wenruo wrote:
>
> On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote:
> > From: Marcos Paulo de Souza <mpdesouza@suse.com>
> >
> > Move the check of dmsetup to check_dm_target_support, and adapt the
> only
> > two places checking if dmsetup is present in the system. Now we
> skip the
> > test if dmsetup isn't available, instead of marking the test as
> failed.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>
> Looks good overall, just a small nitpick inlined below.
>
> > ---
> > tests/common | 9
> +++++++--
> > tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 -
> > .../017-small-backing-size-thin-provision-device/test.sh | 1 -
> > 3 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/common b/tests/common
> > index f138b17e..dc2d084e 100644
> > --- a/tests/common
> > +++ b/tests/common
> > @@ -322,10 +322,15 @@ check_global_prereq()
> > fi
> > }
> >
> > -# check if the targets passed as arguments are available, and if
> not just skip
> > -# the test
> > +# check if dmsetup and targets passed as arguments are available,
> and skip the
> > +# test if they aren't.
> > check_dm_target_support()
> > {
> > + which dmsetup &> /dev/null
> > + if [ $? -ne 0 ]; then
> > + _not_run "This test requires dmsetup tool.";
> > + fi
>
> What about using existing check_global_prereq()?

check_global_prereq call _fail when the executable is not found in
$PATH, that's why I open coded the implementation and just called
_not_run.

>
> Despite that,
>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>
> Thanks,
> Qu
>
> > +
> > for target in "$@"; do
> > $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1
> > $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target
> > diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> > index 329deaf2..2df88db4 100755
> > --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> > +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> > @@ -4,7 +4,6 @@
> > source "$TEST_TOP/common"
> >
> > check_prereq mkfs.btrfs
> > -check_global_prereq dmsetup
> > check_dm_target_support linear
> >
> > setup_root_helper
> > diff --git a/tests/mkfs-tests/017-small-backing-size-thin-
> provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size-
> thin-provision-device/test.sh
> > index 91851945..83f34ecc 100755
> > --- a/tests/mkfs-tests/017-small-backing-size-thin-provision-
> device/test.sh
> > +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision-
> device/test.sh
> > @@ -6,7 +6,6 @@ source "$TEST_TOP/common"
> >
> > check_prereq mkfs.btrfs
> > check_global_prereq udevadm
> > -check_global_prereq dmsetup
> > check_dm_target_support linear thin
> >
> > setup_root_helper
> >
>