[LTP] Split some fanotify tests

[LTP] Split some fanotify tests

[Petr Vorel]

Hi Amir,

FYI we were thinking with Martin about splitting some of the fanotify tests.
Why?

Some of the fanotify tests are hard to review (e.g. fanotify{10,13,14}.c,),
because the have a lot of results.

Also other problem is that some enterprise kernels does not backport minor fixes
and the usual approach is to whitelist the issue based on exit code. But this
cannot be done on the test which has many results. We could workaround it with
tst_kvercmp2(), but that's kind of a last result.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] Split some fanotify tests

[Amir Goldstein]

On Fri, Jan 24, 2025 at 11:02 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> FYI we were thinking with Martin about splitting some of the fanotify tests.
> Why?
>
> Some of the fanotify tests are hard to review (e.g. fanotify{10,13,14}.c,),
> because the have a lot of results.

I assume you are referring to test cases that are declared as regression
tests for a fix commit.
Do you intend to split every such test case into its own test?

I agree that the FAN_DELETE_SELF test case is a good candidate
for splitting, if not for anything else, because the ugliness of requiring this
test case to be the last test case.

>
> Also other problem is that some enterprise kernels does not backport minor fixes
> and the usual approach is to whitelist the issue based on exit code. But this
> cannot be done on the test which has many results. We could workaround it with
> tst_kvercmp2(), but that's kind of a last result.

As a general rule, not testing a know bug because it is not backported
sounds bad to me, although I do agree that there could be exceptions, like:

        if (tc->ignored_flags & FAN_EVENT_ON_CHILD && tst_kvercmp(5,
9, 0) < 0) {
                tst_res(TCONF, "ignored mask in combination with flag
FAN_EVENT_ON_CHILD"
                                " has undefined behavior on kernel < 5.9");

If the test runner whitelists a test it's up to them.
It would be nice if LTP could add the ability to whitelist a test case/variant
or even a specific filesystem for a specific all_filesystems test, but
it's up to
you guys to decide if it is worth it.

The reason I tend to add test cases is to save code duplication between tests,
because there is a lot of boilerplate code for event verification, which often
has bugs and needs fixing in multiple tests.
There is another solution for code duplication - to write generic fanotify event
verification helpers.

It's up to you to decide if there is room for some helpers for fanotify event
verification or if you want to fork the tests with code duplication.
WDYT?

Thanks,
Amir.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] Split some fanotify tests

[Cyril Hrubis]

Hi!
> The reason I tend to add test cases is to save code duplication between tests,
> because there is a lot of boilerplate code for event verification, which often
> has bugs and needs fixing in multiple tests.
> There is another solution for code duplication - to write generic fanotify event
> verification helpers.
> 
> It's up to you to decide if there is room for some helpers for fanotify event
> verification or if you want to fork the tests with code duplication.
> WDYT?

Sharing code is almost never a mistake. We usually put static inline
helpers into a common.h header and include that in tests.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] Split some fanotify tests

[Martin Doucha]

Hi!

On 24. 01. 25 11:21, Amir Goldstein wrote:
> On Fri, Jan 24, 2025 at 11:02 AM Petr Vorel <pvorel@suse.cz> wrote:
>>
>> Hi Amir,
>>
>> FYI we were thinking with Martin about splitting some of the fanotify tests.
>> Why?
>>
>> Some of the fanotify tests are hard to review (e.g. fanotify{10,13,14}.c,),
>> because the have a lot of results.
> 
> I assume you are referring to test cases that are declared as regression
> tests for a fix commit.
> Do you intend to split every such test case into its own test?
> 
> I agree that the FAN_DELETE_SELF test case is a good candidate
> for splitting, if not for anything else, because the ugliness of requiring this
> test case to be the last test case.

Yes, that would be a good candidate for its own test. But in general, 
one test program should have at most 5-10 subtests so that when you need 
to whitelist it as a whole, you don't lose too much test coverage due to 
one failing subtest. There are of course exceptions, closely related 
subtests (e.g. a single set of init and mark flags tested on different 
types of filesystem objects) can be in the same test program even if 
there's way more than 10 of them.

> If the test runner whitelists a test it's up to them.
> It would be nice if LTP could add the ability to whitelist a test case/variant
> or even a specific filesystem for a specific all_filesystems test, but
> it's up to
> you guys to decide if it is worth it.

Exiting a subtest with TCONF is the only way of whitelisting issues 
inside LTP at the moment. The main concern in this thread is the 
difficulty and trade-offs of whitelisting at the test runner level.

> The reason I tend to add test cases is to save code duplication between tests,
> because there is a lot of boilerplate code for event verification, which often
> has bugs and needs fixing in multiple tests.
> There is another solution for code duplication - to write generic fanotify event
> verification helpers.
> 
> It's up to you to decide if there is room for some helpers for fanotify event
> verification or if you want to fork the tests with code duplication.
> WDYT?

Shared file with fanotify verification helper code would be great. 
Thanks in advance.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp