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

[David Gow]

On Thu, 6 Mar 2025 at 08:29, Rae Moar <rmoar@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---

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?

-- 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
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature