Re: [PATCH i-g-t v2 2/2] lib/kunit: Minimize code duplication

From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
To: igt-dev@lists.freedesktop.org
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [PATCH i-g-t v2 2/2] lib/kunit: Minimize code duplication
Date: Thu, 7 Mar 2024 18:48:14 +0100	[thread overview]
Message-ID: <20240307174814.kalq3rg2qka4yg7r@kamilkon-desk.igk.intel.com> (raw)
In-Reply-To: <20240227151128.16802-6-janusz.krzysztofik@linux.intel.com>

Hi Janusz,
On 2024-02-27 at 16:10:42 +0100, Janusz Krzysztofik wrote:
> A new helper has been introduced recently, used for fetching KTAP results
> of a single test case.  Since that helper is called for that purpose
> only after the test module is loaded with all other test cases filtered
> out, its actual implementation is as simple as collecting all results from
> a single KTAP report, no matter how many test suites and test cases it
> covers.  Then, it's a good candidate for reuse in other scenarios when a
> single KTAP report is handled, e.g., when we collect a list of test cases
> from a single test suite or test module.  Go for it.
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

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

> ---
>  lib/igt_kmod.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index e9e00ac5b2..86cf50d5b7 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -1177,9 +1177,8 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  			    const char *opts)
>  {
>  	struct igt_ktap_result *r, *rn;
> -	struct igt_ktap_results *ktap;
>  	unsigned long taints;
> -	int flags, err;
> +	int flags;
>  
>  	igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
>  
> @@ -1203,17 +1202,7 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  	igt_skip_on(modprobe(tst->kmod, opts));
>  	igt_skip_on(igt_kernel_tainted(&taints));
>  
> -	ktap = igt_ktap_alloc(tests);
> -	igt_require(ktap);
> -
> -	do
> -		err = kunit_kmsg_result_get(tests, NULL, tst->kmsg, ktap);
> -	while (err == -EINPROGRESS);
> -
> -	igt_ktap_free(ktap);
> -
> -	igt_skip_on_f(err,
> -		      "KTAP parser failed while getting a list of test cases\n");
> +	igt_skip_on(kunit_get_results(tests, tst->kmsg));
>  
>  	igt_list_for_each_entry_safe(r, rn, tests, link)
>  		igt_require_f(r->code == IGT_EXIT_SKIP,
> -- 
> 2.43.0
>