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

[Marcos Paulo de Souza <mpdesouza@suse.de>]

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
> > 
>