Re: [PATCH] kunit: tool: Fix bug in parsing test plan

On Thu, Mar 6, 2025 at 4:00 AM David Gow davidgow@google.com wrote:

On Thu, 6 Mar 2025 at 08:29, Rae Moar rmoar@google.com wrote:

...

A bug was identified where the KTAP below caused an infinite loop:

TAP version 13 ok 4 test_case 1..4

The infinite loop was caused by the parser not parsing a test plan if following a test result line.

Fix bug to correctly parse test plan and add error if test plan is missing.

Signed-off-by: Rae Moar rmoar@google.com

Thanks for looking into this: I don't think we want to unconditionally error if there's no test plan, though. Pretty much no parameterised tests include one -- it's not always possible to know how many tests there'll be in advance -- so this triggers all of the time.

Maybe we can only include an error if we find a test plan line after an existing result, or something?

Hi David!

I thought I'd include the error in the first version but I figured it might not be accepted. Technically it is improper KTAP for the test plan to be missing so the error would be correct but because it fires on parameterized tests which is not ideal.

I wonder for parameterized tests if we could output a test plan: "1..X" indicating an unknown number of tests or something similar. I'd be happy to implement this. However, I am happy to remove the error for the second version.

Thanks! -Rae

-- David

...

tools/testing/kunit/kunit_parser.py | 12 +++++++----- tools/testing/kunit/kunit_tool_test.py | 5 ++--- 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 29fc27e8949b..5dcbc670e1dc 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -761,20 +761,22 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest: test.name = "main" ktap_line = parse_ktap_header(lines, test, printer) test.log.extend(parse_diagnostic(lines))

•           parse_test_plan(lines, test)
  

•           plan_line = parse_test_plan(lines, test)
            parent_test = True
    else:
            # If not the main test, attempt to parse a test header containing
            # the KTAP version line and/or subtest header line
            ktap_line = parse_ktap_header(lines, test, printer)
            subtest_line = parse_test_header(lines, test)
  

•           test.log.extend(parse_diagnostic(lines))
  

•           plan_line = parse_test_plan(lines, test)
            parent_test = (ktap_line or subtest_line)
            if parent_test:
  

•                   # If KTAP version line and/or subtest header is found, attempt
  

•                   # to parse test plan and print test header
  

•                   test.log.extend(parse_diagnostic(lines))
  

•                   parse_test_plan(lines, test)
                    print_test_header(test, printer)
  

•   if parent_test and not plan_line:
  

•                   test.add_error(printer, 'missing test plan!')
  

•   expected_count = test.expected_count
    subtests = []
    test_num = 1
  

diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index 0bcb0cc002f8..e1e142c1a850 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -181,8 +181,7 @@ class KUnitParserTest(unittest.TestCase): result = kunit_parser.parse_run_tests( kunit_parser.extract_tap_lines( file.readlines()), stdout)

•           # A missing test plan is not an error.
  

•           self.assertEqual(result.counts, kunit_parser.TestCounts(passed=10, errors=0))
  

•           self.assertEqual(result.counts, kunit_parser.TestCounts(passed=10, errors=2))
            self.assertEqual(kunit_parser.TestStatus.SUCCESS, result.status)
  
    def test_no_tests(self):
  

@@ -203,7 +202,7 @@ class KUnitParserTest(unittest.TestCase): self.assertEqual( kunit_parser.TestStatus.NO_TESTS, result.subtests[0].subtests[0].status)

•           self.assertEqual(result.counts, kunit_parser.TestCounts(passed=1, errors=1))
  

•           self.assertEqual(result.counts, kunit_parser.TestCounts(passed=1, errors=2))
  
  
    def test_no_kunit_output(self):
  
  

base-commit: 0619a4868fc1b32b07fb9ed6c69adc5e5cf4e4b2

2.48.1.711.g2feabab25a-goog