On Fri, 14 Apr 2023 at 23:28, Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> 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@xxxxxxxxx> > Cc: David Gow <davidgow@xxxxxxxxxx> > Cc: Rae Moar <rmoar@xxxxxxxxxx> > --- >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 >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
