[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABVgOS=3WjF=LObZNUcW9wsdnjzrmhAsjHP8uu02cO3F5iWkrA@mail.gmail.com>
Date: Fri, 15 Aug 2025 19:04:21 +0800
From: David Gow <davidgow@...gle.com>
To: Marie Zhussupova <marievic@...gle.com>
Cc: rmoar@...gle.com, shuah@...nel.org, brendan.higgins@...ux.dev,
mark.rutland@....com, elver@...gle.com, dvyukov@...gle.com,
lucas.demarchi@...el.com, thomas.hellstrom@...ux.intel.com,
rodrigo.vivi@...el.com, linux-kselftest@...r.kernel.org,
kunit-dev@...glegroups.com, kasan-dev@...glegroups.com,
intel-xe@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/7] kunit: Introduce param_init/exit for parameterized
test context management
On Fri, 15 Aug 2025 at 18:36, Marie Zhussupova <marievic@...gle.com> wrote:
>
> Add (*param_init) and (*param_exit) function pointers to
> `struct kunit_case`. Users will be able to set them via the new
> KUNIT_CASE_PARAM_WITH_INIT() macro.
>
> param_init/exit will be invoked by kunit_run_tests() once before and once
> after the parameterized test, respectively. They will receive the
> `struct kunit` that holds the parameterized test context; facilitating
> init and exit for shared state.
>
> This patch also sets param_init/exit to None in rust/kernel/kunit.rs.
>
> Reviewed-by: Rae Moar <rmoar@...gle.com>
> Signed-off-by: Marie Zhussupova <marievic@...gle.com>
> ---
Thanks, I've tested the param_init failure case, and it works well for me now.
Reviewed-by: David Gow <davidgow@...gle.com>
Cheers,
-- David
>
> Changes in v3:
> v2: https://lore.kernel.org/all/20250811221739.2694336-3-marievic@google.com/
> - kunit_init_parent_param_test() now sets both the `struct kunit_case`
> and the `struct kunit` statuses as failed if the parameterized test
> init failed. The failure message was also changed to include the failure
> code, mirroring the kunit_suite init failure message.
> - A check for parameter init failure was added in kunit_run_tests(). So,
> if the init failed, the framework will skip the parameter runs and
> update the param_test statistics to count that failure.
> - Commit message formatting.
>
> Changes in v2:
> v1: https://lore.kernel.org/all/20250729193647.3410634-3-marievic@google.com/
> - param init/exit were set to None in rust/kernel/kunit.rs to fix the
> Rust breakage.
> - The name of __kunit_init_parent_test was changed to
> kunit_init_parent_param_test and its call was changed to happen only
> if the test is parameterized.
> - The param_exit call was also moved inside the check for if the test is
> parameterized.
> - KUNIT_CASE_PARAM_WITH_INIT() macro logic was change to not automatically
> set generate_params() to KUnit's built-in generator function. Instead,
> the test user will be asked to provide it themselves.
> - The comments and the commit message were changed to reflect the
> parameterized testing terminology. See the patch series cover letter
> change log for the definitions.
>
> ---
> include/kunit/test.h | 25 +++++++++++++++++++++++++
> lib/kunit/test.c | 27 ++++++++++++++++++++++++++-
> rust/kernel/kunit.rs | 4 ++++
> 3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b47b9a3102f3..d2e1b986b161 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -92,6 +92,8 @@ struct kunit_attributes {
> * @name: the name of the test case.
> * @generate_params: the generator function for parameterized tests.
> * @attr: the attributes associated with the test
> + * @param_init: The init function to run before a parameterized test.
> + * @param_exit: The exit function to run after a parameterized test.
> *
> * A test case is a function with the signature,
> * ``void (*)(struct kunit *)``
> @@ -128,6 +130,8 @@ struct kunit_case {
> const char *name;
> const void* (*generate_params)(const void *prev, char *desc);
> struct kunit_attributes attr;
> + int (*param_init)(struct kunit *test);
> + void (*param_exit)(struct kunit *test);
>
> /* private: internal use only. */
> enum kunit_status status;
> @@ -218,6 +222,27 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status)
> .generate_params = gen_params, \
> .attr = attributes, .module_name = KBUILD_MODNAME}
>
> +/**
> + * KUNIT_CASE_PARAM_WITH_INIT - Define a parameterized KUnit test case with custom
> + * param_init() and param_exit() functions.
> + * @test_name: The function implementing the test case.
> + * @gen_params: The function to generate parameters for the test case.
> + * @init: A reference to the param_init() function to run before a parameterized test.
> + * @exit: A reference to the param_exit() function to run after a parameterized test.
> + *
> + * Provides the option to register param_init() and param_exit() functions.
> + * param_init/exit will be passed the parameterized test context and run once
> + * before and once after the parameterized test. The init function can be used
> + * to add resources to share between parameter runs, and any other setup logic.
> + * The exit function can be used to clean up resources that were not managed by
> + * the parameterized test, and any other teardown logic.
> + */
> +#define KUNIT_CASE_PARAM_WITH_INIT(test_name, gen_params, init, exit) \
> + { .run_case = test_name, .name = #test_name, \
> + .generate_params = gen_params, \
> + .param_init = init, .param_exit = exit, \
> + .module_name = KBUILD_MODNAME}
> +
> /**
> * struct kunit_suite - describes a related collection of &struct kunit_case
> *
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 14a8bd846939..917df2e1688d 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -641,6 +641,20 @@ static void kunit_accumulate_stats(struct kunit_result_stats *total,
> total->total += add.total;
> }
>
> +static void kunit_init_parent_param_test(struct kunit_case *test_case, struct kunit *test)
> +{
> + if (test_case->param_init) {
> + int err = test_case->param_init(test);
> +
> + if (err) {
> + kunit_err(test_case, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
> + "# failed to initialize parent parameter test (%d)", err);
> + test->status = KUNIT_FAILURE;
> + test_case->status = KUNIT_FAILURE;
> + }
> + }
> +}
> +
> int kunit_run_tests(struct kunit_suite *suite)
> {
> char param_desc[KUNIT_PARAM_DESC_SIZE];
> @@ -678,6 +692,11 @@ int kunit_run_tests(struct kunit_suite *suite)
> kunit_run_case_catch_errors(suite, test_case, &test);
> kunit_update_stats(¶m_stats, test.status);
> } else {
> + kunit_init_parent_param_test(test_case, &test);
> + if (test_case->status == KUNIT_FAILURE) {
> + kunit_update_stats(¶m_stats, test.status);
> + goto test_case_end;
> + }
> /* Get initial param. */
> param_desc[0] = '\0';
> /* TODO: Make generate_params try-catch */
> @@ -714,10 +733,16 @@ int kunit_run_tests(struct kunit_suite *suite)
> param_desc[0] = '\0';
> curr_param = test_case->generate_params(curr_param, param_desc);
> }
> + /*
> + * TODO: Put into a try catch. Since we don't need suite->exit
> + * for it we can't reuse kunit_try_run_cleanup for this yet.
> + */
> + if (test_case->param_exit)
> + test_case->param_exit(&test);
> /* TODO: Put this kunit_cleanup into a try-catch. */
> kunit_cleanup(&test);
> }
> -
> +test_case_end:
> kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE);
>
> kunit_print_test_stats(&test, param_stats);
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 4b8cdcb21e77..cda64574b44d 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -207,6 +207,8 @@ pub const fn kunit_case(
> status: kernel::bindings::kunit_status_KUNIT_SUCCESS,
> module_name: core::ptr::null_mut(),
> log: core::ptr::null_mut(),
> + param_init: None,
> + param_exit: None,
> }
> }
>
> @@ -226,6 +228,8 @@ pub const fn kunit_case_null() -> kernel::bindings::kunit_case {
> status: kernel::bindings::kunit_status_KUNIT_SUCCESS,
> module_name: core::ptr::null_mut(),
> log: core::ptr::null_mut(),
> + param_init: None,
> + param_exit: None,
> }
> }
>
> --
> 2.51.0.rc1.167.g924127e9c0-goog
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5281 bytes)
Powered by blists - more mailing lists