[PATCH v2 1/2] kunit: tool: fix parsing of test attributes

[PATCH v2 1/2] kunit: tool: fix parsing of test attributes

[Rae Moar]

Add parsing of attributes as diagnostic data. Fixes issue with test plan
being parsed incorrectly as diagnostic data when located after
suite-level attributes.

Note that if there does not exist a test plan line, the diagnostic lines
between the suite header and the first result will be saved in the suite
log rather than the first test case log.

Signed-off-by: Rae Moar <rmoar@google.com>
---
 tools/testing/kunit/kunit_parser.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 79d8832c862a..ce34be15c929 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -450,7 +450,7 @@ def parse_diagnostic(lines: LineStream) -> List[str]:
 	Log of diagnostic lines
 	"""
 	log = []  # type: List[str]
-	non_diagnostic_lines = [TEST_RESULT, TEST_HEADER, KTAP_START, TAP_START]
+	non_diagnostic_lines = [TEST_RESULT, TEST_HEADER, KTAP_START, TAP_START, TEST_PLAN]
 	while lines and not any(re.match(lines.peek())
 			for re in non_diagnostic_lines):
 		log.append(lines.pop())
@@ -726,6 +726,7 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest:
 		# test plan
 		test.name = "main"
 		ktap_line = parse_ktap_header(lines, test)
+		test.log.extend(parse_diagnostic(lines))
 		parse_test_plan(lines, test)
 		parent_test = True
 	else:
@@ -737,6 +738,7 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str], is_subtest:
 		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)
 	expected_count = test.expected_count

base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
-- 
2.43.0.472.g3155946c3a-goog

[PATCH v2 2/2] kunit: tool: add test for parsing attributes

[Rae Moar]

Add test for parsing attributes to kunit_tool_test.py. Test checks
attributes are parsed and saved in the test logs.

This test also checks that the attributes have not interfered with the
parsing of other test information, specifically the suite header as
the test plan was being incorrectely parsed.

Signed-off-by: Rae Moar <rmoar@google.com>
---
 tools/testing/kunit/kunit_tool_test.py           | 16 ++++++++++++++++
 .../kunit/test_data/test_parse_attributes.log    |  9 +++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 tools/testing/kunit/test_data/test_parse_attributes.log

diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index b28c1510be2e..2beb7327e53f 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -331,6 +331,22 @@ class KUnitParserTest(unittest.TestCase):
 			kunit_parser.parse_run_tests(file.readlines())
 		self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
 
+	def test_parse_attributes(self):
+		ktap_log = test_data_path('test_parse_attributes.log')
+		with open(ktap_log) as file:
+			result = kunit_parser.parse_run_tests(file.readlines())
+
+		# Test should pass with no errors
+		self.assertEqual(result.counts, kunit_parser.TestCounts(passed=1, errors=0))
+		self.assertEqual(kunit_parser.TestStatus.SUCCESS, result.status)
+
+		# Ensure suite header is parsed correctly
+		self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
+
+		# Ensure attributes in correct test log
+		self.assertContains('# module: example', result.subtests[0].log)
+		self.assertContains('# test.speed: slow', result.subtests[0].subtests[0].log)
+
 	def test_show_test_output_on_failure(self):
 		output = """
 		KTAP version 1
diff --git a/tools/testing/kunit/test_data/test_parse_attributes.log b/tools/testing/kunit/test_data/test_parse_attributes.log
new file mode 100644
index 000000000000..1a13c371fe9d
--- /dev/null
+++ b/tools/testing/kunit/test_data/test_parse_attributes.log
@@ -0,0 +1,9 @@
+KTAP version 1
+1..1
+  KTAP version 1
+  # Subtest: suite
+  # module: example
+  1..1
+  # test.speed: slow
+  ok 1 test
+ok 1 suite
\ No newline at end of file
-- 
2.43.0.472.g3155946c3a-goog

Re: [PATCH v2 1/2] kunit: tool: fix parsing of test attributes

[David Gow]

On Fri, 8 Dec 2023 at 05:34, Rae Moar <rmoar@google.com> wrote:
>
> Add parsing of attributes as diagnostic data. Fixes issue with test plan
> being parsed incorrectly as diagnostic data when located after
> suite-level attributes.
>
> Note that if there does not exist a test plan line, the diagnostic lines
> between the suite header and the first result will be saved in the suite
> log rather than the first test case log.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---

Thanks, this looks good to me.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

Re: [PATCH v2 2/2] kunit: tool: add test for parsing attributes

[David Gow]

On Fri, 8 Dec 2023 at 05:34, Rae Moar <rmoar@google.com> wrote:
>
> Add test for parsing attributes to kunit_tool_test.py. Test checks
> attributes are parsed and saved in the test logs.
>
> This test also checks that the attributes have not interfered with the
> parsing of other test information, specifically the suite header as
> the test plan was being incorrectely parsed.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---

Thanks -- it's good to have this tested. I'm looking forward to our
actually parsing attributes out and using them for things in the
future, too.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

>  tools/testing/kunit/kunit_tool_test.py           | 16 ++++++++++++++++
>  .../kunit/test_data/test_parse_attributes.log    |  9 +++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 tools/testing/kunit/test_data/test_parse_attributes.log
>
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index b28c1510be2e..2beb7327e53f 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -331,6 +331,22 @@ class KUnitParserTest(unittest.TestCase):
>                         kunit_parser.parse_run_tests(file.readlines())
>                 self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
>
> +       def test_parse_attributes(self):
> +               ktap_log = test_data_path('test_parse_attributes.log')
> +               with open(ktap_log) as file:
> +                       result = kunit_parser.parse_run_tests(file.readlines())
> +
> +               # Test should pass with no errors
> +               self.assertEqual(result.counts, kunit_parser.TestCounts(passed=1, errors=0))
> +               self.assertEqual(kunit_parser.TestStatus.SUCCESS, result.status)
> +
> +               # Ensure suite header is parsed correctly
> +               self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
> +
> +               # Ensure attributes in correct test log
> +               self.assertContains('# module: example', result.subtests[0].log)
> +               self.assertContains('# test.speed: slow', result.subtests[0].subtests[0].log)
> +
>         def test_show_test_output_on_failure(self):
>                 output = """
>                 KTAP version 1
> diff --git a/tools/testing/kunit/test_data/test_parse_attributes.log b/tools/testing/kunit/test_data/test_parse_attributes.log
> new file mode 100644
> index 000000000000..1a13c371fe9d
> --- /dev/null
> +++ b/tools/testing/kunit/test_data/test_parse_attributes.log
> @@ -0,0 +1,9 @@
> +KTAP version 1
> +1..1
> +  KTAP version 1
> +  # Subtest: suite
> +  # module: example
> +  1..1
> +  # test.speed: slow
> +  ok 1 test
> +ok 1 suite
> \ No newline at end of file

Why doesn't this have a newline at the end of the file?

I'm actually okay with it here (it's good to test the case where there
isn't one _somewhere_), but in general, I think we'd want to prefer
KTAP documents end with the trailing newline, like most text file.