RFC tests reorg

[evelikov]

While going through the earlier test PRs (#258 and #375), a few things of varying annoyance stood out. Each of these is mostly independent, so one can thumbs up/down individually or as a whole:

• inconsistent cleanups in error path - mostly addressed by latter PR, finish and merge
• custom checks vs assert_return() - ditto
• test names (as per test-foo --list) starting with test_ (61) vs not (55) - remove the prefix
• arbitrary/non-descriptive test names (alias_1, from_alias, etc) - use util_alias, libkmod_new_from_alias, aka $top-level-dir_testname or $tool_testname
• ^^ for filenames (test-blacklist.c, test-dependencies.c, test-loaded.c, etc) - use test-libkmod-foobar.c
• multiple-files-per-tool (test-modprobe, test-weakdep, etc) - merge into one
• one-test-per-file (mostly likmod API) - merge, unless it causes problems
• dozen+ test binaries, non-unique testnames (practically identical/duplicated test_remove), manually running tests (tab completion doesn't work in sh -c 'source ....) - fold into single binary

@lucasdemarchi @stoeckmann do let me know what you think. All of these are a net positive IMHO, with the* minus being the temporary churn. If you see any item as a potential deterrent from adding more test, please let me know.

[evelikov]

@lucasdemarchi any preferences on the above?

Would prefer to have things somewhat well-defined and consistent before we expand on the tests - hint the recently opened softdep bug and the other one from 2024.

[lucasdemarchi]

• inconsistent cleanups in error path - mostly addressed by latter PR, finish and merge
• custom checks vs assert_return() - ditto

Both replied in #375

• test names (as per test-foo --list) starting with test_ (61) vs not (55) - remove the prefix

Fine, but you will need some changes in the DEFINE_TEST() macro. Example:

static int test_strbuf_pushchar(void)

This is a test for the strbuf_pushchar() internal API. So probably:

-#define DEFINE_TEST(_name, ...) DEFINE_TEST_WITH_FUNC(_name, _name, VA_ARGS)
+#define DEFINE_TEST(_name, ...) DEFINE_TEST_WITH_FUNC(name, test ## _name, VA_ARGS)

and pass the name without the test_ prefix?

• arbitrary/non-descriptive test names (alias_1, from_alias, etc) - use util_alias, libkmod_new_from_alias, aka $top-level-dir_testname or $tool_testname

ack

• ^^ for filenames (test-blacklist.c, test-dependencies.c, test-loaded.c, etc) - use test-libkmod-foobar.c

ack

• multiple-files-per-tool (test-modprobe, test-weakdep, etc) - merge into one

not sure I follow.... test-modprobe is testing the modprobe tool. test-weakdep is testing a libkmod API. Sometimes it's easy to just use the tool to test the API, but that's not the case here.

• one-test-per-file (mostly likmod API) - merge, unless it causes problems
• dozen+ test binaries, non-unique testnames (practically identical/duplicated test_remove), manually running tests (tab completion doesn't work in sh -c 'source ....) - fold into single binary

we'd then need to improve the result reporting making sure the subtests are reported individually and accounted for.

[evelikov]

Fine, but you will need some changes in the DEFINE_TEST() macro. Example:

static int test_strbuf_pushchar(void)

This is a test for the strbuf_pushchar() internal API. So probably:

-#define DEFINE_TEST(_name, ...) DEFINE_TEST_WITH_FUNC(_name, _name, VA_ARGS) +#define DEFINE_TEST(_name, ...) DEFINE_TEST_WITH_FUNC(name, test ## _name, VA_ARGS)

and pass the name without the test_ prefix?

Yes, symbol collision/duplicates need to be resolved. Thanks for the fix/suggestion.

not sure I follow.... test-modprobe is testing the modprobe tool. test-weakdep is testing a libkmod API. Sometimes it's easy to just use the tool to test the API, but that's not the case here.

test-weakdep tests both the libkmod API and the modprobe -C parsing.

we'd then need to improve the result reporting making sure the subtests are reported individually and accounted for.

Unless I've misunderstood what you meant, it should already be fixed with eb575e4 ("testsuite: run all tests, don't exit on first failure").

[evelikov]

The above PR handles most of the remaining topics. With the outstanding ones being - test names aka:

• drop the test_ prefix from test-foo --list
• arbitrary/non-descriptive test names

I'm inclined to drop the last point about merging all the test binaries into one... The ambiguity and tab-completion "need" no longer exists with the grouping in place.