[PATCH i-g-t 2/3] lib/igt_kmod: Stop handling _kunit suffix

[Lucas De Marchi]

On Tue, Nov 05, 2024 at 11:50:05AM +0100, Janusz Krzysztofik wrote:
>Hi Lucas,
>
>On Tuesday, 5 November 2024 06:52:54 CET Lucas De Marchi wrote:
>> It's not used anywhere anymore. Remove it.
>
>I'm not sure.  I added this option based on KUnit documentation.  Has the
>documentation changed since then?  I haven't been watching it.  Have you

I don't see a suffix _kunit for module names mentioned anywhere.
Documentation mentions that the config option usually ends with
_KUNIT_TEST: https://www.kernel.org/doc/html/latest/dev-tools/kunit/start.html#configuring-the-kernel

looking for modules ending in _kunit:
	$ git grep '=.*_kunit\.o'
	drivers/net/ethernet/microchip/vcap/Makefile:obj-$(CONFIG_VCAP_KUNIT_TEST) +=  vcap_model_kunit.o
	kernel/Makefile:obj-$(CONFIG_RESOURCE_KUNIT_TEST) += resource_kunit.o
	lib/Makefile:obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
	lib/Makefile:obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
	lib/Makefile:obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o
	lib/Makefile:obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
	lib/Makefile:obj-$(CONFIG_CHECKSUM_KUNIT) += checksum_kunit.o
	lib/Makefile:obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
	lib/Makefile:obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
	lib/Makefile:obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
	lib/Makefile:obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
	lib/Makefile:obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
	lib/Makefile:obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
	lib/Makefile:obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
	lib/Makefile:obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
	lib/Makefile:obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
	lib/math/Makefile:obj-$(CONFIG_INT_POW_TEST)  += tests/int_pow_kunit.o
	lib/math/tests/Makefile:obj-$(CONFIG_INT_POW_TEST) += int_pow_kunit.o
	rust/Makefile:obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
	sound/core/Makefile:obj-$(CONFIG_SND_CORE_TEST) += sound_kunit.o

vs:

	$ git grep '=.*_test\.o' | wc -l 
	87

I think the behavior is a bit surprising because when passing NULL it
actually runs *all* the testsuites. It's not a shortcut to handle
modules like e.g. usercopy_kunit.o / siphash_kunit.o
that have the testsuite called usercopy / siphash. Instead it's
a way to group the tests for igt reporting purposes. We could very well
just keep the suffix or use a hardcoded "all". Or did I misread the
handling of this arg?

Currently there are no users in igt for modules ending in _kunit and I
don't think we are going to add one any time soon. We can always come
back and revert this if needed.

>checked if core drm doesn't use that suffix?   If it is still documented then
>what if we decide to run kunit tests from other subsytems (dma-buf? iommu?)

Using lib/stackinit_kunit.c as example: we could do either

	igt_kunit("stackinit_kunit", NULL, NULL)
	or
	igt_kunit("stackinit_kunit", "stackinit", NULL)

The difference would only be the name of the igt subtest.

Lucas De Marchi

>and it occurs they still use it?
>
>Thanks,
>Janusz
>
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  lib/igt_kmod.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>> index 4eeab79ec..58edac8fa 100644
>> --- a/lib/igt_kmod.c
>> +++ b/lib/igt_kmod.c
>> @@ -1189,7 +1189,7 @@ static void __igt_kunit(struct igt_ktest *tst,
>>   *	   if NULL then test cases from all test suites provided by the module
>>   *	   are executed as dynamic sub-subtests of one IGT subtest, which name
>>   *	   is derived from the module name by cutting off its optional trailing
>> - *	   _test or _kunit suffix
>> + *	   _test suffix
>>   * @opts: options to load the module
>>   *
>>   * Loads the test module, parses its (k)tap dmesg output, then unloads it
>> @@ -1213,9 +1213,6 @@ void igt_kunit(const char *module_name, const char *suite, const char *opts)
>>  		if (!igt_debug_on(!subtest)) {
>>  			char *suffix = strstr(subtest, "_test");
>>
>> -			if (!suffix)
>> -				suffix = strstr(subtest, "_kunit");
>> -
>>  			if (suffix)
>>  				*suffix = '\0';
>>  		}
>>
>
>
>
>