On Mon, Nov 21, 2022 at 3:51 PM Daniel Latypov dlatypov@google.com wrote:
On Mon, Nov 21, 2022 at 10:48 AM Rae Moar rmoar@google.com wrote:
Change the KUnit parser to be able to parse test output that complies with the KTAP version 1 specification format found here: https://kernel.org/doc/html/latest/dev-tools/ktap.html. Ensure the parser is able to parse tests with the original KUnit test output format as well.
KUnit parser now accepts any of the following test output formats:
Original KUnit test output format:
TAP version 14 1..1 # Subtest: kunit-test-suite 1..3 ok 1 - kunit_test_1 ok 2 - kunit_test_2 ok 3 - kunit_test_3 # kunit-test-suite: pass:3 fail:0 skip:0 total:3 # Totals: pass:3 fail:0 skip:0 total:3 ok 1 - kunit-test-suite
KTAP version 1 test output format:
KTAP version 1 1..1 KTAP version 1 1..3 ok 1 kunit_test_1 ok 2 kunit_test_2 ok 3 kunit_test_3 ok 1 kunit-test-suite
New KUnit test output format (changes made in the next patch of this series):
KTAP version 1 1..1 KTAP version 1 # Subtest: kunit-test-suite 1..3 ok 1 kunit_test_1 ok 2 kunit_test_2 ok 3 kunit_test_3 # kunit-test-suite: pass:3 fail:0 skip:0 total:3 # Totals: pass:3 fail:0 skip:0 total:3 ok 1 kunit-test-suite
Signed-off-by: Rae Moar rmoar@google.com Reviewed-by: Daniel Latypov dlatypov@google.com
Still looks good to me overall. As noted offline, this sadly has a conflict with another recent patch, so it won't apply to the kunit branch right now. That's my fault: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/co...
I found a few optional nits down below that we could also address in the rebased v3.
Reviewed-by: David Gow davidgow@google.com
Changes since v1: https://lore.kernel.org/all/20221104194705.3245738-2-rmoar@google.com/
- Switch order of patches to make changes to the parser before making changes to the test output
- Change placeholder label for test header from “Test suite” to empty string
- Change parser to approve the new KTAP version line in the subtest header to be before the subtest header line rather than after.
Thanks, as noted on the child patch, I think this will make our lives easier in the future, even if it technically violates the v1 spec (which requires the test plan right after the KTAP header IIUC).
Given the wording Diagnostic lines can be anywhere in the test output. I assume most implementations would likely ignore unexpected lines beginning with "# " already.
- Note: Considered changing parser to allow for the top-level of testing to have a '# Subtest' line as discussed in v1 but this breaks the missing test plan test. So I think it would be best to add this ability at a later time or after top-level test name and result lines are discussed for KTAP v2.
Makes sense to me.
message = test.name
if message != "":# Add a leading space before the subtest counts only if a test name# is provided using a "# Subtest" header line.message += " " if test.expected_count: if test.expected_count == 1:
message += ' (1 subtest)'
message += '(1 subtest)'Thanks, I like this output a lot better than having "Test suite" as a placeholder name. Tested this out by tweaking some kunit output locally and I get
[12:39:11] ======================= (4 subtests) ======================= [12:39:11] [PASSED] parse_filter_test [12:39:11] [PASSED] filter_suites_test [12:39:11] [PASSED] filter_suites_test_glob_test [12:39:11] [PASSED] filter_suites_to_empty_test [12:39:11] =============== [PASSED] kunit_executor_test ===============
Yeah I agree, this looks much better.
else:
message += f' ({test.expected_count} subtests)'
message += f'({test.expected_count} subtests)' stdout.print_with_timestamp(format_test_divider(message, len(message)))def print_log(log: Iterable[str]) -> None: @@ -647,7 +653,7 @@ def bubble_up_test_results(test: Test) -> None: elif test.counts.get_status() == TestStatus.TEST_CRASHED: test.status = TestStatus.TEST_CRASHED
-def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test: +def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest: bool) -> Test: """ Finds next test to parse in LineStream, creates new Test object, parses any subtests of the test, populates Test object with all @@ -665,15 +671,32 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test: 1..4 [subtests]
- Subtest header line
- Subtest header (must include either the KTAP version line or"# Subtest" header line)
Example:
Example (preferred format with both KTAP version line and"# Subtest" line):KTAP version 1# Subtest: name1..3[subtests]ok 1 nameExample (only "# Subtest" line): # Subtest: name 1..3 [subtests] ok 1 nameExample (only KTAP version line, compliant with KTAP v1 spec):KTAP version 11..3[subtests]ok 1 name- Test result line Example:@@ -685,28 +708,29 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test: expected_num - expected test number for test to be parsed log - list of strings containing any preceding diagnostic lines corresponding to the current test
is_subtest - boolean indicating whether test is a subtest Return: Test object populated with characteristics and any subtests """ test = Test() test.log.extend(log)
parent_test = Falsemain = parse_ktap_header(lines, test)if main:# If KTAP/TAP header is found, attempt to parse
if not 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) parse_test_plan(lines, test) parent_test = True else:
# If KTAP/TAP header is not found, test must be subtest# header or test result line so parse attempt to parser# subtest headerparent_test = parse_test_header(lines, test)
# If not the main test, attempt to parse a test header contatiningtypo: contatin => contain
Oops, I will change this.
# the KTAP version line and/or subtest header linektap_line = parse_ktap_header(lines, test)subtest_line = parse_test_header(lines, test)parent_test = (ktap_line or subtest_line)LGTM (this is where we changed to parse the KTAP header before " # Subtest").
Optional: do we want to extend kunit_tool_test.py to validate this logic too? E.g. given input like
KTAP version 1 1..1 KTAP version 1 # Subtest: suite 1..1 ok 1 - test ok 1 - subtest
we could assert that the parsed output contains "suite (1 subtest)"
i.e. self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
I like this addition. I will add this test to kunit_tool_test.py.
Daniel