| Message ID | 20250306182009.2255241-1-rmoar@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [v2] kunit: tool: Fix bug in parsing test plan | expand |
On Thu, 6 Mar 2025 at 19:20, 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 this bug to correctly parse test plan line. > > Signed-off-by: Rae Moar <rmoar@google.com> Hi Rae thanks again for taking a look at this. I just noticed kunit_tool_test.py has unit tests for the parsing logic. Maybe worth adding one for the case that was infinite looping before? (I am not really able to offer an actual review as I couldn't really understand the parsing code after a quick reading, but if you need it then let me know and I'll chase you up to ask questions until I can follow it, then I can review!)
On Fri, 7 Mar 2025 at 02:20, 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 this bug to correctly parse test plan line. > > Signed-off-by: Rae Moar <rmoar@google.com> > --- Looks good. I agree with Brendan that this probably should have a test, but I'm happy to take that as a follow-up if you prefer. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > Changes since v1: > - Remove error reported when test plan is missing. > > tools/testing/kunit/kunit_parser.py | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py > index 29fc27e8949b..da53a709773a 100644 > --- a/tools/testing/kunit/kunit_parser.py > +++ b/tools/testing/kunit/kunit_parser.py > @@ -759,7 +759,7 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest: > # If parsing the main/top-level test, parse KTAP version line and > # test plan > test.name = "main" > - ktap_line = parse_ktap_header(lines, test, printer) > + parse_ktap_header(lines, test, printer) > test.log.extend(parse_diagnostic(lines)) > parse_test_plan(lines, test) > parent_test = True > @@ -768,13 +768,12 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest: > # 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)) > + 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) > + > expected_count = test.expected_count > subtests = [] > test_num = 1 > > base-commit: 0619a4868fc1b32b07fb9ed6c69adc5e5cf4e4b2 > -- > 2.49.0.rc0.332.g42c0ae87b1-goog >
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 29fc27e8949b..da53a709773a 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -759,7 +759,7 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest: # If parsing the main/top-level test, parse KTAP version line and # test plan test.name = "main" - ktap_line = parse_ktap_header(lines, test, printer) + parse_ktap_header(lines, test, printer) test.log.extend(parse_diagnostic(lines)) parse_test_plan(lines, test) parent_test = True @@ -768,13 +768,12 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest: # 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)) + 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) + expected_count = test.expected_count subtests = [] test_num = 1
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 this bug to correctly parse test plan line. Signed-off-by: Rae Moar <rmoar@google.com> --- Changes since v1: - Remove error reported when test plan is missing. tools/testing/kunit/kunit_parser.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) base-commit: 0619a4868fc1b32b07fb9ed6c69adc5e5cf4e4b2