Due to the lack of the SKIP directive in the output, if any of the parameterized test was skipped, the parser could not recognize that correctly and was marking the test as PASSED.
This can easily be seen by running the new subtest from patch 1:
$ ./tools/testing/kunit/kunit.py run \ --kunitconfig ./lib/kunit/.kunitconfig *.example_params*
[ ] Starting KUnit Kernel (1/1)... [ ] ============================================================ [ ] =================== example (1 subtest) ==================== [ ] =================== example_params_test =================== [ ] [PASSED] example value 2 [ ] [PASSED] example value 1 [ ] [PASSED] example value 0 [ ] =============== [PASSED] example_params_test =============== [ ] ===================== [PASSED] example ===================== [ ] ============================================================ [ ] Testing complete. Ran 3 tests: passed: 3
$ ./tools/testing/kunit/kunit.py run \ --kunitconfig ./lib/kunit/.kunitconfig *.example_params* \ --raw_output
[ ] Starting KUnit Kernel (1/1)... KTAP version 1 1..1 # example: initializing suite KTAP version 1 # Subtest: example 1..1 KTAP version 1 # Subtest: example_params_test # example_params_test: initializing ok 1 example value 2 # example_params_test: initializing ok 2 example value 1 # example_params_test: initializing ok 3 example value 0 # example_params_test: pass:2 fail:0 skip:1 total:3 ok 1 example_params_test # Totals: pass:2 fail:0 skip:1 total:3 ok 1 example
After adding the SKIP directive, the report looks as expected:
[ ] Starting KUnit Kernel (1/1)... [ ] ============================================================ [ ] =================== example (1 subtest) ==================== [ ] =================== example_params_test =================== [ ] [PASSED] example value 2 [ ] [PASSED] example value 1 [ ] [SKIPPED] example value 0 [ ] =============== [PASSED] example_params_test =============== [ ] ===================== [PASSED] example ===================== [ ] ============================================================ [ ] Testing complete. Ran 3 tests: passed: 2, skipped: 1
[ ] Starting KUnit Kernel (1/1)... KTAP version 1 1..1 # example: initializing suite KTAP version 1 # Subtest: example 1..1 KTAP version 1 # Subtest: example_params_test # example_params_test: initializing ok 1 example value 2 # example_params_test: initializing ok 2 example value 1 # example_params_test: initializing ok 3 example value 0 # SKIP unsupported param value # example_params_test: pass:2 fail:0 skip:1 total:3 ok 1 example_params_test # Totals: pass:2 fail:0 skip:1 total:3 ok 1 example
v2: better align with future support for arbitrary levels of testing v3: rebased on kunit tree [1]
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/lo...
Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
Michal Wajdeczko (3): kunit/test: Add example test showing parameterized testing kunit: Fix reporting of the skipped parameterized tests kunit: Update kunit_print_ok_not_ok function
include/kunit/test.h | 1 + lib/kunit/kunit-example-test.c | 34 +++++++++++++++++++++++++++ lib/kunit/test.c | 43 ++++++++++++++++++++++------------ 3 files changed, 63 insertions(+), 15 deletions(-)
Use of parameterized testing is documented [1] but such use case is not present in demo kunit test. Add small subtest for that.
[1] https://kernel.org/doc/html/latest/dev-tools/kunit/usage.html#parameterized-...
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Reviewed-by: David Gow davidgow@google.com --- lib/kunit/kunit-example-test.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c index 24315c882b31..b69b689ea850 100644 --- a/lib/kunit/kunit-example-test.c +++ b/lib/kunit/kunit-example-test.c @@ -187,6 +187,39 @@ static void example_static_stub_test(struct kunit *test) KUNIT_EXPECT_EQ(test, add_one(1), 2); }
+static const struct example_param { + int value; +} example_params_array[] = { + { .value = 2, }, + { .value = 1, }, + { .value = 0, }, +}; + +static void example_param_get_desc(const struct example_param *p, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "example value %d", p->value); +} + +KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc); + +/* + * This test shows the use of params. + */ +static void example_params_test(struct kunit *test) +{ + const struct example_param *param = test->param_value; + + /* By design, param pointer will not be NULL */ + KUNIT_ASSERT_NOT_NULL(test, param); + + /* Test can be skipped on unsupported param values */ + if (!param->value) + kunit_skip(test, "unsupported param value"); + + /* You can use param values for parameterized testing */ + KUNIT_EXPECT_EQ(test, param->value % param->value, 0); +} + /* * Here we make a list of all the test cases we want to add to the test suite * below. @@ -203,6 +236,7 @@ static struct kunit_case example_test_cases[] = { KUNIT_CASE(example_mark_skipped_test), KUNIT_CASE(example_all_expect_macros_test), KUNIT_CASE(example_static_stub_test), + KUNIT_CASE_PARAM(example_params_test, example_gen_params), {} };
Logs from the parameterized tests that were skipped don't include SKIP directive thus they are displayed as PASSED. Fix that.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Reviewed-by: Rae Moar rmoar@google.com Reviewed-by: David Gow davidgow@google.com --- lib/kunit/test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index f5e4ceffd282..af48d0761d26 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -627,9 +627,11 @@ int kunit_run_tests(struct kunit_suite *suite)
kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT - "%s %d %s", + "%s %d %s%s%s", kunit_status_to_ok_not_ok(test.status), - test.param_index + 1, param_desc); + test.param_index + 1, param_desc, + test.status == KUNIT_SKIPPED ? " # SKIP " : "", + test.status == KUNIT_SKIPPED ? test.status_comment : "");
/* Get next param. */ param_desc[0] = '\0';
There is no need use opaque test_or_suite pointer and is_test flag as we don't use anything from the suite struct. Always expect test pointer and use NULL as indication that provided results are from the suite so we can treat them differently.
Since results could be from nested tests, like parameterized tests, add explicit level parameter to properly indent output messages and thus allow to reuse this function from other places.
While around, remove small code duplication near skip directive.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com --- include/kunit/test.h | 1 + lib/kunit/test.c | 45 +++++++++++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 3028a1a3fcad..d717fc055e06 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -47,6 +47,7 @@ struct kunit; * sub-subtest. See the "Subtests" section in * https://node-tap.org/tap-protocol/ */ +#define KUNIT_INDENT_LEN 4 #define KUNIT_SUBTEST_INDENT " " #define KUNIT_SUBSUBTEST_INDENT " "
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index af48d0761d26..6bc92ced82ee 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -185,16 +185,28 @@ static void kunit_print_suite_start(struct kunit_suite *suite) kunit_suite_num_test_cases(suite)); }
-static void kunit_print_ok_not_ok(void *test_or_suite, - bool is_test, +/* Currently supported test levels */ +enum { + KUNIT_LEVEL_SUITE = 0, + KUNIT_LEVEL_CASE, + KUNIT_LEVEL_CASE_PARAM, +}; + +static void kunit_print_ok_not_ok(struct kunit *test, + unsigned int test_level, enum kunit_status status, size_t test_number, const char *description, const char *directive) { - struct kunit_suite *suite = is_test ? NULL : test_or_suite; - struct kunit *test = is_test ? test_or_suite : NULL; const char *directive_header = (status == KUNIT_SKIPPED) ? " # SKIP " : ""; + const char *directive_body = (status == KUNIT_SKIPPED) ? directive : ""; + + /* + * When test is NULL assume that results are from the suite + * and today suite results are expected at level 0 only. + */ + WARN(!test && test_level, "suite test level can't be %u!\n", test_level);
/* * We do not log the test suite results as doing so would @@ -203,17 +215,18 @@ static void kunit_print_ok_not_ok(void *test_or_suite, * separately seq_printf() the suite results for the debugfs * representation. */ - if (suite) + if (!test) pr_info("%s %zd %s%s%s\n", kunit_status_to_ok_not_ok(status), test_number, description, directive_header, - (status == KUNIT_SKIPPED) ? directive : ""); + directive_body); else kunit_log(KERN_INFO, test, - KUNIT_SUBTEST_INDENT "%s %zd %s%s%s", + "%*s%s %zd %s%s%s", + KUNIT_INDENT_LEN * test_level, "", kunit_status_to_ok_not_ok(status), test_number, description, directive_header, - (status == KUNIT_SKIPPED) ? directive : ""); + directive_body); }
enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite) @@ -239,7 +252,7 @@ static size_t kunit_suite_counter = 1;
static void kunit_print_suite_end(struct kunit_suite *suite) { - kunit_print_ok_not_ok((void *)suite, false, + kunit_print_ok_not_ok(NULL, KUNIT_LEVEL_SUITE, kunit_suite_has_succeeded(suite), kunit_suite_counter++, suite->name, @@ -625,13 +638,11 @@ int kunit_run_tests(struct kunit_suite *suite) "param-%d", test.param_index); }
- kunit_log(KERN_INFO, &test, - KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT - "%s %d %s%s%s", - kunit_status_to_ok_not_ok(test.status), - test.param_index + 1, param_desc, - test.status == KUNIT_SKIPPED ? " # SKIP " : "", - test.status == KUNIT_SKIPPED ? test.status_comment : ""); + kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE_PARAM, + test.status, + test.param_index + 1, + param_desc, + test.status_comment);
/* Get next param. */ param_desc[0] = '\0'; @@ -645,7 +656,7 @@ int kunit_run_tests(struct kunit_suite *suite)
kunit_print_test_stats(&test, param_stats);
- kunit_print_ok_not_ok(&test, true, test_case->status, + kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status, kunit_test_case_num(suite, test_case), test_case->name, test.status_comment);
On Wed, 17 May 2023 at 19:20, Michal Wajdeczko michal.wajdeczko@intel.com wrote:
There is no need use opaque test_or_suite pointer and is_test flag as we don't use anything from the suite struct. Always expect test pointer and use NULL as indication that provided results are from the suite so we can treat them differently.
Since results could be from nested tests, like parameterized tests, add explicit level parameter to properly indent output messages and thus allow to reuse this function from other places.
While around, remove small code duplication near skip directive.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
This looks good to me, thanks.
As a note, this seems to trigger a bug in checkpatch.pl: substr outside of string at ./scripts/checkpatch.pl line 1664. Use of uninitialized value $fmt in substitution (s///) at ./scripts/checkpatch.pl line 6899. Use of uninitialized value $fmt in pattern match (m//) at ./scripts/checkpatch.pl line 6901.
I haven't dug into that myself, yet (it's something to do with format strings), but I don't think we need to change this patch to work around it.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
include/kunit/test.h | 1 + lib/kunit/test.c | 45 +++++++++++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 3028a1a3fcad..d717fc055e06 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -47,6 +47,7 @@ struct kunit;
- sub-subtest. See the "Subtests" section in
- https://node-tap.org/tap-protocol/
*/ +#define KUNIT_INDENT_LEN 4 #define KUNIT_SUBTEST_INDENT " " #define KUNIT_SUBSUBTEST_INDENT " "
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index af48d0761d26..6bc92ced82ee 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -185,16 +185,28 @@ static void kunit_print_suite_start(struct kunit_suite *suite) kunit_suite_num_test_cases(suite)); }
-static void kunit_print_ok_not_ok(void *test_or_suite,
bool is_test,+/* Currently supported test levels */ +enum {
KUNIT_LEVEL_SUITE = 0,KUNIT_LEVEL_CASE,KUNIT_LEVEL_CASE_PARAM,+};
+static void kunit_print_ok_not_ok(struct kunit *test,
unsigned int test_level, enum kunit_status status, size_t test_number, const char *description, const char *directive){
struct kunit_suite *suite = is_test ? NULL : test_or_suite;struct kunit *test = is_test ? test_or_suite : NULL; const char *directive_header = (status == KUNIT_SKIPPED) ? " # SKIP " : "";
const char *directive_body = (status == KUNIT_SKIPPED) ? directive : "";/** When test is NULL assume that results are from the suite* and today suite results are expected at level 0 only.*/WARN(!test && test_level, "suite test level can't be %u!\n", test_level); /* * We do not log the test suite results as doing so would@@ -203,17 +215,18 @@ static void kunit_print_ok_not_ok(void *test_or_suite, * separately seq_printf() the suite results for the debugfs * representation. */
if (suite)
if (!test) pr_info("%s %zd %s%s%s\n", kunit_status_to_ok_not_ok(status), test_number, description, directive_header,
(status == KUNIT_SKIPPED) ? directive : "");
directive_body); else kunit_log(KERN_INFO, test,
KUNIT_SUBTEST_INDENT "%s %zd %s%s%s",
"%*s%s %zd %s%s%s",KUNIT_INDENT_LEN * test_level, "", kunit_status_to_ok_not_ok(status), test_number, description, directive_header,
(status == KUNIT_SKIPPED) ? directive : "");
directive_body);}
enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite) @@ -239,7 +252,7 @@ static size_t kunit_suite_counter = 1;
static void kunit_print_suite_end(struct kunit_suite *suite) {
kunit_print_ok_not_ok((void *)suite, false,
kunit_print_ok_not_ok(NULL, KUNIT_LEVEL_SUITE, kunit_suite_has_succeeded(suite), kunit_suite_counter++, suite->name,@@ -625,13 +638,11 @@ int kunit_run_tests(struct kunit_suite *suite) "param-%d", test.param_index); }
kunit_log(KERN_INFO, &test,KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT"%s %d %s%s%s",kunit_status_to_ok_not_ok(test.status),test.param_index + 1, param_desc,test.status == KUNIT_SKIPPED ? " # SKIP " : "",test.status == KUNIT_SKIPPED ? test.status_comment : "");
kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE_PARAM,test.status,test.param_index + 1,param_desc,test.status_comment); /* Get next param. */ param_desc[0] = '\0';@@ -645,7 +656,7 @@ int kunit_run_tests(struct kunit_suite *suite)
kunit_print_test_stats(&test, param_stats);
kunit_print_ok_not_ok(&test, true, test_case->status,
kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status, kunit_test_case_num(suite, test_case), test_case->name, test.status_comment);-- 2.25.1
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230517111816.984-4-michal.wajd....
linux-kselftest-mirror@lists.linaro.org
-
David Gow -
Michal Wajdeczko