On Sun, 15 Nov 2020 at 13:18, Arpitha Raghunandan 98.arpi@gmail.com wrote:
On 15/11/20 2:28 pm, Marco Elver wrote:
On Sat, 14 Nov 2020 at 13:38, 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
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().
Thanks. There's also a report by the test robot now which noticed this.
/*
- 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.
I think checkpatch.pl thinks this is a multiplication. It's definitely a false positive. Please do format it like a normal pointer.
Thanks, -- Marco