| Message ID | 20201116054035.211498-1-98.arpi@gmail.com |
|---|---|
| State | Accepted |
| Commit | fadb08e7c7501ed42949e646c6865ba4ec5dd948 |
| Headers | show |
| Series | [v9,1/2] kunit: Support for Parameterized Testing | expand |
On Mon, 16 Nov 2020 at 06:41, 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 based on arrays. Generators may also > optionally provide a human-readable description of parameters, which is > displayed where available. > > Note, currently the result of each parameter run is displayed in > diagnostic lines, and only the overall test case output summarizes > TAP-compliant success or failure of all parameter runs. In future, when > supported by kunit-tool, these can be turned into subsubtest outputs. > > 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> This and patch 2/2 look good to me. Thank you! -- Marco > --- > Changes v8->v9: > - No change to this patch of the patch series > > Changes v7->v8: > - Increase KUNIT_PARAM_DESC_SIZE to 128 > - Format pointer style appropriately > > Changes v6->v7: > - Clarify commit message. > - Introduce ability to optionally generate descriptions for parameters; > if no description is provided, we'll still print 'param-N'. > - Change diagnostic line format to: > # <test-case-name>: <ok|not ok> N - [<param description>] > > Changes v5->v6: > - Fix alignment to maintain consistency > > Changes v4->v5: > - Update kernel-doc comments. > - Use const void* for generator return and prev value types. > - Add kernel-doc comment for KUNIT_ARRAY_PARAM. > - Rework parameterized test case execution strategy: each parameter is executed > as if it was its own test case, with its own test initialization and cleanup > (init and exit are called, etc.). However, we cannot add new test cases per TAP > protocol once we have already started execution. Instead, log the result of > each parameter run as a diagnostic comment. > > Changes v3->v4: > - Rename kunit variables > - Rename generator function helper macro > - Add documentation for generator approach > - Display test case name in case of failure along with param index > > Changes v2->v3: > - Modifictaion of generator macro and method > > Changes v1->v2: > - Use of a generator method to access test case parameters > Changes v6->v7: > - Clarify commit message. > - Introduce ability to optionally generate descriptions for parameters; > if no description is provided, we'll still print 'param-N'. > - Change diagnostic line format to: > # <test-case-name>: <ok|not ok> N - [<param description>] > - Before execution of parameterized test case, count number of > parameters and display number of parameters. Currently also as a > diagnostic line, but this may be used in future to generate a subsubtest > plan. A requirement of this change is that generators must generate a > deterministic number of parameters. > > Changes v5->v6: > - Fix alignment to maintain consistency > > Changes v4->v5: > - Update kernel-doc comments. > - Use const void* for generator return and prev value types. > - Add kernel-doc comment for KUNIT_ARRAY_PARAM. > - Rework parameterized test case execution strategy: each parameter is executed > as if it was its own test case, with its own test initialization and cleanup > (init and exit are called, etc.). However, we cannot add new test cases per TAP > protocol once we have already started execution. Instead, log the result of > each parameter run as a diagnostic comment. > > Changes v3->v4: > - Rename kunit variables > - Rename generator function helper macro > - Add documentation for generator approach > - Display test case name in case of failure along with param index > > Changes v2->v3: > - Modifictaion of generator macro and method > > Changes v1->v2: > - Use of a generator method to access test case parameters > > include/kunit/test.h | 51 ++++++++++++++++++++++++++++++++++++++ > lib/kunit/test.c | 59 ++++++++++++++++++++++++++++++++++---------- > 2 files changed, 97 insertions(+), 13 deletions(-) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index db1b0ae666c4..27b42a008c7a 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -94,6 +94,9 @@ struct kunit; > /* Size of log associated with test. */ > #define KUNIT_LOG_SIZE 512 > > +/* Maximum size of parameter description string. */ > +#define KUNIT_PARAM_DESC_SIZE 128 > + > /* > * TAP specifies subtest stream indentation of 4 spaces, 8 spaces for a > * sub-subtest. See the "Subtests" section in > @@ -107,6 +110,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 +145,7 @@ struct kunit; > struct kunit_case { > void (*run_case)(struct kunit *test); > const char *name; > + const void* (*generate_params)(const void *prev, char *desc); > > /* private: internal use only. */ > bool success; > @@ -163,6 +168,27 @@ 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, char *desc) > + * > + * 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. > + * Optionally write a string into @desc (size of KUNIT_PARAM_DESC_SIZE) > + * describing the parameter. > + */ > +#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 +234,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 +1772,25 @@ 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. > + * @get_desc: function to convert param to description; NULL to use default > + * > + * Define function @name_gen_params which uses @array to generate parameters. > + */ > +#define KUNIT_ARRAY_PARAM(name, array, get_desc) \ > + static const void *name##_gen_params(const void *prev, char *desc) \ > + { \ > + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \ > + if (__next - (array) < ARRAY_SIZE((array))) { \ > + void (*__get_desc)(typeof(__next), char *) = get_desc; \ > + if (__get_desc) \ > + __get_desc(__next, desc); \ > + return __next; \ > + } \ > + return NULL; \ > + } > + > #endif /* _KUNIT_TEST_H */ > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index 750704abe89a..ec9494e914ef 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -325,39 +325,72 @@ 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) > { > + char param_desc[KUNIT_PARAM_DESC_SIZE]; > struct kunit_case *test_case; > > 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) { > + /* Get initial param. */ > + param_desc[0] = '\0'; > + test.param_value = test_case->generate_params(NULL, param_desc); > + } > + > + do { > + kunit_run_case_catch_errors(suite, test_case, &test); > + test_success &= test_case->success; > + > + if (test_case->generate_params) { > + if (param_desc[0] == '\0') { > + snprintf(param_desc, sizeof(param_desc), > + "param-%d", test.param_index); > + } > + > + kunit_log(KERN_INFO, &test, > + KUNIT_SUBTEST_INDENT > + "# %s: %s %d - %s", > + test_case->name, > + kunit_status_to_string(test.success), > + test.param_index + 1, param_desc); > + > + /* Get next param. */ > + param_desc[0] = '\0'; > + test.param_value = test_case->generate_params(test.param_value, param_desc); > + 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); > > -- > 2.25.1 >
On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also > optionally provide a human-readable description of parameters, which is > displayed where available. > > Note, currently the result of each parameter run is displayed in > diagnostic lines, and only the overall test case output summarizes > TAP-compliant success or failure of all parameter runs. In future, when > supported by kunit-tool, these can be turned into subsubtest outputs. > > 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> > --- [Resending this because my email client re-defaulted to HTML! Aarrgh!] This looks good to me! I tested it in UML and x86-64 w/ KASAN, and both worked fine. Reviewed-by: David Gow <davidgow@google.com> Tested-by: David Gow <davidgow@google.com> Thanks for sticking with this! -- David
On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also > > optionally provide a human-readable description of parameters, which is > > displayed where available. > > > > Note, currently the result of each parameter run is displayed in > > diagnostic lines, and only the overall test case output summarizes > > TAP-compliant success or failure of all parameter runs. In future, when > > supported by kunit-tool, these can be turned into subsubtest outputs. > > > > 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> > > --- > [Resending this because my email client re-defaulted to HTML! Aarrgh!] > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > both worked fine. > > Reviewed-by: David Gow <davidgow@google.com> > Tested-by: David Gow <davidgow@google.com> Thank you! > Thanks for sticking with this! Will these patches be landing in 5.11 or 5.12? > -- David Thanks, -- Marco
On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote: > > On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: > > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also > > > optionally provide a human-readable description of parameters, which is > > > displayed where available. > > > > > > Note, currently the result of each parameter run is displayed in > > > diagnostic lines, and only the overall test case output summarizes > > > TAP-compliant success or failure of all parameter runs. In future, when > > > supported by kunit-tool, these can be turned into subsubtest outputs. > > > > > > 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> > > > --- > > [Resending this because my email client re-defaulted to HTML! Aarrgh!] > > > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > > both worked fine. > > > > Reviewed-by: David Gow <davidgow@google.com> > > Tested-by: David Gow <davidgow@google.com> > > Thank you! > > > Thanks for sticking with this! > > Will these patches be landing in 5.11 or 5.12? > I can't think of any reason not to have these in 5.11. We haven't started staging things in the kselftest/kunit branch for 5.11 yet, though. Patch 2 will probably need to be acked by Ted for ext4 first. Brendan, Shuah: can you make sure this doesn't get lost in patchwork? Cheers, -- David > > -- David > > Thanks, > -- Marco
On Tue, 24 Nov 2020 at 08:25, David Gow <davidgow@google.com> wrote: > > On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote: > > > > On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: > > > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also > > > > optionally provide a human-readable description of parameters, which is > > > > displayed where available. > > > > > > > > Note, currently the result of each parameter run is displayed in > > > > diagnostic lines, and only the overall test case output summarizes > > > > TAP-compliant success or failure of all parameter runs. In future, when > > > > supported by kunit-tool, these can be turned into subsubtest outputs. > > > > > > > > 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> > > > > --- > > > [Resending this because my email client re-defaulted to HTML! Aarrgh!] > > > > > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > > > both worked fine. > > > > > > Reviewed-by: David Gow <davidgow@google.com> > > > Tested-by: David Gow <davidgow@google.com> > > > > Thank you! > > > > > Thanks for sticking with this! > > > > Will these patches be landing in 5.11 or 5.12? > > > > I can't think of any reason not to have these in 5.11. We haven't > started staging things in the kselftest/kunit branch for 5.11 yet, > though. > > Patch 2 will probably need to be acked by Ted for ext4 first. Patch 2 had already had 1 Reviewed-by on v3 that got lost. The core bits of that test haven't changed since then, but I can't tell if it needs a re-review. https://lkml.kernel.org/r/CAAXuY3o9Xe-atK0Mja6qXLncUhmmVf4pR7hsANsqaoUX71RXVg@mail.gmail.com Thanks, -- Marco > Brendan, Shuah: can you make sure this doesn't get lost in patchwork? > > Cheers, > -- David > > > > -- David > > > > Thanks, > > -- Marco
On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote: > > On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote: > > > > On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: > > > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also > > > > optionally provide a human-readable description of parameters, which is > > > > displayed where available. > > > > > > > > Note, currently the result of each parameter run is displayed in > > > > diagnostic lines, and only the overall test case output summarizes > > > > TAP-compliant success or failure of all parameter runs. In future, when > > > > supported by kunit-tool, these can be turned into subsubtest outputs. > > > > > > > > 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> > > > > --- > > > [Resending this because my email client re-defaulted to HTML! Aarrgh!] > > > > > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > > > both worked fine. > > > > > > Reviewed-by: David Gow <davidgow@google.com> > > > Tested-by: David Gow <davidgow@google.com> > > > > Thank you! > > > > > Thanks for sticking with this! > > > > Will these patches be landing in 5.11 or 5.12? > > > > I can't think of any reason not to have these in 5.11. We haven't > started staging things in the kselftest/kunit branch for 5.11 yet, > though. > > Patch 2 will probably need to be acked by Ted for ext4 first. > > Brendan, Shuah: can you make sure this doesn't get lost in patchwork? Looks good to me. I would definitely like to pick this up. But yeah, in order to pick up 2/2 we will need an ack from either Ted or Iurii. Ted seems to be busy right now, so I think I will just ask Shuah to go ahead and pick this patch up by itself and we or Ted can pick up patch 2/2 later. Cheers
On 11/30/20 3:22 PM, Brendan Higgins wrote: > On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote: >> >> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote: >>> >>> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: >>>> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also >>>>> optionally provide a human-readable description of parameters, which is >>>>> displayed where available. >>>>> >>>>> Note, currently the result of each parameter run is displayed in >>>>> diagnostic lines, and only the overall test case output summarizes >>>>> TAP-compliant success or failure of all parameter runs. In future, when >>>>> supported by kunit-tool, these can be turned into subsubtest outputs. >>>>> >>>>> 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> >>>>> --- >>>> [Resending this because my email client re-defaulted to HTML! Aarrgh!] >>>> >>>> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and >>>> both worked fine. >>>> >>>> Reviewed-by: David Gow <davidgow@google.com> >>>> Tested-by: David Gow <davidgow@google.com> >>> >>> Thank you! >>> >>>> Thanks for sticking with this! >>> >>> Will these patches be landing in 5.11 or 5.12? >>> >> >> I can't think of any reason not to have these in 5.11. We haven't >> started staging things in the kselftest/kunit branch for 5.11 yet, >> though. >> >> Patch 2 will probably need to be acked by Ted for ext4 first. >> >> Brendan, Shuah: can you make sure this doesn't get lost in patchwork? > > Looks good to me. I would definitely like to pick this up. But yeah, > in order to pick up 2/2 we will need an ack from either Ted or Iurii. > > Ted seems to be busy right now, so I think I will just ask Shuah to go > ahead and pick this patch up by itself and we or Ted can pick up patch > 2/2 later. > > Cheers > I am seeing ERROR: need consistent spacing around '*' (ctx:WxV) #272: FILE: include/kunit/test.h:1786: + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \ ^ Can you look into this and send v10? thanks, -- Shuah
On Tue, 1 Dec 2020 at 23:28, Shuah Khan <skhan@linuxfoundation.org> wrote: > > On 11/30/20 3:22 PM, Brendan Higgins wrote: > > On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote: > >> > >> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote: > >>> > >>> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: > >>>> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also > >>>>> optionally provide a human-readable description of parameters, which is > >>>>> displayed where available. > >>>>> > >>>>> Note, currently the result of each parameter run is displayed in > >>>>> diagnostic lines, and only the overall test case output summarizes > >>>>> TAP-compliant success or failure of all parameter runs. In future, when > >>>>> supported by kunit-tool, these can be turned into subsubtest outputs. > >>>>> > >>>>> 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> > >>>>> --- > >>>> [Resending this because my email client re-defaulted to HTML! Aarrgh!] > >>>> > >>>> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > >>>> both worked fine. > >>>> > >>>> Reviewed-by: David Gow <davidgow@google.com> > >>>> Tested-by: David Gow <davidgow@google.com> > >>> > >>> Thank you! > >>> > >>>> Thanks for sticking with this! > >>> > >>> Will these patches be landing in 5.11 or 5.12? > >>> > >> > >> I can't think of any reason not to have these in 5.11. We haven't > >> started staging things in the kselftest/kunit branch for 5.11 yet, > >> though. > >> > >> Patch 2 will probably need to be acked by Ted for ext4 first. > >> > >> Brendan, Shuah: can you make sure this doesn't get lost in patchwork? > > > > Looks good to me. I would definitely like to pick this up. But yeah, > > in order to pick up 2/2 we will need an ack from either Ted or Iurii. > > > > Ted seems to be busy right now, so I think I will just ask Shuah to go > > ahead and pick this patch up by itself and we or Ted can pick up patch > > 2/2 later. > > > > Cheers > > > > I am seeing > > ERROR: need consistent spacing around '*' (ctx:WxV) > #272: FILE: include/kunit/test.h:1786: > + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : > (array); \ > ^ > > Can you look into this and send v10? This is a false positive. I pointed this out here before: https://lkml.kernel.org/r/CANpmjNNhpe6TYt0KmBCCR-Wfz1Bxd8qnhiwegwnDQsxRAWmUMg@mail.gmail.com checkpatch.pl thinks this is a multiplication, but this is a pointer, so the spacing here is correct. Thanks, -- Marco > thanks, > -- Shuah
On Mon, Nov 30, 2020 at 02:22:22PM -0800, 'Brendan Higgins' via KUnit Development wrote: > > Looks good to me. I would definitely like to pick this up. But yeah, > in order to pick up 2/2 we will need an ack from either Ted or Iurii. > > Ted seems to be busy right now, so I think I will just ask Shuah to go > ahead and pick this patch up by itself and we or Ted can pick up patch > 2/2 later. I have been paying attention to this patch series, but I had presumed that this was much more of a kunit change than an ext4 change, and the critical bits was a review of the kunit infrastructure. I certainly have no objection to changing the ext4 test to use the new parameterized testing, and if you'd like me to give a quick review, I'll take a quick look. I assume, Brendan, that you've already tried doing a compile and run test of the patch series, so I'm not going to do that? - Ted
On 12/1/20 4:31 PM, Marco Elver wrote: > On Tue, 1 Dec 2020 at 23:28, Shuah Khan <skhan@linuxfoundation.org> wrote: >> >> On 11/30/20 3:22 PM, Brendan Higgins wrote: >>> On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote: >>>> >>>> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote: >>>>> >>>>> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote: >>>>>> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also >>>>>>> optionally provide a human-readable description of parameters, which is >>>>>>> displayed where available. >>>>>>> >>>>>>> Note, currently the result of each parameter run is displayed in >>>>>>> diagnostic lines, and only the overall test case output summarizes >>>>>>> TAP-compliant success or failure of all parameter runs. In future, when >>>>>>> supported by kunit-tool, these can be turned into subsubtest outputs. >>>>>>> >>>>>>> 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> >>>>>>> --- >>>>>> [Resending this because my email client re-defaulted to HTML! Aarrgh!] >>>>>> >>>>>> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and >>>>>> both worked fine. >>>>>> >>>>>> Reviewed-by: David Gow <davidgow@google.com> >>>>>> Tested-by: David Gow <davidgow@google.com> >>>>> >>>>> Thank you! >>>>> >>>>>> Thanks for sticking with this! >>>>> >>>>> Will these patches be landing in 5.11 or 5.12? >>>>> >>>> >>>> I can't think of any reason not to have these in 5.11. We haven't >>>> started staging things in the kselftest/kunit branch for 5.11 yet, >>>> though. >>>> >>>> Patch 2 will probably need to be acked by Ted for ext4 first. >>>> >>>> Brendan, Shuah: can you make sure this doesn't get lost in patchwork? >>> >>> Looks good to me. I would definitely like to pick this up. But yeah, >>> in order to pick up 2/2 we will need an ack from either Ted or Iurii. >>> >>> Ted seems to be busy right now, so I think I will just ask Shuah to go >>> ahead and pick this patch up by itself and we or Ted can pick up patch >>> 2/2 later. >>> >>> Cheers >>> >> >> I am seeing >> >> ERROR: need consistent spacing around '*' (ctx:WxV) >> #272: FILE: include/kunit/test.h:1786: >> + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : >> (array); \ >> ^ >> >> Can you look into this and send v10? > > This is a false positive. I pointed this out here before: > https://lkml.kernel.org/r/CANpmjNNhpe6TYt0KmBCCR-Wfz1Bxd8qnhiwegwnDQsxRAWmUMg@mail.gmail.com > > checkpatch.pl thinks this is a multiplication, but this is a pointer, > so the spacing here is correct. > Thank you for confirming. I will apply this. thanks, -- Shuah
diff --git a/include/kunit/test.h b/include/kunit/test.h index db1b0ae666c4..27b42a008c7a 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -94,6 +94,9 @@ struct kunit; /* Size of log associated with test. */ #define KUNIT_LOG_SIZE 512 +/* Maximum size of parameter description string. */ +#define KUNIT_PARAM_DESC_SIZE 128 + /* * TAP specifies subtest stream indentation of 4 spaces, 8 spaces for a * sub-subtest. See the "Subtests" section in @@ -107,6 +110,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 +145,7 @@ struct kunit; struct kunit_case { void (*run_case)(struct kunit *test); const char *name; + const void* (*generate_params)(const void *prev, char *desc); /* private: internal use only. */ bool success; @@ -163,6 +168,27 @@ 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, char *desc) + * + * 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. + * Optionally write a string into @desc (size of KUNIT_PARAM_DESC_SIZE) + * describing the parameter. + */ +#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 +234,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 +1772,25 @@ 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. + * @get_desc: function to convert param to description; NULL to use default + * + * Define function @name_gen_params which uses @array to generate parameters. + */ +#define KUNIT_ARRAY_PARAM(name, array, get_desc) \ + static const void *name##_gen_params(const void *prev, char *desc) \ + { \ + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \ + if (__next - (array) < ARRAY_SIZE((array))) { \ + void (*__get_desc)(typeof(__next), char *) = get_desc; \ + if (__get_desc) \ + __get_desc(__next, desc); \ + return __next; \ + } \ + return NULL; \ + } + #endif /* _KUNIT_TEST_H */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 750704abe89a..ec9494e914ef 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -325,39 +325,72 @@ 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) { + char param_desc[KUNIT_PARAM_DESC_SIZE]; struct kunit_case *test_case; 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) { + /* Get initial param. */ + param_desc[0] = '\0'; + test.param_value = test_case->generate_params(NULL, param_desc); + } + + do { + kunit_run_case_catch_errors(suite, test_case, &test); + test_success &= test_case->success; + + if (test_case->generate_params) { + if (param_desc[0] == '\0') { + snprintf(param_desc, sizeof(param_desc), + "param-%d", test.param_index); + } + + kunit_log(KERN_INFO, &test, + KUNIT_SUBTEST_INDENT + "# %s: %s %d - %s", + test_case->name, + kunit_status_to_string(test.success), + test.param_index + 1, param_desc); + + /* Get next param. */ + param_desc[0] = '\0'; + test.param_value = test_case->generate_params(test.param_value, param_desc); + 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);