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).
> /*
> * 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'.
> + 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
