[igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip

[Mauro Carvalho Chehab]

On Tue, 13 Jun 2023 09:27:26 +0200
Dominik Karol Piatkowski <dominik.karol.piatkowski at intel.com> wrote:

> Sample drm_buddy output with missing requirements:
> 	Starting subtest: drm_buddy_test
> 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> 	Subtest drm_buddy_test: SKIP (0.001s)
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski at intel.com>
> Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Cc: Mauro Carvalho Chehab <mauro.chehab at linux.intel.com>
> ---
>  lib/igt_kmod.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 1511bdef4..04220f404 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name, const char *opts)
>  	int ret;
>  	struct ktap_test_results *results;
>  	struct ktap_test_results_element *temp;
> +	bool skip = false;
>  
>  	/* get normalized module name */
>  	if (igt_ktest_init(&tst, module_name) != 0) {
> -		igt_warn("Unable to initialize ktest for %s\n", module_name);
> -		igt_fail(IGT_EXIT_SKIP);
> +		igt_skip("Unable to initialize ktest for %s\n", module_name);
>  	}
>  
>  	if (igt_ktest_begin(&tst) != 0) {
> -		igt_warn("Unable to begin ktest for %s\n", module_name);
> -
>  		igt_ktest_fini(&tst);
> -		igt_fail(IGT_EXIT_SKIP);
> +		igt_skip("Unable to begin ktest for %s\n", module_name);
>  	}
>  
>  	if (tst.kmsg < 0) {
>  		igt_warn("Could not open /dev/kmsg\n");
> +		skip = true;

Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
skipping the test.

IMO, the test should be skipped only if:

1. The Kernel is not compiled with KUnit support;
2. The KUnit test was not compiled.

Errors for all other conditions should be considered as failures.

Now, I'm not sure what would be the proper way to test for such
skip condition. I suspect you could check the error code at
igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
e. g. on this part of __igt_kunit():

        /* The KUnit module is required for running any KUnit tests */
        if (igt_kmod_load("kunit", NULL) != 0 ||
            kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 0) {
                igt_warn("Unable to load KUnit\n");
                igt_fail(IGT_EXIT_FAILURE);
        }

        is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;

        results = ktap_parser_start(f, is_builtin);

        if (igt_kmod_load(module_name, opts) != 0) {
                igt_warn("Unable to load %s module\n", module_name);
                ret = ktap_parser_stop();
                igt_fail(IGT_EXIT_FAILURE);
        }


This could be replaced by something similar to:

	err = igt_kmod_load("kunit", NULL);
	igt_skip_on(err == ENOENT, "KUnit not supported");
	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
        if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 0) {
                igt_warn("Unable to load KUnit\n");
                igt_fail(IGT_EXIT_FAILURE);
        }


        is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;

        results = ktap_parser_start(f, is_builtin);

        err = igt_kmod_load(module_name, opts) 
	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
	if (err) {
                igt_warn("Error %d when trying to load %s module\n", err, module_name);
                ret = ktap_parser_stop();
                igt_fail(IGT_EXIT_FAILURE);
        }


In order to explicitly check for (1) and (2).

Regards,
Mauro