On 28.09.2023 22:53, Rae Moar wrote:
On Mon, Sep 25, 2023 at 1:58 PM Michal Wajdeczko michal.wajdeczko@intel.com wrote:
When running parametrized test cases, diagnostic messages are not properly aligned with the test result lines:
$ ./tools/testing/kunit/kunit.py run --raw_output \ --kunitconfig ./lib/kunit/.kunitconfig *.example_params* KTAP version 1 1..1 # example: initializing suite KTAP version 1 # Subtest: example # module: kunit_example_test 1..1 KTAP version 1 # Subtest: example_params_test # example_params_test: initializing # example_params_test: cleaning up ok 1 example value 3 # SKIP unsupported param value 3 # example_params_test: initializing # example_params_test: cleaning up ok 2 example value 2 # example_params_test: initializing # example_params_test: cleaning up ok 3 example value 1 # example_params_test: initializing # example_params_test: cleaning up ok 4 example value 0 # SKIP unsupported param value 0 # example_params_test: pass:2 fail:0 skip:2 total:4 ok 1 example_params_test # example: exiting suite # Totals: pass:2 fail:0 skip:2 total:4 ok 1 exampleAdd test level attribute and use it to generate proper indent at the runtime:
KTAP version 1 1..1 # example: initializing suite KTAP version 1 # Subtest: example # module: kunit_example_test 1..1 KTAP version 1 # Subtest: example_params_test # example_params_test: initializing # example_params_test: cleaning up ok 1 example value 3 # SKIP unsupported param value 3 # example_params_test: initializing # example_params_test: cleaning up ok 2 example value 2 # example_params_test: initializing # example_params_test: cleaning up ok 3 example value 1 # example_params_test: initializing # example_params_test: cleaning up ok 4 example value 0 # SKIP unsupported param value 0 # example_params_test: pass:2 fail:0 skip:2 total:4 ok 1 example_params_test # example: exiting suite # Totals: pass:2 fail:0 skip:2 total:4 ok 1 exampleSigned-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
Hello!
Great to see these changes! Just a few comments below.
Thanks! -Rae
include/kunit/test.h | 3 ++- lib/kunit/test.c | 52 ++++++++++++++++++++------------------------ 2 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 158876c4cb43..4804d539e10f 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -276,6 +276,7 @@ struct kunit { void *priv;
/* private: internal use only. */
unsigned int level; /* Helps in proper log indent */ const char *name; /* Read only after initialization! */ struct string_stream *log; /* Points at case log after initialization */ struct kunit_try_catch try_catch;@@ -519,7 +520,7 @@ enum { #define kunit_level(test_or_suite) \ _Generic((test_or_suite), \ struct kunit_suite * : KUNIT_LEVEL_SUITE, \
struct kunit * : KUNIT_LEVEL_CASE)
struct kunit * : ((struct kunit *)(test_or_suite))->level)#define kunit_indent_level(test_or_suite) \ (KUNIT_INDENT_LEN * kunit_level(test_or_suite)) diff --git a/lib/kunit/test.c b/lib/kunit/test.c index d10e6d610e20..43c3efc286e4 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -99,14 +99,13 @@ static void kunit_print_test_stats(struct kunit *test, if (!kunit_should_print_stats(stats)) return;
kunit_log(KERN_INFO, test,KUNIT_SUBTEST_INDENT"# %s: pass:%lu fail:%lu skip:%lu total:%lu",test->name,stats.passed,stats.failed,stats.skipped,stats.total);
kunit_log_indent(KERN_INFO, test,"# %s: pass:%lu fail:%lu skip:%lu total:%lu",test->name,stats.passed,stats.failed,stats.skipped,stats.total);I would prefer if we keep the same indentation level as before.
note that then scripts/checkpatch.pl will complain with:
CHECK: Alignment should match open parenthesis #109: FILE: lib/kunit/test.c:103: + kunit_log_indent(KERN_INFO, test, "# %s: pass:%lu fail:%lu skip:%lu total:%lu",
CHECK: Alignment should match open parenthesis #141: FILE: lib/kunit/test.c:178: + kunit_log_indent(KERN_INFO, test, + "%s %zd %s%s%s",
are you ok with that?
Michal
Otherwise looks good.
}
/* Append formatted message to log. */ @@ -154,7 +153,6 @@ static void kunit_print_suite_start(struct kunit_suite *suite) }
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,@@ -163,12 +161,6 @@ static void kunit_print_ok_not_ok(struct kunit *test, 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 * mean debugfs display would consist of an incorrect test@@ -182,12 +174,11 @@ static void kunit_print_ok_not_ok(struct kunit *test, test_number, description, directive_header, directive_body); else
kunit_log(KERN_INFO, test,"%*s%s %zd %s%s%s",KUNIT_INDENT_LEN * test_level, "",kunit_status_to_ok_not_ok(status),test_number, description, directive_header,directive_body);
kunit_log_indent(KERN_INFO, test,"%s %zd %s%s%s",kunit_status_to_ok_not_ok(status),test_number, description, directive_header,directive_body);Again, I would prefer we keep the same indentation as before as it matches the rest of the file.
}
enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite) @@ -213,7 +204,7 @@ static size_t kunit_suite_counter = 1;
static void kunit_print_suite_end(struct kunit_suite *suite) {
kunit_print_ok_not_ok(NULL, KUNIT_LEVEL_SUITE,
kunit_print_ok_not_ok(NULL, kunit_suite_has_succeeded(suite), kunit_suite_counter++, suite->name,@@ -322,6 +313,7 @@ void kunit_init_test(struct kunit *test, const char *name, struct string_stream { spin_lock_init(&test->lock); INIT_LIST_HEAD(&test->resources);
test->level = KUNIT_LEVEL_CASE; test->name = name; test->log = log; if (test->log)@@ -584,14 +576,15 @@ int kunit_run_tests(struct kunit_suite *suite) kunit_run_case_catch_errors(suite, test_case, &test); kunit_update_stats(¶m_stats, test.status); } else {
/* Parameterized test is one level up from simple test-case. */test.level++;/* Get initial param. */ param_desc[0] = '\0'; test.param_value = test_case->generate_params(NULL, param_desc); test_case->status = KUNIT_SKIPPED;
kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT"KTAP version 1\n");kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT"# Subtest: %s", test_case->name);
kunit_log_indent(KERN_INFO, &test, "KTAP version 1\n");kunit_log_indent(KERN_INFO, &test, "# Subtest: %s", test_case->name); while (test.param_value) { kunit_run_case_catch_errors(suite, test_case, &test);@@ -601,7 +594,7 @@ int kunit_run_tests(struct kunit_suite *suite) "param-%d", test.param_index); }
kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE_PARAM,
kunit_print_ok_not_ok(&test, test.status, test.param_index + 1, param_desc,@@ -616,13 +609,16 @@ int kunit_run_tests(struct kunit_suite *suite) test.status = KUNIT_SUCCESS; test.status_comment[0] = '\0'; }
/* Return to parent (test-case) level. */test.level--; } kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE); kunit_print_test_stats(&test, param_stats);
kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status,
kunit_print_ok_not_ok(&test, test_case->status, kunit_test_case_num(suite, test_case), test_case->name, test.status_comment);-- 2.25.1