On Wed, 11 Jun 2025 at 15:38, Thomas Weißschuh <thomas.weisssc...@linutronix.de> wrote: > > If a subtest itself reports success, but the outer testcase fails, > the whole testcase should be reported as a failure. > However the status is recalculated based on the test counts, > overwriting the outer test result. > Synthesize a failed test in this case to make sure the failure is not > swallowed. > > Signed-off-by: Thomas Weißschuh <thomas.weisssc...@linutronix.de> > ---
Hmm... this is definitely a nasty edge-case. I don't completely like this solution, but none of the other options seem drastically better. I think the more obvious options are either to _always_ count tests alongside their subtests, or to _never_ do so, but acknowledge that "test failed, but failure count is 0" is a valid option. But neither of those are especially satisfying, either greatly inflating test counts, or creating obvious contradictions. So I'm tentatively in favour of this, but if anyone has a nicer way of doing it, I'm all ears. The implementation looks good. If we can add the explicit checks for the sub(sub)test results as mentioned in the previous patch, that'd be even better. Reviewed-by: David Gow <david...@google.com> > tools/testing/kunit/kunit_parser.py | 5 > +++++ > tools/testing/kunit/kunit_tool_test.py | 2 +- > tools/testing/kunit/test_data/test_is_test_passed-failure-nested.log | 3 +++ > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/kunit/kunit_parser.py > b/tools/testing/kunit/kunit_parser.py > index > c176487356e6c94882046b19ea696d750905b8d5..2478beb28fc3db825855ad46200340e884da7df1 > 100644 > --- a/tools/testing/kunit/kunit_parser.py > +++ b/tools/testing/kunit/kunit_parser.py > @@ -686,6 +686,11 @@ def bubble_up_test_results(test: Test) -> None: > counts.add_status(status) > elif test.counts.get_status() == TestStatus.TEST_CRASHED: > test.status = TestStatus.TEST_CRASHED > + if not test.ok_status(): > + for t in subtests: > + if not t.ok_status(): > + counts.add_status(t.status) > + break > > def parse_test(lines: LineStream, expected_num: int, log: List[str], > is_subtest: bool, printer: Printer) -> Test: > """ > diff --git a/tools/testing/kunit/kunit_tool_test.py > b/tools/testing/kunit/kunit_tool_test.py > index > 691cde9b030f7729128490c1bdb42ccee1967ad6..c25f52650837e83325b06bddd2aa665fd29f91d9 > 100755 > --- a/tools/testing/kunit/kunit_tool_test.py > +++ b/tools/testing/kunit/kunit_tool_test.py > @@ -170,7 +170,7 @@ class KUnitParserTest(unittest.TestCase): > with open(nested_log) as file: > result = > kunit_parser.parse_run_tests(file.readlines(), stdout) > self.assertEqual(kunit_parser.TestStatus.FAILURE, > result.status) > - self.assertEqual(result.counts.failed, 2) > + self.assertEqual(result.counts.failed, 3) > self.assertEqual(kunit_parser.TestStatus.FAILURE, > result.subtests[0].status) Could we add: self.assertEqual(kunit_parser.TestStatus.SUCCESS, result.subtests[0].subtests[0].status) > self.assertEqual(kunit_parser.TestStatus.FAILURE, > result.subtests[1].status) and self.assertEqual(kunit_parser.TestStatus.FAILURE, result.subtests[1].subtests[0].status) > > diff --git > a/tools/testing/kunit/test_data/test_is_test_passed-failure-nested.log > b/tools/testing/kunit/test_data/test_is_test_passed-failure-nested.log > index > 835816e0a07715a514f5f5afab1b6250037feaf4..cd9033c464792e6294905a5676346684182874ad > 100644 > --- a/tools/testing/kunit/test_data/test_is_test_passed-failure-nested.log > +++ b/tools/testing/kunit/test_data/test_is_test_passed-failure-nested.log > @@ -1,5 +1,8 @@ > KTAP version 1 > 1..2 > + KTAP version 1 > + 1..1 > + ok 1 test 1 > not ok 1 subtest 1 > KTAP version 1 > 1..1 > > -- > 2.49.0 >
smime.p7s
Description: S/MIME Cryptographic Signature
