kunit: tool: print clearer error message when there's no TAP output

[PATCH] kunit: tool: print clearer error message when there's no TAP output

Posted by Daniel Latypov 4 years, 2 months ago

Before:
$ ./tools/testing/kunit/kunit.py parse /dev/null
...
[ERROR] Test : invalid KTAP input!

After:
$ ./tools/testing/kunit/kunit.py parse /dev/null
...
[ERROR] Test <missing>: could not find any KTAP output!

This error message gets printed out when extract_tap_output() yielded no
lines. So while it could be because of malformed KTAP output from KUnit,
it could also be due to to not having any KTAP output at all.

Try and make the error message here more clear.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 tools/testing/kunit/kunit_parser.py    | 3 ++-
 tools/testing/kunit/kunit_tool_test.py | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 05ff334761dd..103d95a66a7e 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -817,7 +817,8 @@ def parse_run_tests(kernel_output: Iterable[str]) -> Test:
 	lines = extract_tap_lines(kernel_output)
 	test = Test()
 	if not lines:
-		test.add_error('invalid KTAP input!')
+		test.name = '<missing>'
+		test.add_error('could not find any KTAP output!')
 		test.status = TestStatus.FAILURE_TO_PARSE_TESTS
 	else:
 		test = parse_test(lines, 0, [])
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 352369dffbd9..f14934853ea1 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -226,7 +226,7 @@ class KUnitParserTest(unittest.TestCase):
 		with open(crash_log) as file:
 			result = kunit_parser.parse_run_tests(
 				kunit_parser.extract_tap_lines(file.readlines()))
-		print_mock.assert_any_call(StrContains('invalid KTAP input!'))
+		print_mock.assert_any_call(StrContains('could not find any KTAP output!'))
 		print_mock.stop()
 		self.assertEqual(0, len(result.subtests))
 
@@ -559,7 +559,7 @@ class KUnitMainTest(unittest.TestCase):
 		self.assertEqual(e.exception.code, 1)
 		self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
 		self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
-		self.print_mock.assert_any_call(StrContains('invalid KTAP input!'))
+		self.print_mock.assert_any_call(StrContains('could not find any KTAP output!'))
 
 	def test_exec_no_tests(self):
 		self.linux_source_mock.run_kernel = mock.Mock(return_value=['TAP version 14', '1..0'])

base-commit: 13776ebb9964b2ea66ffb8c824c0762eed6da784
-- 
2.35.1.1021.g381101b075-goog

Re: [PATCH] kunit: tool: print clearer error message when there's no TAP output

Posted by Brendan Higgins 4 years, 1 month ago

On Tue, Mar 29, 2022 at 5:43 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> Before:
> $ ./tools/testing/kunit/kunit.py parse /dev/null
> ...
> [ERROR] Test : invalid KTAP input!
>
> After:
> $ ./tools/testing/kunit/kunit.py parse /dev/null
> ...
> [ERROR] Test <missing>: could not find any KTAP output!
>
> This error message gets printed out when extract_tap_output() yielded no
> lines. So while it could be because of malformed KTAP output from KUnit,
> it could also be due to to not having any KTAP output at all.
>
> Try and make the error message here more clear.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

Re: [PATCH] kunit: tool: print clearer error message when there's no TAP output

Posted by David Gow 4 years, 2 months ago

On Wed, Mar 30, 2022 at 5:43 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Before:
> $ ./tools/testing/kunit/kunit.py parse /dev/null
> ...
> [ERROR] Test : invalid KTAP input!
>
> After:
> $ ./tools/testing/kunit/kunit.py parse /dev/null
> ...
> [ERROR] Test <missing>: could not find any KTAP output!
>
> This error message gets printed out when extract_tap_output() yielded no
> lines. So while it could be because of malformed KTAP output from KUnit,
> it could also be due to to not having any KTAP output at all.
>
> Try and make the error message here more clear.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

At first I thought that this was "working as intended", but I agree
that it's a bit confusing, so changing it is for the best.
(And there's no sense getting too bogged down in the philosophical
difference between "invalid TAP" and "not valid TAP" :-))

This works fine here, and the code looks sensible. I tested it with
the --json option as well, and the result ("test_cases" being empty)
makes sense to me:
---
{
   "name": "KUnit Test Group",
   "arch": "UM",
   "defconfig": "kunit_defconfig",
   "build_environment": "",
   "sub_groups": [],
   "test_cases": [],
   "lab_name": null,
   "kernel": null,
   "job": null,
   "git_branch": "kselftest"
}
---

So, this is:
Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

Re: [PATCH] kunit: tool: print clearer error message when there's no TAP output

Posted by Daniel Latypov 4 years, 2 months ago

On Tue, Mar 29, 2022 at 10:17 PM David Gow <davidgow@google.com> wrote:
>
> On Wed, Mar 30, 2022 at 5:43 AM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > Before:
> > $ ./tools/testing/kunit/kunit.py parse /dev/null
> > ...
> > [ERROR] Test : invalid KTAP input!
> >
> > After:
> > $ ./tools/testing/kunit/kunit.py parse /dev/null
> > ...
> > [ERROR] Test <missing>: could not find any KTAP output!
> >
> > This error message gets printed out when extract_tap_output() yielded no
> > lines. So while it could be because of malformed KTAP output from KUnit,
> > it could also be due to to not having any KTAP output at all.
> >
> > Try and make the error message here more clear.
> >
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > ---
>
> At first I thought that this was "working as intended", but I agree
> that it's a bit confusing, so changing it is for the best.
> (And there's no sense getting too bogged down in the philosophical
> difference between "invalid TAP" and "not valid TAP" :-))

extract_tap_output() returning something means it just saw a TAP header.
So this error realistically only happens when there's no TAP at all :P

My allusion to malformed KTAP is in the sense that
 KTA<random stuff>P ver<more stuff>sion 1
is malformed.

We could potentially be more completely precise w/ our error message
and say "no K?TAP header found", but I initially thought this might be
a bit more user-friendly.