On 15/11/20 2:28 pm, Marco Elver wrote:
> On Sat, 14 Nov 2020 at 13:38, Arpitha Raghunandan <98.arpi@xxxxxxxxx> 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@xxxxxxxxx>
>> Co-developed-by: Marco Elver <elver@xxxxxxxxxx>
>> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
>> ---
>> 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..cf5f33b1c890 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 64
>
> I think we need to make this larger, perhaps 128. I just noticed a few
> of the inode-test strings are >64 chars (and it should probably also
> use strncpy() to copy to description, which is my bad).
>
Okay, I will make the description size larger and use strncpy().
>> /*
>> * 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;
> [...]
>> +/**
>> + * 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); \
>
> Why did you reintroduce a space between * and __next? AFAIK, this
> should follow the same style as the rest of the kernel, and it should
> just be 'thetype *ptr'.
>
I introduced this space because checkpatch.pl gave an error without the space:
ERROR: need consistent spacing around '*' (ctx:WxV)
#1786: FILE: ./include/kunit/test.h:1786:
+ typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \
But, if this is a mistake as it doesn't recognize __next to be a pointer, I will remove the space.
>> + 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; \
>> + }
>> +
>
> Thanks,
> -- Marco
>
Thanks!
