Re: [PATCH 1/9] kunit: Add parent kunit for parameterized test context

On Wed, 30 Jul 2025 at 03:37, Marie Zhussupova marievic@google.com wrote:

Currently, KUnit parameterized tests lack a mechanism to share resources across individual test invocations because the same `struct kunit` instance is reused for each test.

This patch refactors kunit_run_tests() to provide each parameterized test with its own `struct kunit` instance. A new parent pointer is added to `struct kunit`, allowing individual parameterized tests to reference a shared parent kunit instance. Resources added to this parent will then be accessible to all individual parameter test executions.

Signed-off-by: Marie Zhussupova marievic@google.com

I'm definitely a fan of this, though it's not without its downsides.

For most tests, I don't think this will be a noticeable difference, since the "shared" struct kunit is reset after each parameter anyway, so this is a pretty safe change.

Having a 'parent' struct kunit, and hence potentially a hierarchy, does give us a good way of implementing more complicated parameterised tests which might need some more persistent state. (Ultimately, we could have another level for suites, and then allow suite_init/suite_exit to setup something persistent, too.)

Anyway, this looks good to me. I've left some small notes below, but nothing I think is actionable.

Reviewed-by: David Gow davidgow@google.com

Cheers, -- David

include/kunit/test.h | 12 ++++++++++-- lib/kunit/test.c | 32 +++++++++++++++++++------------- 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h index 39c768f87dc9..a42d0c8cb985 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -268,14 +268,22 @@ struct kunit_suite_set {

• @priv: for user to store arbitrary data. Commonly used to pass data

•   created in the init function (see &struct kunit_suite).
  

• • @parent: for user to store data that they want to shared across

• •     parameterized tests.
    

A part of me would prefer this not to explicitly call out parameterized tests here, as the obvious extensions to this (having suites have a struct kunit, or other forms of test hierarchy) would still make use of this in non-parameterised use-cases.

But since parameterised tests are the only current use-case for it, I can live with it if you'd prefer.

• Used to store information about the current context under which the test
• is running. Most of this data is private and should only be accessed

• • indirectly via public functions; the one exception is @priv which can be
• • used by the test writer to store arbitrary data.

• • indirectly via public functions; the two exceptions are @priv and @parent
• • which can be used by the test writer to store arbitrary data or data that is
• • available to all parameter test executions, respectively.
  */

struct kunit { void *priv;

•   /*
  

•    * Reference to the parent struct kunit for storing shared resources
  

•    * during parameterized testing.
  

•    */
  

•   struct kunit *parent;
  
    /* private: internal use only. */
    const char *name; /* Read only after initialization! */
  

diff --git a/lib/kunit/test.c b/lib/kunit/test.c index f3c6b11f12b8..4d6a39eb2c80 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -647,6 +647,7 @@ int kunit_run_tests(struct kunit_suite *suite) struct kunit_case *test_case; struct kunit_result_stats suite_stats = { 0 }; struct kunit_result_stats total_stats = { 0 };

•   const void *curr_param;
  
    /* Taint the kernel so we know we've run tests. */
    add_taint(TAINT_TEST, LOCKDEP_STILL_OK);
  

@@ -679,36 +680,39 @@ int kunit_run_tests(struct kunit_suite *suite) } else { /* Get initial param. */ param_desc[0] = '\0';

•                   test.param_value = test_case->generate_params(NULL, param_desc);
  

•                   /* TODO: Make generate_params try-catch */
  

Thanks for adding the TODO here: this isn't a regression, but it's good to note that we should get around to fixing it.

•                   curr_param = test_case->generate_params(NULL, param_desc);
                    test_case->status = KUNIT_SKIPPED;
                    kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
                              "KTAP version 1\n");
                    kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
                              "# Subtest: %s", test_case->name);
  
  

•                   while (test.param_value) {
  

•                           kunit_run_case_catch_errors(suite, test_case, &test);
  

•                   while (curr_param) {
  

•                           struct kunit param_test = {
  

•                                   .param_value = curr_param,
  

•                                   .param_index = ++test.param_index,
  

•                                   .parent = &test,
  

•                           };
  

•                           kunit_init_test(&param_test, test_case->name, test_case->log);
  

•                           kunit_run_case_catch_errors(suite, test_case, &param_test);
  
                            if (param_desc[0] == '\0') {
                                    snprintf(param_desc, sizeof(param_desc),
                                             "param-%d", test.param_index);
                            }
  
  

•                           kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE_PARAM,
  

•                                                 test.status,
  

•                                                 test.param_index + 1,
  

•                           kunit_print_ok_not_ok(&param_test, KUNIT_LEVEL_CASE_PARAM,
  

•                                                 param_test.status,
  

•                                                 param_test.param_index,
                                                  param_desc,
  

•                                                 test.status_comment);
  

•                                                 param_test.status_comment);
  
  

•                           kunit_update_stats(&param_stats, test.status);
  

•                           kunit_update_stats(&param_stats, param_test.status);
  
                            /* Get next param. */
                            param_desc[0] = '\0';
  

•                           test.param_value = test_case->generate_params(test.param_value, param_desc);
  

•                           test.param_index++;
  

•                           test.status = KUNIT_SUCCESS;
  

•                           test.status_comment[0] = '\0';
  

•                           test.priv = NULL;
  

•                           curr_param = test_case->generate_params(curr_param, param_desc);
                    }
            }
  
  

@@ -723,6 +727,8 @@ int kunit_run_tests(struct kunit_suite *suite)

            kunit_update_stats(&suite_stats, test_case->status);
            kunit_accumulate_stats(&total_stats, param_stats);

•           /* TODO: Put this kunit_cleanup into a try-catch. */
  

•           kunit_cleanup(&test);
  

Hmm... it is a shame we can't easily include this in the existing try-catch mechanism. Definitely worth fixing in a follow-up.

    }

    if (suite->suite_exit)

-- 2.50.1.552.g942d659e1b-goog