Message ID | 20220921102655.31156-1-pvorel@suse.cz |
---|---|
State | Superseded |
Headers | show |
Series | [RFC,1/1] tst_test.sh: Fix filesystem support detection | expand |
On Wed, Sep 21, 2022 at 6:27 PM Petr Vorel <pvorel@suse.cz> wrote: > Filesystem detection of locally used filesystem was broken on tests > which did not use TST_ALL_FILESYSTEMS as it 1) expected used filesystem > is $TST_FS_TYPE 2) this variable was not yet set. > > Also this check makes sense only if test defines TST_SKIP_FILESYSTEMS > (to align with the condition in do_test_setup() in C API). > > Move filesystem check after (optional) cd "$TST_TMPDIR" (TMPDIR can have > different filesystem). > > Not printing extra TINFO "$_tst_fs is supported by the test" (which is > printed in C API) when test is supported, because there is already > similar TINFO from testcases/lib/tst_supported_fs.c: > tst_supported_fs.c:104: TINFO: btrfs is not skipped > > Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") > > Reported-by: Martin Doucha <mdoucha@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > First, sorry for introducing a regression. > > I don't like df and tail dependency. Also df output is stable (even on > busybox), but I'd prefer to use code from do_test_setup() in > lib/tst_test.c: > > long fs_type = tst_fs_type("."); > const char *fs_name = tst_fs_type_name(fs_type); > > Instead adding yet another binary, I wonder if we could add extra getopt > parameter to ask for current filesystem. > > i.e. to replace: > -tst_supported_fs -s skip_list fs_type > +tst_supported_fs -s skip_list -d DIR > +1, and Martin already sent out the achievement patch, you can rebase this patch on that. And I'd like to vote for merging your both patches for adding to the new release.
Hi Li, all, ... > > Instead adding yet another binary, I wonder if we could add extra getopt > > parameter to ask for current filesystem. > > i.e. to replace: > > -tst_supported_fs -s skip_list fs_type > > +tst_supported_fs -s skip_list -d DIR > +1, and Martin already sent out the achievement patch, > you can rebase this patch on that. Yep, I'm planning to do. > And I'd like to vote for merging your both patches for > adding to the new release. +1
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 229317713..1691846ae 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -703,12 +703,6 @@ tst_run() [ "$TST_FORMAT_DEVICE" = 1 -o "$TST_ALL_FILESYSTEMS" = 1 ] && TST_NEEDS_DEVICE=1 [ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1 - if [ "$TST_ALL_FILESYSTEMS" != 1 ]; then - if ! tst_supported_fs -s "$TST_SKIP_FILESYSTEMS" $TST_FS_TYPE > /dev/null; then - tst_brk TCONF "$TST_FS_TYPE is skipped by the test" - fi - fi - if [ "$TST_NEEDS_DEVICE" = 1 ]; then TST_DEVICE=$(tst_device acquire) @@ -736,6 +730,15 @@ tst_run() cd "$TST_TMPDIR" fi + if [ "$TST_ALL_FILESYSTEMS" != 1 -a "$TST_SKIP_FILESYSTEMS" ]; then + tst_require_cmds df tail + _tst_fs="$(df -T . | tail -1 | awk '{print $2}')" + [ "$_tst_fs" ] || tst_brk TBROK "failed to detect filesystem for $PWD" + if ! tst_supported_fs -s "$TST_SKIP_FILESYSTEMS" $_tst_fs > /dev/null; then + tst_brk TCONF "$_tst_fs is not supported by the test" + fi + fi + [ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints TST_MNTPOINT="${TST_MNTPOINT:-$PWD/mntpoint}"
Filesystem detection of locally used filesystem was broken on tests which did not use TST_ALL_FILESYSTEMS as it 1) expected used filesystem is $TST_FS_TYPE 2) this variable was not yet set. Also this check makes sense only if test defines TST_SKIP_FILESYSTEMS (to align with the condition in do_test_setup() in C API). Move filesystem check after (optional) cd "$TST_TMPDIR" (TMPDIR can have different filesystem). Not printing extra TINFO "$_tst_fs is supported by the test" (which is printed in C API) when test is supported, because there is already similar TINFO from testcases/lib/tst_supported_fs.c: tst_supported_fs.c:104: TINFO: btrfs is not skipped Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") Reported-by: Martin Doucha <mdoucha@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- First, sorry for introducing a regression. I don't like df and tail dependency. Also df output is stable (even on busybox), but I'd prefer to use code from do_test_setup() in lib/tst_test.c: long fs_type = tst_fs_type("."); const char *fs_name = tst_fs_type_name(fs_type); Instead adding yet another binary, I wonder if we could add extra getopt parameter to ask for current filesystem. i.e. to replace: -tst_supported_fs -s skip_list fs_type +tst_supported_fs -s skip_list -d DIR Because 'tst_supported_fs -s skip_list fs_type' mode is used only in tst_test.sh. Whole help would be: $ ./testcases/lib/tst_supported_fs -h * all filesystems tst_supported_fs [-s skip_list] print the list of supported filesystems if fs_type is supported and not in skip_list (optional), print list of supported filesystems and return 0 if fs_type isn't supported or in skip_list, return 1 * single filesystem tst_supported_fs fs_type if fs_type is supported, return 0 otherwise return 1 -tst_supported_fs -s skip_list fs_type +tst_supported_fs -s skip_list -d DIR if fs_type is in skip_list, return 1 otherwise return 0 fs_type - a specified filesystem type skip_list - filesystems to skip, delimiter: ',' testcases/lib/tst_test.sh | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)