On Tue, Jul 29, 2025 at 3:37 PM Marie Zhussupova marievic@google.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.
Hello!
Very intrigued by this idea to add an init and exit function for parameterized tests. In a way, this allows parameterized test series to act more like suites. Either way I am happy to see more flexibility being brought to the parameterized test framework.
I have a few comments below that I would like to discuss before a final review. But this patch is looking good.
Thanks! -Rae
These functions are invoked by kunit_run_tests() once before and once after the entire parameterized test series, respectively.
This is a philosophical question but should we refer to a group of parameterized tests as a parameterized test series or a parameterized test suite? In the KTAP, the appearance is identical to a suite but in the running of the tests it acts distinct to a test case or suite. Curious on David's opinion here.
They will receive the parent kunit test instance, allowing users to register and manage shared resources. Resources added to this parent kunit test will be accessible to all individual parameterized tests, facilitating init and exit for shared state.
Signed-off-by: Marie Zhussupova marievic@google.com
include/kunit/test.h | 33 ++++++++++++++++++++++++++++++++- lib/kunit/test.c | 23 ++++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index a42d0c8cb985..d8dac7efd745 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 parameterized tests.
- @param_exit: The exit function to run after parameterized tests.
If we decide on a terminology for the parameterized test group, it might be clearer to label these "The init function to run before parameterized test [suite/series]." and same for the exit function.
- A test case is a function with the signature,
- ``void (*)(struct kunit *)``
@@ -129,6 +131,13 @@ struct kunit_case { const void* (*generate_params)(const void *prev, char *desc); struct kunit_attributes attr;
/*
* Optional user-defined functions: one to register shared resources once
* before the parameterized test series, and another to release them after.
*/
int (*param_init)(struct kunit *test);
void (*param_exit)(struct kunit *test);
/* private: internal use only. */ enum kunit_status status; char *module_name;
@@ -218,6 +227,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
- init and exit functions.
- @test_name: The function implementing the test case.
- @gen_params: The function to generate parameters for the test case.
- @init: The init function to run before parameterized tests.
- @exit: The exit function to run after parameterized tests.
If we do change the description above of param_init/param_exit, it might be nice to change it here too.
- Provides the option to register init and exit functions that take in the
- parent of the parameterized tests and run once before and once after the
- parameterized test series. The init function can be used to add any resources
- to share between the parameterized tests or to pass parameter arrays. The
- exit function can be used to clean up any resources that are not managed by
- the test.
- */
+#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
@@ -269,7 +299,8 @@ 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.
parameterized tests. Typically, the data is provided in
the param_init function (see &struct kunit_case).
- 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
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 4d6a39eb2c80..d80b5990d85d 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -641,6 +641,19 @@ static void kunit_accumulate_stats(struct kunit_result_stats *total, total->total += add.total; }
+static void __kunit_init_parent_test(struct kunit_case *test_case, struct kunit *test)
It would be nice to include "param" in this function name. Currently it sounds more like you are initializing the @parent field of 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.");
test_case->status = KUNIT_FAILURE;
}
}
+}
int kunit_run_tests(struct kunit_suite *suite) { char param_desc[KUNIT_PARAM_DESC_SIZE]; @@ -668,6 +681,8 @@ int kunit_run_tests(struct kunit_suite *suite) struct kunit_result_stats param_stats = { 0 };
kunit_init_test(&test, test_case->name, test_case->log);
__kunit_init_parent_test(test_case, &test);
Is it possible to move this so this function is only called when it is a parameterized test? I see the check for KUNIT_FAILURE is useful but I think I would still prefer this within the section for parameterized tests.
if (test_case->status == KUNIT_SKIPPED) { /* Test marked as skip */ test.status = KUNIT_SKIPPED;
@@ -677,7 +692,7 @@ int kunit_run_tests(struct kunit_suite *suite) test_case->status = KUNIT_SKIPPED; kunit_run_case_catch_errors(suite, test_case, &test); kunit_update_stats(¶m_stats, test.status);
} else {
} else if (test_case->status != KUNIT_FAILURE) { /* Get initial param. */ param_desc[0] = '\0'; /* TODO: Make generate_params try-catch */
@@ -727,6 +742,12 @@ 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 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);
Also here I am not sure why this is done outside of the check for if the test is parameterized? Either way this should definitely be done before the test stats and ok/not ok line are printed because if there is any log output during the param_exit function it is necessary to print that before the status line to identify that that log corresponds with that test.
Also just curious why you chose to implement a function to perform the param_init but not the param_exit?
/* TODO: Put this kunit_cleanup into a try-catch. */ kunit_cleanup(&test); }
-- 2.50.1.552.g942d659e1b-goog