All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] kunit: tool: parse KTAP compliant test output
@ 2022-11-21 18:47 Rae Moar
  2022-11-21 18:47 ` [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit " Rae Moar
  2022-11-21 20:51 ` [PATCH v2 1/2] kunit: tool: parse KTAP compliant " Daniel Latypov
  0 siblings, 2 replies; 8+ messages in thread
From: Rae Moar @ 2022-11-21 18:47 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov
  Cc: skhan, mauro.chehab, kunit-dev, linux-kernel, linux-kselftest,
	isabbasso, anders.roxell, Rae Moar

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

tools/testing/kunit/kunit_parser.py           | 79 ++++++++++++-------
 tools/testing/kunit/kunit_tool_test.py        |  8 ++
 .../test_data/test_parse_ktap_output.log      |  8 ++
 3 files changed, 67 insertions(+), 28 deletions(-)
 create mode 100644 tools/testing/kunit/test_data/test_parse_ktap_output.log

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index a56c75a973b5..ed752d53d6a8 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -441,6 +441,7 @@ def parse_diagnostic(lines: LineStream) -> List[str]:
 	- '# Subtest: [test name]'
 	- '[ok|not ok] [test number] [-] [test name] [optional skip
 		directive]'
+	- 'KTAP version [version number]'
 
 	Parameters:
 	lines - LineStream of KTAP output to parse
@@ -449,8 +450,9 @@ def parse_diagnostic(lines: LineStream) -> List[str]:
 	Log of diagnostic lines
 	"""
 	log = []  # type: List[str]
-	while lines and not TEST_RESULT.match(lines.peek()) and not \
-			TEST_HEADER.match(lines.peek()):
+	non_diagnostic_lines = [TEST_RESULT, TEST_HEADER, KTAP_START]
+	while lines and not any(re.match(lines.peek())
+			for re in non_diagnostic_lines):
 		log.append(lines.pop())
 	return log
 
@@ -496,11 +498,15 @@ def print_test_header(test: Test) -> None:
 	test - Test object representing current test being printed
 	"""
 	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)'
 		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: name
+	1..3
+	[subtests]
+	ok 1 name
+
+	Example (only "# Subtest" line):
 
 	# Subtest: name
 	1..3
 	[subtests]
 	ok 1 name
 
+	Example (only KTAP version line, compliant with KTAP v1 spec):
+
+	KTAP version 1
+	1..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 = False
-	main = 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 header
-		parent_test = parse_test_header(lines, test)
+		# If not the main test, attempt to parse a test header contatining
+		# the KTAP version line and/or subtest header line
+		ktap_line = parse_ktap_header(lines, test)
+		subtest_line = parse_test_header(lines, test)
+		parent_test = (ktap_line or subtest_line)
 		if parent_test:
-			# If subtest header is found, attempt to parse
-			# test plan and print header
+			# If KTAP version line and/or subtest header is found, attempt
+			# to parse test plan and print test header
 			parse_test_plan(lines, test)
 			print_test_header(test)
 	expected_count = test.expected_count
@@ -721,7 +745,7 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test:
 		sub_log = parse_diagnostic(lines)
 		sub_test = Test()
 		if not lines or (peek_test_name_match(lines, test) and
-				not main):
+				is_subtest):
 			if expected_count and test_num <= expected_count:
 				# If parser reaches end of test before
 				# parsing expected number of subtests, print
@@ -735,20 +759,19 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test:
 				test.log.extend(sub_log)
 				break
 		else:
-			sub_test = parse_test(lines, test_num, sub_log)
+			sub_test = parse_test(lines, test_num, sub_log, True)
 		subtests.append(sub_test)
 		test_num += 1
 	test.subtests = subtests
-	if not main:
+	if is_subtest:
 		# If not main test, look for test result line
 		test.log.extend(parse_diagnostic(lines))
-		if (parent_test and peek_test_name_match(lines, test)) or \
-				not parent_test:
-			parse_test_result(lines, test, expected_num)
-		else:
+		if test.name != "" and not peek_test_name_match(lines, test):
 			test.add_error('missing subtest result line!')
