On Wed, Apr 27, 2022 at 1:33 AM Daniel Latypov dlatypov@google.com wrote:
Consider this invocation $ ./tools/testing/kunit/kunit.py parse <<EOF TAP version 14 1..2 ok 1 - suite # Subtest: no_tests_suite # catastrophic error! not ok 1 - no_tests_suite EOF
It will have a 0 exit code even though there's a "not ok".
Consider this one: $ ./tools/testing/kunit/kunit.py parse <<EOF TAP version 14 1..2 ok 1 - suite not ok 1 - no_tests_suite EOF
It will a non-zero exit code.
Why? We have this line in the kunit_parser.py
parent_test = parse_test_header(lines, test)
where we have special handling when we see "# Subtest" and we ignore the explicit reported "not ok 1" status!
Also, NO_TESTS at a suite-level only results in a non-zero status code where then there's only one suite atm.
This change is the minimal one to make sure we don't overwrite it.
Signed-off-by: Daniel Latypov dlatypov@google.com
This seems sensible to me, though it doesn't change a lot in practice given that the in-kernel KUnit will mark empty suites as skipped: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/...
(And I think that is probably the correct thing for it to do, as "no tests run" seems closer to skipped than to passed or failed, which are the other options KTAP gives us.)
That being said, it's definitely true that we don't want to override a "not ok" needlessly in kunit_tool, so this patch is definite improvement.
Reviewed-by: David Gow davidgow@google.com
-- David
tools/testing/kunit/kunit_parser.py | 7 +++++-- .../test_data/test_is_test_passed-no_tests_no_plan.log | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 7a0faf527a98..45c2c5837281 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -775,8 +775,11 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test:
# Check for there being no tests if parent_test and len(subtests) == 0:
test.status = TestStatus.NO_TESTS
test.add_error('0 tests run!')
# Don't override a bad status if this test had one reported.
# Assumption: no subtests means CRASHED is from Test.__init__()
if test.status in (TestStatus.TEST_CRASHED, TestStatus.SUCCESS):
test.status = TestStatus.NO_TESTS
test.add_error('0 tests run!') # Add statuses to TestCounts attribute in Test object bubble_up_test_results(test)
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log index dd873c981108..4f81876ee6f1 100644 --- a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log +++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log @@ -3,5 +3,5 @@ TAP version 14 # Subtest: suite 1..1 # Subtest: case
- ok 1 - case # SKIP
- ok 1 - case
ok 1 - suite
2.36.0.rc2.479.g8af0fa9b8e-goog