Re: [PATCH 6/9] kunit: Enable direct registration of parameter arrays to a KUnit test

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

KUnit parameterized tests currently support two primary methods for getting parameters:

1. Defining custom logic within a `generate_params` function.
2. Using the KUNIT_ARRAY_PARAM and KUNIT_ARRAY_PARAM_DESC macros with pre-defined static arrays.

These methods present limitations when dealing with dynamically generated parameter arrays, or in scenarios where populating parameters sequentially via `generate_params` is inefficient or overly complex.

This patch addresses these limitations by adding a new `params_data` field to `struct kunit`, of the type `kunit_params`. The struct `kunit_params` is designed to store the parameter array itself, along with essential metadata including the parameter count, parameter size, and a `get_description` function for providing custom descriptions for individual parameters.

The `params_data` field can be populated by calling the new `kunit_register_params_array` macro from within a `param_init` function. By attaching the parameter array directly to the parent kunit test instance, these parameters can be iterated over in kunit_run_tests() behind the scenes.

This modification provides greater flexibility to the KUnit framework, allowing testers to easily register and utilize both dynamic and static parameter arrays.

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

A few thoughts: - Refactoring out the parameter data into struct kunit_params makes sense, particularly given the new features. - We now have 3 APIs to handle parameters (all of which use a generator function behind the scenes): a static array (which uses a macro to invent a custom generator function), a dynamic array (which uses kunit_get_next_param_and_desc as a -- possibly implied -- generator function), and a user-provided generator function. I don't think that's a problem, but maybe these can be brought slightly closer together in implementation? - Should users pass NULL or kunit_get_next_param_and_desc as the gen_params function if they're using a dynamic array? We seem to support both here, but it's unsure which is preferred. My gut feeling is that we should either totally pick one or the other: if we say NULL implies kunit_get_next_param_and_desc, we should just set kunit_params.generate_params to NULL, and only check when we call it; if we suggest people use kunit_get_next_param_and_desc, we should not support NULL at all (it goes back to implying a non-parameterised test). My gut preference is for the latter, though it's not without downsides.

More comments below.

Cheers, -- David

include/kunit/test.h | 54 ++++++++++++++++++++++++++++++++++++++++---- lib/kunit/test.c | 26 ++++++++++++++++++++- 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h index 4ba65dc35710..9143f0e22323 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -245,7 +245,8 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status) */ #define KUNIT_CASE_PARAM_WITH_INIT(test_name, gen_params, init, exit) \ { .run_case = test_name, .name = #test_name, \

•             .generate_params = gen_params,                                \
  

•             .generate_params = (gen_params)                               \
  

•              ?: kunit_get_next_param_and_desc,                            \
  

A part of me wonders whether we need this, or should just tell people to pass kunit_get_next_param_and_desc manually, rather than NULL.

(If so, _maybe_ it'd make sense to rename it to something more obvious, like kunit_get_array_param?)

              .param_init = init, .param_exit = exit,                       \
              .module_name = KBUILD_MODNAME}

@@ -294,6 +295,21 @@ struct kunit_suite_set { struct kunit_suite * const *end; };

