Re: [PATCH] kunit: test: Move fault tests behind KUNIT_FAULT_TEST Kconfig option

[Rae Moar]

On Tue, Apr 23, 2024 at 5:08 AM David Gow <davidgow@xxxxxxxxxx> wrote:
>
> The NULL dereference tests in kunit_fault deliberately trigger a kernel
> BUG(), and therefore print the associated stack trace, even when the
> test passes. This is both annoying (as it bloats the test output), and
> can confuse some test harnesses, which assume any BUG() is a failure.
>
> Allow these tests to be specifically disabled (without disabling all
> of KUnit's other tests), by placing them behind the
> CONFIG_KUNIT_FAULT_TEST Kconfig option. This is enabled by default, but
> can be set to 'n' to disable the test. An empty 'kunit_fault' suite is
> left behind, which will automatically be marked 'skipped'.
>
> As the fault tests already were disabled under UML (as they weren't
> compatible with its fault handling), we can simply adapt those
> conditions, and add a dependency on !UML for our new option.
>
> Suggested-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/928249cc-e027-4f7f-b43f-502f99a1ea63@xxxxxxxxxxxx/
> Fixes: 82b0beff3497 ("kunit: Add tests for fault")
> Signed-off-by: David Gow <davidgow@xxxxxxxxxx>

Hello!

This looks good to me. Very useful!

Reviewed-by: Rae Moar <rmoar@xxxxxxxxxx>

Thanks!
-Rae

> ---
>  lib/kunit/Kconfig      | 11 +++++++++++
>  lib/kunit/kunit-test.c |  8 ++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 68a6daec0aef..34d7242d526d 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -24,6 +24,17 @@ config KUNIT_DEBUGFS
>           test suite, which allow users to see results of the last test suite
>           run that occurred.
>
> +config KUNIT_FAULT_TEST
> +       bool "Enable KUnit tests which print BUG stacktraces"
> +       depends on KUNIT_TEST
> +       depends on !UML
> +       default y
> +       help
> +         Enables fault handling tests for the KUnit framework. These tests may
> +         trigger a kernel BUG(), and the associated stack trace, even when they
> +         pass. If this conflicts with your test infrastrcture (or is confusing
> +         or annoying), they can be disabled by setting this to N.
> +
>  config KUNIT_TEST
>         tristate "KUnit test for KUnit" if !KUNIT_ALL_TESTS
>         default KUNIT_ALL_TESTS
> diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
> index 0fdca5fffaec..e3412e0ca399 100644
> --- a/lib/kunit/kunit-test.c
> +++ b/lib/kunit/kunit-test.c
> @@ -109,7 +109,7 @@ static struct kunit_suite kunit_try_catch_test_suite = {
>         .test_cases = kunit_try_catch_test_cases,
>  };
>
> -#ifndef CONFIG_UML
> +#if IS_ENABLED(CONFIG_KUNIT_FAULT_TEST)
>
>  static void kunit_test_null_dereference(void *data)
>  {
> @@ -136,12 +136,12 @@ static void kunit_test_fault_null_dereference(struct kunit *test)
>         KUNIT_EXPECT_TRUE(test, ctx->function_called);
>  }
>
> -#endif /* !CONFIG_UML */
> +#endif /* CONFIG_KUNIT_FAULT_TEST */
>
>  static struct kunit_case kunit_fault_test_cases[] = {
> -#ifndef CONFIG_UML
> +#if IS_ENABLED(CONFIG_KUNIT_FAULT_TEST)
>         KUNIT_CASE(kunit_test_fault_null_dereference),
> -#endif /* !CONFIG_UML */
> +#endif /* CONFIG_KUNIT_FAULT_TEST */
>         {}
>  };
>
> --
> 2.44.0.769.g3c40516874-goog
>