| Message ID | 20230414152737.1630-4-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: Fix reporting of the skipped parameterized tests | expand |
On Fri, 14 Apr 2023 at 23:28, 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> > --- From a quick glance, this looks pretty good to me, thanks. It'll need rebasing on top of the kselftest/kunit[1] tree: there are some conflicts with the debugfs fixes. I can do that if you'd prefer: I'll need to do so before giving it a more thorough review next week. Cheers, -- David [1]: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=kunit > 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 08d3559dd703..5e5af167e7f8 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 5679197b5f8a..ca636c9f793c 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -154,16 +154,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 > @@ -173,17 +185,18 @@ static void kunit_print_ok_not_ok(void *test_or_suite, > * separately seq_printf() the suite status 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) > @@ -209,7 +222,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, > @@ -554,13 +567,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'; > @@ -574,7 +585,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 >
diff --git a/include/kunit/test.h b/include/kunit/test.h index 08d3559dd703..5e5af167e7f8 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 5679197b5f8a..ca636c9f793c 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -154,16 +154,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 @@ -173,17 +185,18 @@ static void kunit_print_ok_not_ok(void *test_or_suite, * separately seq_printf() the suite status 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) @@ -209,7 +222,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, @@ -554,13 +567,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'; @@ -574,7 +585,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);
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(-)