+		else:
+			parse_test_result(lines, test, expected_num)
 
-	# Check for there being no tests
+	# Check for there being no subtests within parent test
 	if parent_test and len(subtests) == 0:
 		# Don't override a bad status if this test had one reported.
 		# Assumption: no subtests means CRASHED is from Test.__init__()
@@ -758,11 +781,11 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test:
 
 	# Add statuses to TestCounts attribute in Test object
 	bubble_up_test_results(test)
-	if parent_test and not main:
+	if parent_test and is_subtest:
 		# If test has subtests and is not the main test object, print
 		# footer.
 		print_test_footer(test)
-	elif not main:
+	elif is_subtest:
 		print_test_result(test)
 	return test
 
@@ -785,7 +808,7 @@ def parse_run_tests(kernel_output: Iterable[str]) -> Test:
 		test.add_error('could not find any KTAP output!')
 		test.status = TestStatus.FAILURE_TO_PARSE_TESTS
 	else:
-		test = parse_test(lines, 0, [])
+		test = parse_test(lines, 0, [], False)
 		if test.status != TestStatus.NO_TESTS:
 			test.status = test.counts.get_status()
 	stdout.print_with_timestamp(DIVIDER)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 90c65b072be9..7c2e2a45f330 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -312,6 +312,14 @@ class KUnitParserTest(unittest.TestCase):
 		self.assertEqual(kunit_parser._summarize_failed_tests(result),
 			'Failures: all_failed_suite, some_failed_suite.test2')
 
+	def test_ktap_format(self):
+		ktap_log = test_data_path('test_parse_ktap_output.log')
+		with open(ktap_log) as file:
+			result = kunit_parser.parse_run_tests(file.readlines())
+		self.assertEqual(result.counts, kunit_parser.TestCounts(passed=3))
+		self.assertEqual('suite', result.subtests[0].name)
+		self.assertEqual('case_1', result.subtests[0].subtests[0].name)
+		self.assertEqual('case_2', result.subtests[0].subtests[1].name)
 
 def line_stream_from_strs(strs: Iterable[str]) -> kunit_parser.LineStream:
 	return kunit_parser.LineStream(enumerate(strs, start=1))
diff --git a/tools/testing/kunit/test_data/test_parse_ktap_output.log b/tools/testing/kunit/test_data/test_parse_ktap_output.log
new file mode 100644
index 000000000000..ccdf244e5303
--- /dev/null
+++ b/tools/testing/kunit/test_data/test_parse_ktap_output.log
@@ -0,0 +1,8 @@
+KTAP version 1
+1..1
+  KTAP version 1
+  1..3
+  ok 1 case_1
+  ok 2 case_2
+  ok 3 case_3
+ok 1 suite

base-commit: 6fe1ad4a156095859721fef85073df3ed43081d4
-- 
2.38.1.584.g0f3c55d4c2-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit test output
  2022-11-21 18:47 [PATCH v2 1/2] kunit: tool: parse KTAP compliant test output Rae Moar
@ 2022-11-21 18:47 ` Rae Moar
  2022-11-21 20:32   ` Daniel Latypov
  2022-11-22  9:17   ` Anders Roxell
  2022-11-21 20:51 ` [PATCH v2 1/2] kunit: tool: parse KTAP compliant " Daniel Latypov
  1 sibling, 2 replies; 8+ messages in thread
From: Rae Moar @ 2022-11-21 18:47 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov
  Cc: skhan, mauro.chehab, kunit-dev, linux-kernel, linux-kselftest,
	isabbasso, anders.roxell, Rae Moar

Change KUnit test output to better comply with KTAP v1 specifications
found here: https://kernel.org/doc/html/latest/dev-tools/ktap.html.
1) Use "KTAP version 1" instead of "TAP version 14" as test output header
2) Remove '-' between test number and test name on test result lines
2) Add KTAP version lines to each subtest header as well

