[PATCH i-g-t v4 8/8] lib/kunit: Perform test suite filtering on the kernel side

[Kamil Konieczny]

Hi Janusz,
On 2024-02-06 at 11:00:32 +0100, Janusz Krzysztofik wrote:
> When getting a list of test cases provided by a KUnit test module, we now
> set KUnit filters in a way that results in all test cases from all test
> suites of that module being reported as skipped, then we delete test cases
> that don't belong to a requested test suite from the list.  Moreover,
> before executing test cases from that list, we modify KUnit filters in a
> way that all test cases from all test suites of the test module are
> executed, but we report only those from the requested test suite.  Since
> KUnit base module has a capability of filtering test cases by test suite
> name, which we don't use, our approach to filtering is not optimal.
> 
> Limit kernel side processing to test cases that belong to the requested
> test suite by setting KUnit filter_glob parameter to that test suite name,
> and drop user side test suite filtering.  With user side post-processing
> of test case lists limited to SKIP verification, defer cleanup of memory
> occupied by the list entries to a first igt_fixture section that follows.
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> ---
>  lib/igt_kmod.c | 34 +++++++++-------------------------
>  1 file changed, 9 insertions(+), 25 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 506093c954..9b92696f1f 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -1157,7 +1157,6 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  			    const char *suite,
>  			    const char *opts)
>  {
> -	char *suite_name = NULL, *case_name = NULL;
>  	struct igt_ktap_result *r, *rn;
>  	struct igt_ktap_results *ktap;
>  	unsigned long taints;
> @@ -1179,7 +1178,7 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  	 * perfectly -- seems to be more safe than extracting a test case list
>  	 * of unknown length from /dev/kmsg.
>  	 */
> -	if (igt_debug_on(!kunit_set_filtering(NULL, "module=none", "skip")))
> +	if (igt_debug_on(!kunit_set_filtering(suite, "module=none", "skip")))
>  		return false;
>  
>  	igt_skip_on(modprobe(tst->kmod, opts));
> @@ -1194,30 +1193,13 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  
>  	igt_ktap_free(ktap);
>  
> -	if (!err)
> -		igt_list_for_each_entry_safe(r, rn, tests, link) {
> -			if (igt_debug_on(r->code != IGT_EXIT_SKIP)) {
> -				err = r->code ?: -EPROTO;
> -				break;
> -			}
> -
> -			if (!suite)
> -				continue;
> -
> -			if (strcmp(r->suite_name, suite))
> -				kunit_result_free(&r, &case_name, &suite_name);
> -		}
> -
> -	if (err) {
> -		kunit_results_free(tests, &case_name, &suite_name);
> -	} else {
> -		free(suite_name);
> -		free(case_name);
> -	}
> -
>  	igt_skip_on_f(err,
>  		      "KTAP parser failed while getting a list of test cases\n");
>  
> +	igt_list_for_each_entry_safe(r, rn, tests, link)
> +		igt_require_f(r->code == IGT_EXIT_SKIP,
> +			      "Unexpected non-SKIP result while listing test cases\n");
> +
>  	igt_skip_on(kmod_module_remove_module(tst->kmod, KMOD_REMOVE_FORCE));
>  
>  	return true;
> @@ -1225,6 +1207,7 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  
>  static void __igt_kunit(struct igt_ktest *tst,
>  			const char *subtest,
> +			const char *suite,
>  			const char *opts,
>  			struct igt_list_head *tests)
>  {
> @@ -1249,7 +1232,8 @@ static void __igt_kunit(struct igt_ktest *tst,
>  			      t->case_name) {
>  
>  			if (!modprobe.thread) {
> -				igt_require(kunit_set_filtering(NULL, NULL, NULL));
> +				igt_require(kunit_set_filtering(suite,
> +								NULL, NULL));

Can you keep it in one line? With or without it,

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

>  
>  				igt_assert_eq(pthread_mutexattr_init(&attr), 0);
>  				igt_assert_eq(pthread_mutexattr_setrobust(&attr,
> @@ -1443,7 +1427,7 @@ void igt_kunit(const char *module_name, const char *suite, const char *opts)
>  		if (!kunit_get_tests(&tests, &tst, suite, opts))
>  			__igt_kunit_legacy(&tst, subtest, opts);
>  		else
> -			__igt_kunit(&tst, subtest, opts, &tests);
> +			__igt_kunit(&tst, subtest, suite, opts, &tests);
>  	}
>  
>  	igt_fixture {
> -- 
> 2.43.0
>