diff mbox series

btrfs-progs: tests: not_run for global_prereq fail

Message ID 149265ca8a94688008c1cda96c95cf83ac55950c.1690024017.git.anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: tests: not_run for global_prereq fail | expand

Commit Message

Anand Jain July 25, 2023, 9:43 a.m. UTC
Prerequisite checks using global_prereq() aren't global, so why
fail and stop further test cases? Instead, just don't run them.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/common | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Sterba July 27, 2023, 4:51 p.m. UTC | #1
On Tue, Jul 25, 2023 at 05:43:54PM +0800, Anand Jain wrote:
> Prerequisite checks using global_prereq() aren't global, so why
> fail and stop further test cases? Instead, just don't run them.

I'd rather let the whole testsuite run, a missing global prerequisity
means the environment is not set up properly, so this fail as intended.
If you look what kind of utilities are checked in that way it's eg. dd,
mke2fs, chattr, losetup. All are supposed to be available on a common
system so it's not expected to fail.

If there's some less common utility and a test which could be optional
then this should be a special case, which would require a new helper.
Anand Jain July 28, 2023, 3:39 a.m. UTC | #2
On 28/07/2023 00:51, David Sterba wrote:
> On Tue, Jul 25, 2023 at 05:43:54PM +0800, Anand Jain wrote:
>> Prerequisite checks using global_prereq() aren't global, so why
>> fail and stop further test cases? Instead, just don't run them.
> 
> I'd rather let the whole testsuite run, a missing global prerequisity
> means the environment is not set up properly, so this fail as intended.
> If you look what kind of utilities are checked in that way it's eg. dd,
> mke2fs, chattr, losetup. All are supposed to be available on a common
> system so it's not expected to fail.
> 
> If there's some less common utility and a test which could be optional
> then this should be a special case, which would require a new helper.

I would prefer using check_global_prereq() to verify all the mandatory
prerequisites at once.

If each test case checks for a different binary using
check_global_prereq() and it fails during that test case, we have to
restart, which becomes messy.

The support for mkreiserfs on OL was removed, causing the test case
to abruptly stop in the middle.
Anand Jain July 28, 2023, 3:51 a.m. UTC | #3
On 28/07/2023 11:39, Anand Jain wrote:
> On 28/07/2023 00:51, David Sterba wrote:
>> On Tue, Jul 25, 2023 at 05:43:54PM +0800, Anand Jain wrote:
>>> Prerequisite checks using global_prereq() aren't global, so why
>>> fail and stop further test cases? Instead, just don't run them.
>>
>> I'd rather let the whole testsuite run, a missing global prerequisity
>> means the environment is not set up properly, so this fail as intended.
>> If you look what kind of utilities are checked in that way it's eg. dd,
>> mke2fs, chattr, losetup. All are supposed to be available on a common
>> system so it's not expected to fail.
>>
>> If there's some less common utility and a test which could be optional
>> then this should be a special case, which would require a new helper.
> 
> I would prefer using check_global_prereq() to verify all the mandatory
> prerequisites at once.
> 
> If each test case checks for a different binary using
> check_global_prereq() and it fails during that test case, we have to
> restart, which becomes messy.

  Commit 'btrfs-progs: tests: add script to check global prerequisities'
  addressed the messy situation mentioned above.

> The support for mkreiserfs on OL was removed, causing the test case
> to abruptly stop in the middle.

  Is there a way to skip the test cases?
David Sterba Aug. 11, 2023, 5:57 p.m. UTC | #4
On Fri, Jul 28, 2023 at 11:51:18AM +0800, Anand Jain wrote:
> On 28/07/2023 11:39, Anand Jain wrote:
> > On 28/07/2023 00:51, David Sterba wrote:
> >> On Tue, Jul 25, 2023 at 05:43:54PM +0800, Anand Jain wrote:
> >>> Prerequisite checks using global_prereq() aren't global, so why
> >>> fail and stop further test cases? Instead, just don't run them.
> >>
> >> I'd rather let the whole testsuite run, a missing global prerequisity
> >> means the environment is not set up properly, so this fail as intended.
> >> If you look what kind of utilities are checked in that way it's eg. dd,
> >> mke2fs, chattr, losetup. All are supposed to be available on a common
> >> system so it's not expected to fail.
> >>
> >> If there's some less common utility and a test which could be optional
> >> then this should be a special case, which would require a new helper.
> > 
> > I would prefer using check_global_prereq() to verify all the mandatory
> > prerequisites at once.
> > 
> > If each test case checks for a different binary using
> > check_global_prereq() and it fails during that test case, we have to
> > restart, which becomes messy.
> 
>   Commit 'btrfs-progs: tests: add script to check global prerequisities'
>   addressed the messy situation mentioned above.
> 
> > The support for mkreiserfs on OL was removed, causing the test case
> > to abruptly stop in the middle.
> 
>   Is there a way to skip the test cases?

No, though there's the newly added TEST_FROM=test that skips all tests
preceding the one. Skipping a given class of tests would make sense,
though the easiest thing would be to skip by the directory name, so e.g.
TEST_SKIP=*resiserfs* that would be a followup filter after TEST.

Aloso we can mark the reiserfs tests as obsolete as the package are
being dropped from distros (openSUSE Tumbleweed does not ship it
anymore) and kernel support for reiserfs is going to be removed next
year. I don't want to add some generic infrastructure just for that so
I'll think about something simple like silently skipping the tests by
default.
diff mbox series

Patch

diff --git a/tests/common b/tests/common
index 602a4122f8bd..4bde06ad9d1d 100644
--- a/tests/common
+++ b/tests/common
@@ -395,7 +395,7 @@  check_global_prereq()
 {
 	type -p "$1" &> /dev/null
 	if [ $? -ne 0 ]; then
-		_fail "Failed system wide prerequisities: $1";
+		_not_run "Failed system wide prerequisities: $1";
 	fi
 }