| Message ID | 20201106182657.30492-1-98.arpi@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [v5,1/2] kunit: Support for Parameterized Testing | expand |
On Fri, 6 Nov 2020 at 19:28, Arpitha Raghunandan <98.arpi@gmail.com> wrote: > Implementation of support for parameterized testing in KUnit. > This approach requires the creation of a test case using the > KUNIT_CASE_PARAM macro that accepts a generator function as input. > This generator function should return the next parameter given the > previous parameter in parameterized tests. It also provides > a macro to generate common-case generators. > > Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> > Co-developed-by: Marco Elver <elver@google.com> > Signed-off-by: Marco Elver <elver@google.com> > --- [...] > include/kunit/test.h | 36 ++++++++++++++++++++++++++++++++++ > lib/kunit/test.c | 46 +++++++++++++++++++++++++++++++------------- > 2 files changed, 69 insertions(+), 13 deletions(-) I think this is ready. Thank you for your patience, and addressing my comments! I'm obviously fine with v5, but I think my Ack or Review won't count much because of Co-developed. :-) Others: Please take another look. Thanks, -- Marco
On Fri, 6 Nov 2020 at 19:28, Arpitha Raghunandan <98.arpi@gmail.com> wrote: > > Implementation of support for parameterized testing in KUnit. > This approach requires the creation of a test case using the > KUNIT_CASE_PARAM macro that accepts a generator function as input. > This generator function should return the next parameter given the > previous parameter in parameterized tests. It also provides > a macro to generate common-case generators. > > Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> > Co-developed-by: Marco Elver <elver@google.com> > Signed-off-by: Marco Elver <elver@google.com> [...] > - kunit_suite_for_each_test_case(suite, test_case) > - kunit_run_case_catch_errors(suite, test_case); > + kunit_suite_for_each_test_case(suite, test_case) { > + struct kunit test = { .param_value = NULL, .param_index = 0 }; > + bool test_success = true; > + > + if (test_case->generate_params) > + test.param_value = test_case->generate_params(NULL); > + > + do { > + kunit_run_case_catch_errors(suite, test_case, &test); > + test_success &= test_case->success; > + > + if (test_case->generate_params) { > + kunit_log(KERN_INFO, &test, > + KUNIT_SUBTEST_INDENT > + "# %s: param-%d %s", > + test_case->name, test.param_index, > + kunit_status_to_string(test.success)); Sorry, I still found something. The patch I sent had this aligned with the '(', whereas when I apply this patch it no longer is aligned. Why? I see the rest of the file also aligns arguments with opening '(', so I think your change is inconsistent. Thanks, -- Marco
On 07/11/20 12:15 am, Marco Elver wrote: > On Fri, 6 Nov 2020 at 19:28, Arpitha Raghunandan <98.arpi@gmail.com> wrote: >> >> Implementation of support for parameterized testing in KUnit. >> This approach requires the creation of a test case using the >> KUNIT_CASE_PARAM macro that accepts a generator function as input. >> This generator function should return the next parameter given the >> previous parameter in parameterized tests. It also provides >> a macro to generate common-case generators. >> >> Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> >> Co-developed-by: Marco Elver <elver@google.com> >> Signed-off-by: Marco Elver <elver@google.com> > [...] >> - kunit_suite_for_each_test_case(suite, test_case) >> - kunit_run_case_catch_errors(suite, test_case); >> + kunit_suite_for_each_test_case(suite, test_case) { >> + struct kunit test = { .param_value = NULL, .param_index = 0 }; >> + bool test_success = true; >> + >> + if (test_case->generate_params) >> + test.param_value = test_case->generate_params(NULL); >> + >> + do { >> + kunit_run_case_catch_errors(suite, test_case, &test); >> + test_success &= test_case->success; >> + >> + if (test_case->generate_params) { >> + kunit_log(KERN_INFO, &test, >> + KUNIT_SUBTEST_INDENT >> + "# %s: param-%d %s", >> + test_case->name, test.param_index, >> + kunit_status_to_string(test.success)); > > Sorry, I still found something. The patch I sent had this aligned with > the '(', whereas when I apply this patch it no longer is aligned. Why? > > I see the rest of the file also aligns arguments with opening '(', so > I think your change is inconsistent. > Ah those lines had spaces instead of tab and I think I messed up the alignment fixing that. I will send another version fixing this. Thanks! > Thanks, > -- Marco >
On Fri, 6 Nov 2020 at 20:00, Arpitha Raghunandan <98.arpi@gmail.com> wrote: > > On 07/11/20 12:15 am, Marco Elver wrote: > > On Fri, 6 Nov 2020 at 19:28, Arpitha Raghunandan <98.arpi@gmail.com> wrote: > >> > >> Implementation of support for parameterized testing in KUnit. > >> This approach requires the creation of a test case using the > >> KUNIT_CASE_PARAM macro that accepts a generator function as input. > >> This generator function should return the next parameter given the > >> previous parameter in parameterized tests. It also provides > >> a macro to generate common-case generators. > >> > >> Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> > >> Co-developed-by: Marco Elver <elver@google.com> > >> Signed-off-by: Marco Elver <elver@google.com> > > [...] > >> - kunit_suite_for_each_test_case(suite, test_case) > >> - kunit_run_case_catch_errors(suite, test_case); > >> + kunit_suite_for_each_test_case(suite, test_case) { > >> + struct kunit test = { .param_value = NULL, .param_index = 0 }; > >> + bool test_success = true; > >> + > >> + if (test_case->generate_params) > >> + test.param_value = test_case->generate_params(NULL); > >> + > >> + do { > >> + kunit_run_case_catch_errors(suite, test_case, &test); > >> + test_success &= test_case->success; > >> + > >> + if (test_case->generate_params) { > >> + kunit_log(KERN_INFO, &test, > >> + KUNIT_SUBTEST_INDENT > >> + "# %s: param-%d %s", > >> + test_case->name, test.param_index, > >> + kunit_status_to_string(test.success)); > > > > Sorry, I still found something. The patch I sent had this aligned with > > the '(', whereas when I apply this patch it no longer is aligned. Why? > > > > I see the rest of the file also aligns arguments with opening '(', so > > I think your change is inconsistent. > > > > Ah those lines had spaces instead of tab and I think I messed up the alignment > fixing that. I will send another version fixing this. > Thanks! It was tabs then <8 spaces to align. checkpatch.pl certainly is happy with that. > > Thanks, > > -- Marco > > >
diff --git a/include/kunit/test.h b/include/kunit/test.h index 9197da792336..ae5488a37e48 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -107,6 +107,7 @@ struct kunit; * * @run_case: the function representing the actual test case. * @name: the name of the test case. + * @generate_params: the generator function for parameterized tests. * * A test case is a function with the signature, * ``void (*)(struct kunit *)`` @@ -141,6 +142,7 @@ struct kunit; struct kunit_case { void (*run_case)(struct kunit *test); const char *name; + const void* (*generate_params)(const void *prev); /* private: internal use only. */ bool success; @@ -163,6 +165,22 @@ static inline char *kunit_status_to_string(bool status) */ #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name } +/** + * KUNIT_CASE_PARAM - A helper for creation a parameterized &struct kunit_case + * + * @test_name: a reference to a test case function. + * @gen_params: a reference to a parameter generator function. + * + * The generator function ``const void* gen_params(const void *prev)`` is used + * to lazily generate a series of arbitrarily typed values that fit into a + * void*. The argument @prev is the previously returned value, which should be + * used to derive the next value; @prev is set to NULL on the initial generator + * call. When no more values are available, the generator must return NULL. + */ +#define KUNIT_CASE_PARAM(test_name, gen_params) \ + { .run_case = test_name, .name = #test_name, \ + .generate_params = gen_params } + /** * struct kunit_suite - describes a related collection of &struct kunit_case * @@ -208,6 +226,10 @@ struct kunit { const char *name; /* Read only after initialization! */ char *log; /* Points at case log after initialization */ struct kunit_try_catch try_catch; + /* param_value is the current parameter value for a test case. */ + const void *param_value; + /* param_index stores the index of the parameter in parameterized tests. */ + int param_index; /* * success starts as true, and may only be set to false during a * test case; thus, it is safe to update this across multiple @@ -1742,4 +1764,18 @@ do { \ fmt, \ ##__VA_ARGS__) +/** + * KUNIT_ARRAY_PARAM() - Define test parameter generator from an array. + * @name: prefix for the test parameter generator function. + * @array: array of test parameters. + * + * Define function @name_gen_params which uses @array to generate parameters. + */ +#define KUNIT_ARRAY_PARAM(name, array) \ + static const void *name##_gen_params(const void *prev) \ + { \ + typeof((array)[0]) * __next = prev ? ((typeof(__next)) prev) + 1 : (array); \ + return __next - (array) < ARRAY_SIZE((array)) ? __next : NULL; \ + } + #endif /* _KUNIT_TEST_H */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 750704abe89a..b8b63aeda504 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -325,29 +325,25 @@ static void kunit_catch_run_case(void *data) * occur in a test case and reports them as failures. */ static void kunit_run_case_catch_errors(struct kunit_suite *suite, - struct kunit_case *test_case) + struct kunit_case *test_case, + struct kunit *test) { struct kunit_try_catch_context context; struct kunit_try_catch *try_catch; - struct kunit test; - kunit_init_test(&test, test_case->name, test_case->log); - try_catch = &test.try_catch; + kunit_init_test(test, test_case->name, test_case->log); + try_catch = &test->try_catch; kunit_try_catch_init(try_catch, - &test, + test, kunit_try_run_case, kunit_catch_run_case); - context.test = &test; + context.test = test; context.suite = suite; context.test_case = test_case; kunit_try_catch_run(try_catch, &context); - test_case->success = test.success; - - kunit_print_ok_not_ok(&test, true, test_case->success, - kunit_test_case_num(suite, test_case), - test_case->name); + test_case->success = test->success; } int kunit_run_tests(struct kunit_suite *suite) @@ -356,8 +352,32 @@ int kunit_run_tests(struct kunit_suite *suite) kunit_print_subtest_start(suite); - kunit_suite_for_each_test_case(suite, test_case) - kunit_run_case_catch_errors(suite, test_case); + kunit_suite_for_each_test_case(suite, test_case) { + struct kunit test = { .param_value = NULL, .param_index = 0 }; + bool test_success = true; + + if (test_case->generate_params) + test.param_value = test_case->generate_params(NULL); + + do { + kunit_run_case_catch_errors(suite, test_case, &test); + test_success &= test_case->success; + + if (test_case->generate_params) { + kunit_log(KERN_INFO, &test, + KUNIT_SUBTEST_INDENT + "# %s: param-%d %s", + test_case->name, test.param_index, + kunit_status_to_string(test.success)); + test.param_value = test_case->generate_params(test.param_value); + test.param_index++; + } + } while (test.param_value); + + kunit_print_ok_not_ok(&test, true, test_success, + kunit_test_case_num(suite, test_case), + test_case->name); + } kunit_print_subtest_end(suite);