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.weissschuh@linutronix.de>
---
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)
self.assertEqual(kunit_parser.TestStatus.FAILURE, result.subtests[1].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
On Wed, 11 Jun 2025 at 15:38, Thomas Weißschuh <thomas.weissschuh@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.weissschuh@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 <davidgow@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 >
On Fri, Jun 20, 2025 at 05:37:44PM +0800, David Gow wrote: > On Wed, 11 Jun 2025 at 15:38, Thomas Weißschuh > <thomas.weissschuh@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.weissschuh@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. Agreed, it is not great. I'd also be happy for better ideas. > 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 <davidgow@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) Ack. > and > > self.assertEqual(kunit_parser.TestStatus.FAILURE, > result.subtests[1].subtests[0].status) This is now already in the previous patch. > > > > 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 > >
© 2016 - 2025 Red Hat, Inc.