Re: [PATCH 1/2] kunit: Add ability to skip entire test suites
From: Vaibhav Jain <hidden>
Date: 2026-06-08 09:04:33
Also in:
kvm, linux-kselftest, lkml
Hi David, Thanks for looking into this patch. My responses inline below: David Gow [off-list ref] writes: <snip>
Thanks — this is great! There are a few cases it's not handling properly, though, particularly with respect to the debugfs support. I think we need to: - Reset suite->status to KUNIT_SUCCESS in kunit_init_suite(), so that a suite which is re-run via debugfs isn't automatically skipped again. - Fix the result handling in debugfs to handle skipped suites.
Agree, and I have addressed this in v2 of the patch using the code-snippets you have shared.
- (Optional) Maybe we could get rid of kunit_suite::suite_init_err now that there's a specific status value. That'd have to be done carefully to preserve all of the semantics, though.
Yes fully agree that kunit_suite::suite_init_err will be redundant after
introduction of kunit_suite::status. I have started looking into this
and will post a different patch{series} for this.
quoted hunk ↗ jump to hunk
Here's a quick (and slightly hacky) patch to fix the first couple of issues: ---diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index 9c326f1837bd..23d34bfdba95 100644 --- a/lib/kunit/debugfs.c +++ b/lib/kunit/debugfs.c@@ -58,6 +58,22 @@ static void debugfs_print_result(struct seq_file*seq, struct string_stream *log spin_unlock(&log->lock); } +/* Print the result line for a suite. */ +static void debugfs_print_ok_not_ok(struct seq_file *seq, + enum kunit_status status, + size_t test_number, + const char *description, + const char *directive) +{ + const char *directive_header = (status == KUNIT_SKIPPED) ? " # SKIP " : ""; + const char *directive_body = (status == KUNIT_SKIPPED) ? directive : ""; + + seq_printf(seq, "%s %zd %s%s%s\n", + kunit_status_to_ok_not_ok(status), + test_number, description, directive_header, + directive_body); +} + /* * /sys/kernel/debug/kunit/<testsuite>/results shows all results for testsuite. */@@ -77,17 +93,17 @@ static int debugfs_print_results(struct seq_file*seq, void *v) seq_puts(seq, "1..1\n"); /* Print suite header because it is not stored in the test logs. */ - seq_puts(seq, KUNIT_SUBTEST_INDENT "KTAP version 1\n"); - seq_printf(seq, KUNIT_SUBTEST_INDENT "# Subtest: %s\n", suite->name); - seq_printf(seq, KUNIT_SUBTEST_INDENT "1..%zd\n", kunit_suite_num_test_cases(suite)); - - kunit_suite_for_each_test_case(suite, test_case) - debugfs_print_result(seq, test_case->log); + if (suite->status != KUNIT_SKIPPED) { + seq_puts(seq, KUNIT_SUBTEST_INDENT "KTAP version 1\n"); + seq_printf(seq, KUNIT_SUBTEST_INDENT "# Subtest: %s\n", suite->name); + seq_printf(seq, KUNIT_SUBTEST_INDENT "1..%zd\n", kunit_suite_num_test_cases(suite)); + kunit_suite_for_each_test_case(suite, test_case) + debugfs_print_result(seq, test_case->log); + } debugfs_print_result(seq, suite->log); - seq_printf(seq, "%s %d %s\n", - kunit_status_to_ok_not_ok(success), 1, suite->name); + debugfs_print_ok_not_ok(seq, success, 1, suite->name, suite->status_comment); return 0; }
Thanks, have modified these proposed changes a bit for v2
quoted hunk ↗ jump to hunk
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index c0ae45a22b2c..2ff145796450 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c@@ -798,9 +798,6 @@ int kunit_run_tests(struct kunit_suite *suite) /* Taint the kernel so we know we've run tests. */ add_taint(TAINT_TEST, LOCKDEP_STILL_OK); - if (suite->status == KUNIT_SKIPPED) - goto suite_end; - if (suite->suite_init) { suite->suite_init_err = suite->suite_init(suite); if (suite->suite_init_err) {@@ -836,6 +833,7 @@ static void kunit_init_suite(struct kunit_suite *suite) kunit_debugfs_create_suite(suite); suite->status_comment[0] = '\0'; suite->suite_init_err = 0; + suite->status = KUNIT_SUCCESS;
Thanks, taken this change for v2.
if (suite->log) string_stream_clear(suite->log); ---quoted
include/kunit/test.h | 1 + lib/kunit/test.c | 11 +++++++++++ 2 files changed, 12 insertions(+)diff --git a/include/kunit/test.h b/include/kunit/test.h index ce0573e196ce..395221d623f7 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h@@ -285,6 +285,7 @@ struct kunit_suite { struct string_stream *log; int suite_init_err; bool is_init; + enum kunit_status status; }; /* Stores an array of suites, end points one past the end */diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 99773e000e1b..989acc770265 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c@@ -214,6 +214,9 @@ enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite) const struct kunit_case *test_case; enum kunit_status status = KUNIT_SKIPPED; + if (suite->status == KUNIT_SKIPPED) + return KUNIT_SKIPPED; + if (suite->suite_init_err) return KUNIT_FAILURE;@@ -795,12 +798,20 @@ int kunit_run_tests(struct kunit_suite *suite) /* Taint the kernel so we know we've run tests. */ add_taint(TAINT_TEST, LOCKDEP_STILL_OK); + if (suite->status == KUNIT_SKIPPED) + goto suite_end; +Do we want this? If a test is run more than once, we probably want to re-run suite_init so that we can tell if we should still skip it.
This condition should prevent a 'kunit_suite' from running if its marked skipped before suite_init() is called.
While I don't think it's likely that a test which was previously skipped will suddenly become available, it's not impossible with, e.g., CPU hotplug.
Agree, I have updated kunit_init_suite() to set 'kunit_suite.status' == KUNIT_SUCCESS. This should force the suite_init() to be calledback again when the suite is re-run. -- Cheers ~ Vaibhav