+/* Stores the pointer to the parameter array and its metadata. */ +struct kunit_params {

•   /*
  

•    * Reference to the parameter array for the parameterized tests. This
  

•    * is NULL if a parameter array wasn't directly passed to the
  

•    * parent kunit struct via the kunit_register_params_array macro.
  

•    */
  

Would it make sense to update KUNIT_ARRAY_PARAM(,_DESC) to use this member?

I'd imagine we could get rid of the custom generated function for KUNIT_ARRAY_PARAM, though we'd still need it for the _DESC variant.

(Unless you want to go overboard, add a new offset_of_description member, and make it possible to pass a description field even with dynamic arrays. Though I don't know of anyone who actually needs that.)

The existing "generated function" method is still correct (and maybe slightly faster, though if this is a serious bottleneck, I'll eat my hat), so I don't know that this change is necessary. Maybe worth trying, or looking at for a follow-up.

•   const void *params;
  

•   /* Reference to a function that gets the description of a parameter. */
  

•   void (*get_description)(const void *param, char *desc);
  

•   int num_params;
  

•   size_t elem_size;
  

+};

/**

• struct kunit - represents a running instance of a test.

@@ -302,12 +318,14 @@ struct kunit_suite_set {

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

•     parameterized tests. Typically, the data is provided in
  

•     the param_init function (see &struct kunit_case).
  

• • @params_data: for users to directly store the parameter array.
  • 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 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.

• • indirectly via public functions. There are three exceptions to this: @priv,
• • @parent, and @params_data. These members can be used by the test writer to
• • store arbitrary data, data available to all parameter test executions, and
• • the parameter array, respectively.
  */

struct kunit { void *priv; @@ -316,6 +334,8 @@ struct kunit { * during parameterized testing. */ struct kunit *parent;

•   /* Stores the params array and all data related to it. */
  

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

@@ -386,6 +406,8 @@ void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr) struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set, struct kunit_suite_set suite_set);

+const void *kunit_get_next_param_and_desc(struct kunit *test, const void *prev, char *desc);

#if IS_BUILTIN(CONFIG_KUNIT) int kunit_run_all_tests(void); #else @@ -1735,6 +1757,30 @@ do { \ return NULL; \ }

+/**

• • kunit_register_params_array() - Register parameters for a KUnit test.
• • @test: The KUnit test structure to which parameters will be added.
• • @params_arr: An array of test parameters.
• • @param_cnt: Number of parameters.
• • @get_desc: A pointer to a function that generates a string description for
• • a given parameter element.
• • This macro initializes the @test's parameter array data, storing information
• • including the parameter array, its count, the element size, and the parameter
• • description function within `test->params_data`. KUnit's built-in
• • `kunit_get_next_param_and_desc` function will automatically read this
• • data when a custom `generate_params` function isn't provided.
• */

+#define kunit_register_params_array(test, params_arr, param_cnt, get_desc) \

•   do {                                                                                    \
  

•           struct kunit *_test = (test);                                           \
  

•           const typeof((params_arr)[0]) * _params_ptr = &(params_arr)[0];                 \
  

•           _test->params_data.params = _params_ptr;                                        \
  

•           _test->params_data.num_params = (param_cnt);                                    \
  

•           _test->params_data.elem_size = sizeof(*_params_ptr);                            \
  

•           _test->params_data.get_description = (get_desc);                                \
  

•   } while (0)
  

// TODO(dlatypov@google.com): consider eventually migrating users to explicitly // include resource.h themselves if they need it. #include <kunit/resource.h> diff --git a/lib/kunit/test.c b/lib/kunit/test.c index f50ef82179c4..2f4b7087db3f 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -337,6 +337,13 @@ void __kunit_do_failed_assertion(struct kunit *test, } EXPORT_SYMBOL_GPL(__kunit_do_failed_assertion);

+static void __kunit_init_params(struct kunit *test) +{

•   test->params_data.params = NULL;
  

•   test->params_data.num_params = 0;
  

•   test->params_data.elem_size = 0;
  

+}

void kunit_init_test(struct kunit *test, const char *name, struct string_stream *log) { spin_lock_init(&test->lock); @@ -347,6 +354,7 @@ void kunit_init_test(struct kunit *test, const char *name, struct string_stream string_stream_clear(log); test->status = KUNIT_SUCCESS; test->status_comment[0] = '\0';

•   __kunit_init_params(test);
  

} EXPORT_SYMBOL_GPL(kunit_init_test);

@@ -641,6 +649,22 @@ static void kunit_accumulate_stats(struct kunit_result_stats *total, total->total += add.total; }

+const void *kunit_get_next_param_and_desc(struct kunit *test, const void *prev, char *desc)

As Rae points out, this needs to be exported if we want this to work in modules.

(Unless you want to go all of the way down the "generate_params == NULL implies a parameter array" route, in which case we could get away with it.)

+{

•   struct kunit_params *params_arr = &test->params_data;
  

•   const void *param;
  

•   if (test->param_index < params_arr->num_params) {
  

•           param = (char *)params_arr->params
  

•                   + test->param_index * params_arr->elem_size;
  

•           if (params_arr->get_description)
  

•                   params_arr->get_description(param, desc);
  

•           return param;
  

•   }
  

•   return NULL;
  

+}

static void __kunit_init_parent_test(struct kunit_case *test_case, struct kunit *test) { if (test_case->param_init) { @@ -687,7 +711,7 @@ int kunit_run_tests(struct kunit_suite *suite) /* Test marked as skip */ test.status = KUNIT_SKIPPED; kunit_update_stats(&param_stats, test.status);

•           } else if (!test_case->generate_params) {
  

•           } else if (!test_case->generate_params && !test.params_data.params) {
  

As above, this really depends on if we want generate_params == NULL to imply that we're using a parameter array, or if it should imply that we're a non-parameterised test.

                    /* Non-parameterised test. */
                    test_case->status = KUNIT_SKIPPED;
                    kunit_run_case_catch_errors(suite, test_case, &test);

-- 2.50.1.552.g942d659e1b-goog