Note that the new KUnit output still includes the “# Subtest” line now
located after the KTAP version line. This does not completely match the
KTAP v1 spec but since it is classified as a diagnostic line, it is not
expected to be disruptive or break any existing parsers. This
“# Subtest” line comes from the TAP 14 spec
(https://testanything.org/tap-version-14-specification.html)
and it is used to define the test name before the results.

Original output:

 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

New output:

 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>
Reviewed-by: David Gow <davidgow@google.com>
---

Changes since v1:
https://lore.kernel.org/all/20221104194705.3245738-1-rmoar@google.com/
- Switch order of patches to make changes to the parser before making
  changes to the test output
- Change location of the new KTAP version line in subtest header to be
  before the subtest header line

 lib/kunit/executor.c | 6 +++---
 lib/kunit/test.c     | 5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 9bbc422c284b..74982b83707c 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -166,7 +166,7 @@ static void kunit_exec_run_tests(struct suite_set *suite_set)
 {
 	size_t num_suites = suite_set->end - suite_set->start;
 
-	pr_info("TAP version 14\n");
+	pr_info("KTAP version 1\n");
 	pr_info("1..%zu\n", num_suites);
 
 	__kunit_test_suites_init(suite_set->start, num_suites);
@@ -177,8 +177,8 @@ static void kunit_exec_list_tests(struct suite_set *suite_set)
 	struct kunit_suite * const *suites;
 	struct kunit_case *test_case;
 
-	/* Hack: print a tap header so kunit.py can find the start of KUnit output. */
-	pr_info("TAP version 14\n");
+	/* Hack: print a ktap header so kunit.py can find the start of KUnit output. */
+	pr_info("KTAP version 1\n");
 
 	for (suites = suite_set->start; suites < suite_set->end; suites++)
 		kunit_suite_for_each_test_case((*suites), test_case) {
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 90640a43cf62..19344cb501c5 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -149,6 +149,7 @@ EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
 
 static void kunit_print_suite_start(struct kunit_suite *suite)
 {
+	kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "KTAP version 1\n");
 	kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "# Subtest: %s",
 		  suite->name);
 	kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "1..%zd",
@@ -175,13 +176,13 @@ static void kunit_print_ok_not_ok(void *test_or_suite,
 	 * representation.
 	 */
 	if (suite)
-		pr_info("%s %zd - %s%s%s\n",
+		pr_info("%s %zd %s%s%s\n",
 			kunit_status_to_ok_not_ok(status),
 			test_number, description, directive_header,
 			(status == KUNIT_SKIPPED) ? directive : "");
 	else
 		kunit_log(KERN_INFO, test,
-			  KUNIT_SUBTEST_INDENT "%s %zd - %s%s%s",
+			  KUNIT_SUBTEST_INDENT "%s %zd %s%s%s",
 			  kunit_status_to_ok_not_ok(status),
 			  test_number, description, directive_header,
 			  (status == KUNIT_SKIPPED) ? directive : "");
-- 
2.38.1.584.g0f3c55d4c2-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit test output
  2022-11-21 18:47 ` [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit " Rae Moar
@ 2022-11-21 20:32   ` Daniel Latypov
  2022-11-22  9:17   ` Anders Roxell
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel Latypov @ 2022-11-21 20:32 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, skhan, mauro.chehab, kunit-dev,
	linux-kernel, linux-kselftest, isabbasso, anders.roxell

On Mon, Nov 21, 2022 at 10:48 AM Rae Moar <rmoar@google.com> wrote:
>
> Change KUnit test output to better comply with KTAP v1 specifications
> found here: https://kernel.org/doc/html/latest/dev-tools/ktap.html.
> 1) Use "KTAP version 1" instead of "TAP version 14" as test output header
> 2) Remove '-' between test number and test name on test result lines
> 2) Add KTAP version lines to each subtest header as well
>
> Note that the new KUnit output still includes the “# Subtest” line now
> located after the KTAP version line. This does not completely match the
> KTAP v1 spec but since it is classified as a diagnostic line, it is not
> expected to be disruptive or break any existing parsers. This
> “# Subtest” line comes from the TAP 14 spec
> (https://testanything.org/tap-version-14-specification.html)
> and it is used to define the test name before the results.
>
> Original output:
>
>  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
>
> New output:
>
>  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>
> Reviewed-by: David Gow <davidgow@google.com>
> ---
>
> Changes since v1:
> https://lore.kernel.org/all/20221104194705.3245738-1-rmoar@google.com/
> - Switch order of patches to make changes to the parser before making
>   changes to the test output
> - Change location of the new KTAP version line in subtest header to be
>   before the subtest header line

This patch still looks good to me. In fact, it looks better.

I prefer this updated version since this works a bit better with
debugfs. This way, kunit.py won't just skip over the subtest line when
looking for the initial KTAP header.

Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] kunit: tool: parse KTAP compliant test output
  2022-11-21 18:47 [PATCH v2 1/2] kunit: tool: parse KTAP compliant test output Rae Moar
  2022-11-21 18:47 ` [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit " Rae Moar
@ 2022-11-21 20:51 ` Daniel Latypov
  2022-11-22 22:06   ` Rae Moar
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Latypov @ 2022-11-21 20:51 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, skhan, mauro.chehab, kunit-dev,
	linux-kernel, linux-kselftest, isabbasso, anders.roxell

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/commit/?h=kunit&id=2a883a9f5c1f1c7bb9d9116da68e2ef2faeae5b8

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

>                 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: name
> +       1..3
> +       [subtests]
> +       ok 1 name
> +
> +       Example (only "# Subtest" line):
>
>         # Subtest: name
>         1..3
>         [subtests]
>         ok 1 name
>
> +       Example (only KTAP version line, compliant with KTAP v1 spec):
> +
> +       KTAP version 1
> +       1..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 = False
> -       main = 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 header
> -               parent_test = parse_test_header(lines, test)
> +               # If not the main test, attempt to parse a test header contatining

typo: contatin => contain

> +               # the KTAP version line and/or subtest header line
> +               ktap_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)'))

Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit test output
  2022-11-21 18:47 ` [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit " Rae Moar
  2022-11-21 20:32   ` Daniel Latypov
@ 2022-11-22  9:17   ` Anders Roxell
  2022-11-22 17:14     ` Daniel Latypov
  1 sibling, 1 reply; 8+ messages in thread
From: Anders Roxell @ 2022-11-22  9:17 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, dlatypov, skhan, mauro.chehab,
	kunit-dev, linux-kernel, linux-kselftest, isabbasso

On Mon, 21 Nov 2022 at 19:48, Rae Moar <rmoar@google.com> wrote:
>
> Change KUnit test output to better comply with KTAP v1 specifications
> found here: https://kernel.org/doc/html/latest/dev-tools/ktap.html.
> 1) Use "KTAP version 1" instead of "TAP version 14" as test output header
> 2) Remove '-' between test number and test name on test result lines
> 2) Add KTAP version lines to each subtest header as well
>
> Note that the new KUnit output still includes the “# Subtest” line now
> located after the KTAP version line. This does not completely match the
> KTAP v1 spec but since it is classified as a diagnostic line, it is not
> expected to be disruptive or break any existing parsers. This
> “# Subtest” line comes from the TAP 14 spec
> (https://testanything.org/tap-version-14-specification.html)
> and it is used to define the test name before the results.
>
> Original output:
>
>  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
>
> New output:
>
>  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>
> Reviewed-by: David Gow <davidgow@google.com>

I tried this patch, see the full boot log [1] and I can still see some
 tests that output the "old" format with 'ok 1 - kunit_test_1', for example

ok 1 - 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits

Isn't this something that should be converted too?

Cheers,
Anders
[1] http://ix.io/4gwf

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit test output
  2022-11-22  9:17   ` Anders Roxell
@ 2022-11-22 17:14     ` Daniel Latypov
  2022-11-22 20:19       ` Rae Moar
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Latypov @ 2022-11-22 17:14 UTC (permalink / raw)
  To: Anders Roxell
  Cc: Rae Moar, brendanhiggins, davidgow, skhan, mauro.chehab,
	kunit-dev, linux-kernel, linux-kselftest, isabbasso

On Tue, Nov 22, 2022 at 1:17 AM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> On Mon, 21 Nov 2022 at 19:48, Rae Moar <rmoar@google.com> wrote:
> >
> > Change KUnit test output to better comply with KTAP v1 specifications
> > found here: https://kernel.org/doc/html/latest/dev-tools/ktap.html.
> > 1) Use "KTAP version 1" instead of "TAP version 14" as test output header
> > 2) Remove '-' between test number and test name on test result lines
> > 2) Add KTAP version lines to each subtest header as well
> >
> > Note that the new KUnit output still includes the “# Subtest” line now
> > located after the KTAP version line. This does not completely match the
> > KTAP v1 spec but since it is classified as a diagnostic line, it is not
> > expected to be disruptive or break any existing parsers. This
> > “# Subtest” line comes from the TAP 14 spec
> > (https://testanything.org/tap-version-14-specification.html)
> > and it is used to define the test name before the results.
> >
> > Original output:
> >
> >  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
> >
> > New output:
> >
> >  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>
> > Reviewed-by: David Gow <davidgow@google.com>
>
> I tried this patch, see the full boot log [1] and I can still see some
>  tests that output the "old" format with 'ok 1 - kunit_test_1', for example
>
> ok 1 - 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
>
> Isn't this something that should be converted too?

Yes, thanks for catching that.
That's what I get from only looking over the diff this time since I
thought I remembered the code...

We also want this diff to fix a) debugfs, b) subtests.

diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 1048ef1b8d6e..de0ee2e03ed6 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -63,7 +63,7 @@ static int debugfs_print_results(struct seq_file
*seq, void *v)
        kunit_suite_for_each_test_case(suite, test_case)
                debugfs_print_result(seq, suite, test_case);

-       seq_printf(seq, "%s %d - %s\n",
+       seq_printf(seq, "%s %d %s\n",
                   kunit_status_to_ok_not_ok(success), 1, suite->name);
        return 0;
 }
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 19344cb501c5..c9d57a1d9524 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -556,7 +556,7 @@ int kunit_run_tests(struct kunit_suite *suite)

                                kunit_log(KERN_INFO, &test,
                                          KUNIT_SUBTEST_INDENT
KUNIT_SUBTEST_INDENT
-                                         "%s %d - %s",
+                                         "%s %d %s",

kunit_status_to_ok_not_ok(test.status),
                                          test.param_index + 1, param_desc);

Daniel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit test output
  2022-11-22 17:14     ` Daniel Latypov
@ 2022-11-22 20:19       ` Rae Moar
  0 siblings, 0 replies; 8+ messages in thread
From: Rae Moar @ 2022-11-22 20:19 UTC (permalink / raw)
  To: Daniel Latypov, Anders Roxell
  Cc: brendanhiggins, davidgow, skhan, mauro.chehab, kunit-dev,
	linux-kernel, linux-kselftest, isabbasso

On Tue, Nov 22, 2022 at 12:14 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Tue, Nov 22, 2022 at 1:17 AM Anders Roxell <anders.roxell@linaro.org> wrote:
> >
> > On Mon, 21 Nov 2022 at 19:48, Rae Moar <rmoar@google.com> wrote:
> > >
> > > Change KUnit test output to better comply with KTAP v1 specifications
> > > found here: https://kernel.org/doc/html/latest/dev-tools/ktap.html.
> > > 1) Use "KTAP version 1" instead of "TAP version 14" as test output header
> > > 2) Remove '-' between test number and test name on test result lines
> > > 2) Add KTAP version lines to each subtest header as well
> > >
> > > Note that the new KUnit output still includes the “# Subtest” line now
> > > located after the KTAP version line. This does not completely match the
> > > KTAP v1 spec but since it is classified as a diagnostic line, it is not
> > > expected to be disruptive or break any existing parsers. This
> > > “# Subtest” line comes from the TAP 14 spec
> > > (https://testanything.org/tap-version-14-specification.html)
> > > and it is used to define the test name before the results.
> > >
> > > Original output:
> > >
> > >  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
> > >
> > > New output:
> > >
> > >  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>
> > > Reviewed-by: David Gow <davidgow@google.com>
> >
> > I tried this patch, see the full boot log [1] and I can still see some
> >  tests that output the "old" format with 'ok 1 - kunit_test_1', for example
> >
> > ok 1 - 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
> >
> > Isn't this something that should be converted too?

Hello! Sorry for missing that. You are definitely right. Those results should
be converted as well.

>
> Yes, thanks for catching that.
> That's what I get from only looking over the diff this time since I
> thought I remembered the code...
>
> We also want this diff to fix a) debugfs, b) subtests.
>
> diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
> index 1048ef1b8d6e..de0ee2e03ed6 100644
> --- a/lib/kunit/debugfs.c
> +++ b/lib/kunit/debugfs.c
> @@ -63,7 +63,7 @@ static int debugfs_print_results(struct seq_file
> *seq, void *v)
>         kunit_suite_for_each_test_case(suite, test_case)
>                 debugfs_print_result(seq, suite, test_case);
>
> -       seq_printf(seq, "%s %d - %s\n",
> +       seq_printf(seq, "%s %d %s\n",
>                    kunit_status_to_ok_not_ok(success), 1, suite->name);
>         return 0;
>  }
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 19344cb501c5..c9d57a1d9524 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -556,7 +556,7 @@ int kunit_run_tests(struct kunit_suite *suite)
>
>                                 kunit_log(KERN_INFO, &test,
>                                           KUNIT_SUBTEST_INDENT
> KUNIT_SUBTEST_INDENT
> -                                         "%s %d - %s",
> +                                         "%s %d %s",
>
> kunit_status_to_ok_not_ok(test.status),
>                                           test.param_index + 1, param_desc);
>
> Daniel

Thanks Daniel!

I think that should do the trick with the addition of adding the "KTAP
version 1" line, which you can do with the following diff:

diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c9d57a1d9524..1c9d8d962d67 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -543,6 +543,8 @@ int kunit_run_tests(struct kunit_suite *suite)
                        /* Get initial param. */
                        param_desc[0] = '\0';
                        test.param_value =
test_case->generate_params(NULL, param_desc);
+                       kunit_log(KERN_INFO, &test,
KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
+                                 "KTAP version 1\n");
                        kunit_log(KERN_INFO, &test,
KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
                                  "# Subtest: %s", test_case->name);

Going to run through some more examples to make sure this version
works otherwise I will make a v3 with the addition of fixing any merge
conflicts.

Rae

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] kunit: tool: parse KTAP compliant test output
  2022-11-21 20:51 ` [PATCH v2 1/2] kunit: tool: parse KTAP compliant " Daniel Latypov
@ 2022-11-22 22:06   ` Rae Moar
  0 siblings, 0 replies; 8+ messages in thread
From: Rae Moar @ 2022-11-22 22:06 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: brendanhiggins, davidgow, skhan, mauro.chehab, kunit-dev,
	linux-kernel, linux-kselftest, isabbasso, anders.roxell

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/commit/?h=kunit&id=2a883a9f5c1f1c7bb9d9116da68e2ef2faeae5b8
>
> 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: name
> > +       1..3
> > +       [subtests]
> > +       ok 1 name
> > +
> > +       Example (only "# Subtest" line):
> >
> >         # Subtest: name
> >         1..3
> >         [subtests]
> >         ok 1 name
> >
> > +       Example (only KTAP version line, compliant with KTAP v1 spec):
> > +
> > +       KTAP version 1
> > +       1..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 = False
> > -       main = 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 header
> > -               parent_test = parse_test_header(lines, test)
> > +               # If not the main test, attempt to parse a test header contatining
>
> typo: contatin => contain

Oops, I will change this.

>
> > +               # the KTAP version line and/or subtest header line
> > +               ktap_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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-22 22:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-21 18:47 [PATCH v2 1/2] kunit: tool: parse KTAP compliant test output Rae Moar
2022-11-21 18:47 ` [PATCH v2 2/2] kunit: improve KTAP compliance of KUnit " Rae Moar
2022-11-21 20:32   ` Daniel Latypov
2022-11-22  9:17   ` Anders Roxell
2022-11-22 17:14     ` Daniel Latypov
2022-11-22 20:19       ` Rae Moar
2022-11-21 20:51 ` [PATCH v2 1/2] kunit: tool: parse KTAP compliant " Daniel Latypov
2022-11-22 22:06   ` Rae Moar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.