linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header
@ 2021-06-29 22:35 Rae Moar
  2021-06-29 22:57 ` David Gow
  2021-06-29 23:13 ` Daniel Latypov
  0 siblings, 2 replies; 5+ messages in thread
From: Rae Moar @ 2021-06-29 22:35 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov, shuah
  Cc: kunit-dev, linux-kselftest, linux-kernel, Rae Moar

In the case of the TAP output having an incorrect header format, the
parser used to output an error message of 'no tests run!'. Additionally,
in the case of TAP output with the correct header but no tests, the
parser used to output an error message of 'could not parse test
results!'.  This patch corrects the error messages for these two cases
by switching the original outputted error messages and correcting the
tests in kunit_toot_test.py.

Signed-off-by: Rae Moar <rmoar@google.com>
---
 tools/testing/kunit/kunit_parser.py           |  6 +-
 tools/testing/kunit/kunit_tool_test.py        | 16 +++-
 ...is_test_passed-no_tests_run_no_header.log} |  0
 ...s_test_passed-no_tests_run_with_header.log | 77 +++++++++++++++++++
 4 files changed, 94 insertions(+), 5 deletions(-)
 rename tools/testing/kunit/test_data/{test_is_test_passed-no_tests_run.log => test_is_test_passed-no_tests_run_no_header.log} (100%)
 create mode 100644 tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index c3c524b79db8..b88db3f51dc5 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -338,9 +338,11 @@ def bubble_up_suite_errors(test_suites: Iterable[TestSuite]) -> TestStatus:
 def parse_test_result(lines: LineStream) -> TestResult:
 	consume_non_diagnostic(lines)
 	if not lines or not parse_tap_header(lines):
-		return TestResult(TestStatus.NO_TESTS, [], lines)
+		return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
 	expected_test_suite_num = parse_test_plan(lines)
-	if not expected_test_suite_num:
+	if expected_test_suite_num == 0:
+		return TestResult(TestStatus.NO_TESTS, [], lines)
+	elif expected_test_suite_num is None:
 		return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
 	test_suites = []
 	for i in range(1, expected_test_suite_num + 1):
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index bdae0e5f6197..75045aa0f8a1 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -157,8 +157,18 @@ class KUnitParserTest(unittest.TestCase):
 			kunit_parser.TestStatus.FAILURE,
 			result.status)
 
+	def test_no_header(self):
+		empty_log = test_data_path('test_is_test_passed-no_tests_run_no_header.log')
+		with open(empty_log) as file:
+			result = kunit_parser.parse_run_tests(
+				kunit_parser.extract_tap_lines(file.readlines()))
+		self.assertEqual(0, len(result.suites))
+		self.assertEqual(
+			kunit_parser.TestStatus.FAILURE_TO_PARSE_TESTS,
+			result.status)
+
 	def test_no_tests(self):
-		empty_log = test_data_path('test_is_test_passed-no_tests_run.log')
+		empty_log = test_data_path('test_is_test_passed-no_tests_run_with_header.log')
 		with open(empty_log) as file:
 			result = kunit_parser.parse_run_tests(
 				kunit_parser.extract_tap_lines(file.readlines()))
@@ -173,7 +183,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('no tests run!'))
+		print_mock.assert_any_call(StrContains('could not parse test results!'))
 		print_mock.stop()
 		file.close()
 
@@ -309,7 +319,7 @@ class KUnitJsonTest(unittest.TestCase):
 			result["sub_groups"][1]["test_cases"][0])
 
 	def test_no_tests_json(self):
-		result = self._json_for('test_is_test_passed-no_tests_run.log')
+		result = self._json_for('test_is_test_passed-no_tests_run_with_header.log')
 		self.assertEqual(0, len(result['sub_groups']))
 
 class StrContains(str):
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
similarity index 100%
rename from tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
rename to tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
new file mode 100644
index 000000000000..18215b236783
--- /dev/null
+++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
@@ -0,0 +1,77 @@
+Core dump limits :
+	soft - 0
+	hard - NONE
+Checking environment variables for a tempdir...none found
+Checking if /dev/shm is on tmpfs...OK
+Checking PROT_EXEC mmap in /dev/shm...OK
+Adding 24743936 bytes to physical memory to account for exec-shield gap
+Linux version 4.12.0-rc3-00010-g7319eb35f493-dirty (brendanhiggins@mactruck.svl.corp.google.com) (gcc version 7.3.0 (Debian 7.3.0-5) ) #29 Thu Mar 15 14:57:19 PDT 2018
+Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 14038
+Kernel command line: root=98:0
+PID hash table entries: 256 (order: -1, 2048 bytes)
+Dentry cache hash table entries: 8192 (order: 4, 65536 bytes)
+Inode-cache hash table entries: 4096 (order: 3, 32768 bytes)
+Memory: 27868K/56932K available (1681K kernel code, 480K rwdata, 400K rodata, 89K init, 205K bss, 29064K reserved, 0K cma-reserved)
+SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
+NR_IRQS:15
+clocksource: timer: mask: 0xffffffffffffffff max_cycles: 0x1cd42e205, max_idle_ns: 881590404426 ns
+Calibrating delay loop... 7384.26 BogoMIPS (lpj=36921344)
+pid_max: default: 32768 minimum: 301
+Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
+Mountpoint-cache hash table entries: 512 (order: 0, 4096 bytes)
+Checking that host ptys support output SIGIO...Yes
+Checking that host ptys support SIGIO on close...No, enabling workaround
+Using 2.6 host AIO
+clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
+futex hash table entries: 256 (order: 0, 6144 bytes)
+clocksource: Switched to clocksource timer
+console [stderr0] disabled
+mconsole (version 2) initialized on /usr/local/google/home/brendanhiggins/.uml/6Ijecl/mconsole
+Checking host MADV_REMOVE support...OK
+workingset: timestamp_bits=62 max_order=13 bucket_order=0
+Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254)
+io scheduler noop registered
+io scheduler deadline registered
+io scheduler cfq registered (default)
+io scheduler mq-deadline registered
+io scheduler kyber registered
+Initialized stdio console driver
+Using a channel type which is configured out of UML
+setup_one_line failed for device 1 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 2 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 3 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 4 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 5 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 6 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 7 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 8 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 9 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 10 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 11 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 12 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 13 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 14 : Configuration failed
+Using a channel type which is configured out of UML
+setup_one_line failed for device 15 : Configuration failed
+Console initialized on /dev/tty0
+console [tty0] enabled
+console [mc-1] enabled
+TAP version 14
+1..0
+List of all partitions:
+No filesystem could mount root, tried:
+
+Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0)
-- 
2.32.0.93.g670b81a890-goog


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

* Re: [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header
  2021-06-29 22:35 [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header Rae Moar
@ 2021-06-29 22:57 ` David Gow
  2021-06-29 23:13 ` Daniel Latypov
  1 sibling, 0 replies; 5+ messages in thread
From: David Gow @ 2021-06-29 22:57 UTC (permalink / raw)
  To: Rae Moar
  Cc: Brendan Higgins, Daniel Latypov, Shuah Khan, KUnit Development,
	open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List

On Wed, Jun 30, 2021 at 6:36 AM Rae Moar <rmoar@google.com> wrote:
>
> In the case of the TAP output having an incorrect header format, the
> parser used to output an error message of 'no tests run!'. Additionally,
> in the case of TAP output with the correct header but no tests, the
> parser used to output an error message of 'could not parse test
> results!'.  This patch corrects the error messages for these two cases
> by switching the original outputted error messages and correcting the
> tests in kunit_toot_test.py.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---

These error messages make a lot more sense, thank you!

With this patch, we can close
https://bugzilla.kernel.org/show_bug.cgi?id=207923

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

Cheers,
-- David

>  tools/testing/kunit/kunit_parser.py           |  6 +-
>  tools/testing/kunit/kunit_tool_test.py        | 16 +++-
>  ...is_test_passed-no_tests_run_no_header.log} |  0
>  ...s_test_passed-no_tests_run_with_header.log | 77 +++++++++++++++++++
>  4 files changed, 94 insertions(+), 5 deletions(-)
>  rename tools/testing/kunit/test_data/{test_is_test_passed-no_tests_run.log => test_is_test_passed-no_tests_run_no_header.log} (100%)
>  create mode 100644 tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
>
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index c3c524b79db8..b88db3f51dc5 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -338,9 +338,11 @@ def bubble_up_suite_errors(test_suites: Iterable[TestSuite]) -> TestStatus:
>  def parse_test_result(lines: LineStream) -> TestResult:
>         consume_non_diagnostic(lines)
>         if not lines or not parse_tap_header(lines):
> -               return TestResult(TestStatus.NO_TESTS, [], lines)
> +               return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
>         expected_test_suite_num = parse_test_plan(lines)
> -       if not expected_test_suite_num:
> +       if expected_test_suite_num == 0:
> +               return TestResult(TestStatus.NO_TESTS, [], lines)
> +       elif expected_test_suite_num is None:
>                 return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
>         test_suites = []
>         for i in range(1, expected_test_suite_num + 1):
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index bdae0e5f6197..75045aa0f8a1 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -157,8 +157,18 @@ class KUnitParserTest(unittest.TestCase):
>                         kunit_parser.TestStatus.FAILURE,
>                         result.status)
>
> +       def test_no_header(self):
> +               empty_log = test_data_path('test_is_test_passed-no_tests_run_no_header.log')
> +               with open(empty_log) as file:
> +                       result = kunit_parser.parse_run_tests(
> +                               kunit_parser.extract_tap_lines(file.readlines()))
> +               self.assertEqual(0, len(result.suites))
> +               self.assertEqual(
> +                       kunit_parser.TestStatus.FAILURE_TO_PARSE_TESTS,
> +                       result.status)
> +
>         def test_no_tests(self):
> -               empty_log = test_data_path('test_is_test_passed-no_tests_run.log')
> +               empty_log = test_data_path('test_is_test_passed-no_tests_run_with_header.log')
>                 with open(empty_log) as file:
>                         result = kunit_parser.parse_run_tests(
>                                 kunit_parser.extract_tap_lines(file.readlines()))
> @@ -173,7 +183,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('no tests run!'))
> +               print_mock.assert_any_call(StrContains('could not parse test results!'))
>                 print_mock.stop()
>                 file.close()
>
> @@ -309,7 +319,7 @@ class KUnitJsonTest(unittest.TestCase):
>                         result["sub_groups"][1]["test_cases"][0])
>
>         def test_no_tests_json(self):
> -               result = self._json_for('test_is_test_passed-no_tests_run.log')
> +               result = self._json_for('test_is_test_passed-no_tests_run_with_header.log')
>                 self.assertEqual(0, len(result['sub_groups']))
>
>  class StrContains(str):
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
> similarity index 100%
> rename from tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
> rename to tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
> new file mode 100644
> index 000000000000..18215b236783
> --- /dev/null
> +++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
> @@ -0,0 +1,77 @@
> +Core dump limits :
> +       soft - 0
> +       hard - NONE
> +Checking environment variables for a tempdir...none found
> +Checking if /dev/shm is on tmpfs...OK
> +Checking PROT_EXEC mmap in /dev/shm...OK
> +Adding 24743936 bytes to physical memory to account for exec-shield gap
> +Linux version 4.12.0-rc3-00010-g7319eb35f493-dirty (brendanhiggins@mactruck.svl.corp.google.com) (gcc version 7.3.0 (Debian 7.3.0-5) ) #29 Thu Mar 15 14:57:19 PDT 2018
> +Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 14038
> +Kernel command line: root=98:0
> +PID hash table entries: 256 (order: -1, 2048 bytes)
> +Dentry cache hash table entries: 8192 (order: 4, 65536 bytes)
> +Inode-cache hash table entries: 4096 (order: 3, 32768 bytes)
> +Memory: 27868K/56932K available (1681K kernel code, 480K rwdata, 400K rodata, 89K init, 205K bss, 29064K reserved, 0K cma-reserved)
> +SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> +NR_IRQS:15
> +clocksource: timer: mask: 0xffffffffffffffff max_cycles: 0x1cd42e205, max_idle_ns: 881590404426 ns
> +Calibrating delay loop... 7384.26 BogoMIPS (lpj=36921344)
> +pid_max: default: 32768 minimum: 301
> +Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
> +Mountpoint-cache hash table entries: 512 (order: 0, 4096 bytes)
> +Checking that host ptys support output SIGIO...Yes
> +Checking that host ptys support SIGIO on close...No, enabling workaround
> +Using 2.6 host AIO
> +clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
> +futex hash table entries: 256 (order: 0, 6144 bytes)
> +clocksource: Switched to clocksource timer
> +console [stderr0] disabled
> +mconsole (version 2) initialized on /usr/local/google/home/brendanhiggins/.uml/6Ijecl/mconsole
> +Checking host MADV_REMOVE support...OK
> +workingset: timestamp_bits=62 max_order=13 bucket_order=0
> +Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254)
> +io scheduler noop registered
> +io scheduler deadline registered
> +io scheduler cfq registered (default)
> +io scheduler mq-deadline registered
> +io scheduler kyber registered
> +Initialized stdio console driver
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 1 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 2 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 3 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 4 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 5 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 6 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 7 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 8 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 9 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 10 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 11 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 12 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 13 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 14 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 15 : Configuration failed
> +Console initialized on /dev/tty0
> +console [tty0] enabled
> +console [mc-1] enabled
> +TAP version 14
> +1..0
> +List of all partitions:
> +No filesystem could mount root, tried:
> +
> +Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0)
> --
> 2.32.0.93.g670b81a890-goog
>

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

* Re: [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header
  2021-06-29 22:35 [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header Rae Moar
  2021-06-29 22:57 ` David Gow
@ 2021-06-29 23:13 ` Daniel Latypov
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Latypov @ 2021-06-29 23:13 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, shuah, kunit-dev, linux-kselftest,
	linux-kernel

On Tue, Jun 29, 2021 at 3:36 PM Rae Moar <rmoar@google.com> wrote:
>
> In the case of the TAP output having an incorrect header format, the
> parser used to output an error message of 'no tests run!'. Additionally,
> in the case of TAP output with the correct header but no tests, the
> parser used to output an error message of 'could not parse test
> results!'.  This patch corrects the error messages for these two cases
> by switching the original outputted error messages and correcting the
> tests in kunit_toot_test.py.

You might want to include an example like this in your commit description:

Before:
$ ./tools/testing/kunit/kunit.py parse /dev/null
[ERROR] no tests run!
...

After:
$ ./tools/testing/kunit/kunit.py parse /dev/null
[ERROR] could not parse test results!
...

We could also include an example with a header but 0 tests, but I
think /dev/null illustrates this enough.
But if we wanted to:

Before:
$ echo -e 'TAP version 14\n1..0' | ./tools/testing/kunit/kunit.py parse
[ERROR] could not parse test results!

After:
$ echo -e 'TAP version 14\n1..0' | ./tools/testing/kunit/kunit.py parse
[ERROR] no tests run!


>
> Signed-off-by: Rae Moar <rmoar@google.com>

Reviewed-by: Daniel Latypov <dlatypov@google.com>

Looks good to me.
One minor nit/request about the new test log we've added.

> ---
>  tools/testing/kunit/kunit_parser.py           |  6 +-
>  tools/testing/kunit/kunit_tool_test.py        | 16 +++-
>  ...is_test_passed-no_tests_run_no_header.log} |  0
>  ...s_test_passed-no_tests_run_with_header.log | 77 +++++++++++++++++++
>  4 files changed, 94 insertions(+), 5 deletions(-)
>  rename tools/testing/kunit/test_data/{test_is_test_passed-no_tests_run.log => test_is_test_passed-no_tests_run_no_header.log} (100%)
>  create mode 100644 tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
>
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index c3c524b79db8..b88db3f51dc5 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -338,9 +338,11 @@ def bubble_up_suite_errors(test_suites: Iterable[TestSuite]) -> TestStatus:
>  def parse_test_result(lines: LineStream) -> TestResult:
>         consume_non_diagnostic(lines)
>         if not lines or not parse_tap_header(lines):
> -               return TestResult(TestStatus.NO_TESTS, [], lines)
> +               return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
>         expected_test_suite_num = parse_test_plan(lines)
> -       if not expected_test_suite_num:
> +       if expected_test_suite_num == 0:
> +               return TestResult(TestStatus.NO_TESTS, [], lines)
> +       elif expected_test_suite_num is None:
>                 return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
>         test_suites = []
>         for i in range(1, expected_test_suite_num + 1):
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index bdae0e5f6197..75045aa0f8a1 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -157,8 +157,18 @@ class KUnitParserTest(unittest.TestCase):
>                         kunit_parser.TestStatus.FAILURE,
>                         result.status)
>
> +       def test_no_header(self):
> +               empty_log = test_data_path('test_is_test_passed-no_tests_run_no_header.log')
> +               with open(empty_log) as file:
> +                       result = kunit_parser.parse_run_tests(
> +                               kunit_parser.extract_tap_lines(file.readlines()))
> +               self.assertEqual(0, len(result.suites))
> +               self.assertEqual(
> +                       kunit_parser.TestStatus.FAILURE_TO_PARSE_TESTS,
> +                       result.status)
> +
>         def test_no_tests(self):
> -               empty_log = test_data_path('test_is_test_passed-no_tests_run.log')
> +               empty_log = test_data_path('test_is_test_passed-no_tests_run_with_header.log')
>                 with open(empty_log) as file:
>                         result = kunit_parser.parse_run_tests(
>                                 kunit_parser.extract_tap_lines(file.readlines()))
> @@ -173,7 +183,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('no tests run!'))
> +               print_mock.assert_any_call(StrContains('could not parse test results!'))
>                 print_mock.stop()
>                 file.close()
>
> @@ -309,7 +319,7 @@ class KUnitJsonTest(unittest.TestCase):
>                         result["sub_groups"][1]["test_cases"][0])
>
>         def test_no_tests_json(self):
> -               result = self._json_for('test_is_test_passed-no_tests_run.log')
> +               result = self._json_for('test_is_test_passed-no_tests_run_with_header.log')
>                 self.assertEqual(0, len(result['sub_groups']))
>
>  class StrContains(str):
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
> similarity index 100%
> rename from tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
> rename to tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
> new file mode 100644
> index 000000000000..18215b236783
> --- /dev/null
> +++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log

Can we truncate this log down to a smaller one w/ just the essential bits?
I don't know that printing out lines about Brendan's workstation is
necessarily relevant to the test :)

The main part we need is

+TAP version 14
+1..0

I know this is just copying from what we had before in the "no_header"
version, but it feels a bit excessive to have all this.

> @@ -0,0 +1,77 @@
> +Core dump limits :
> +       soft - 0
> +       hard - NONE
> +Checking environment variables for a tempdir...none found
> +Checking if /dev/shm is on tmpfs...OK
> +Checking PROT_EXEC mmap in /dev/shm...OK
> +Adding 24743936 bytes to physical memory to account for exec-shield gap
> +Linux version 4.12.0-rc3-00010-g7319eb35f493-dirty (brendanhiggins@mactruck.svl.corp.google.com) (gcc version 7.3.0 (Debian 7.3.0-5) ) #29 Thu Mar 15 14:57:19 PDT 2018
> +Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 14038
> +Kernel command line: root=98:0
> +PID hash table entries: 256 (order: -1, 2048 bytes)
> +Dentry cache hash table entries: 8192 (order: 4, 65536 bytes)
> +Inode-cache hash table entries: 4096 (order: 3, 32768 bytes)
> +Memory: 27868K/56932K available (1681K kernel code, 480K rwdata, 400K rodata, 89K init, 205K bss, 29064K reserved, 0K cma-reserved)
> +SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> +NR_IRQS:15
> +clocksource: timer: mask: 0xffffffffffffffff max_cycles: 0x1cd42e205, max_idle_ns: 881590404426 ns
> +Calibrating delay loop... 7384.26 BogoMIPS (lpj=36921344)
> +pid_max: default: 32768 minimum: 301
> +Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
> +Mountpoint-cache hash table entries: 512 (order: 0, 4096 bytes)
> +Checking that host ptys support output SIGIO...Yes
> +Checking that host ptys support SIGIO on close...No, enabling workaround
> +Using 2.6 host AIO
> +clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
> +futex hash table entries: 256 (order: 0, 6144 bytes)
> +clocksource: Switched to clocksource timer
> +console [stderr0] disabled
> +mconsole (version 2) initialized on /usr/local/google/home/brendanhiggins/.uml/6Ijecl/mconsole
> +Checking host MADV_REMOVE support...OK
> +workingset: timestamp_bits=62 max_order=13 bucket_order=0
> +Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254)
> +io scheduler noop registered
> +io scheduler deadline registered
> +io scheduler cfq registered (default)
> +io scheduler mq-deadline registered
> +io scheduler kyber registered
> +Initialized stdio console driver
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 1 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 2 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 3 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 4 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 5 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 6 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 7 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 8 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 9 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 10 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 11 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 12 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 13 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 14 : Configuration failed
> +Using a channel type which is configured out of UML
> +setup_one_line failed for device 15 : Configuration failed
> +Console initialized on /dev/tty0
> +console [tty0] enabled
> +console [mc-1] enabled
> +TAP version 14
> +1..0
> +List of all partitions:
> +No filesystem could mount root, tried:
> +
> +Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0)
> --
> 2.32.0.93.g670b81a890-goog
>

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

* Re: [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header
  2021-06-30 23:39 Rae Moar
@ 2021-07-01  0:48 ` Daniel Latypov
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Latypov @ 2021-07-01  0:48 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, shuah, kunit-dev, linux-kselftest,
	linux-kernel

On Wed, Jun 30, 2021 at 4:40 PM Rae Moar <rmoar@google.com> wrote:
>
> This patch addresses misleading error messages reported by kunit_tool in
> two cases. First, in the case of TAP output having an incorrect header
> format or missing a header, the parser used to output an error message of
> 'no tests run!'. Now the parser outputs an error message of 'could not
> parse test results!'.
>
> As an example:
>
> Before:
> $ ./tools/testing/kunit/kunit.py parse /dev/null
> [ERROR] no tests run!
> ...
>
> After:
> $ ./tools/testing/kunit/kunit.py parse /dev/null
> [ERROR] could not parse test results!
> ...
>
> Second, in the case of TAP output with the correct header but no
> tests, the parser used to output an error message of 'could not parse
> test results!'. Now the parser outputs an error message of 'no tests
> run!'.
>
> As an example:
>
> Before:
> $ echo -e 'TAP version 14\n1..0' | ./tools/testing/kunit/kunit.py parse
> [ERROR] could not parse test results!
>
> After:
> $ echo -e 'TAP version 14\n1..0' | ./tools/testing/kunit/kunit.py parse
> [ERROR] no tests run!
>
> Additionally, this patch also corrects the tests in kunit_tool_test.py
> and adds a test to check the error in the case of TAP output with the
> correct header but no tests (the log for this test was simplified from
> the first version of this patch).

Just as a note: usually people will mention changes between versions
of a patch series just above the diffstat.
Picking a random example from linux-kselftest:
https://lore.kernel.org/linux-kselftest/20210630081202.4423-7-yangbo.lu@nxp.com/

Note the extra "---" above those change notes, just like above the diffstat.

>
> Signed-off-by: Rae Moar <rmoar@google.com>

Reviewed-by: Daniel Latypov <dlatypov@google.com>

Thanks! Looks good to me.

Just a note to others, this is a v2 of
https://lore.kernel.org/linux-kselftest/20210629223541.1512296-1-rmoar@google.com/

> ---
>  tools/testing/kunit/kunit_parser.py              |  6 ++++--
>  tools/testing/kunit/kunit_tool_test.py           | 16 +++++++++++++---
>  ...st_is_test_passed-no_tests_run_no_header.log} |  0
>  ...t_is_test_passed-no_tests_run_with_header.log |  2 ++
>  4 files changed, 19 insertions(+), 5 deletions(-)
>  rename tools/testing/kunit/test_data/{test_is_test_passed-no_tests_run.log => test_is_test_passed-no_tests_run_no_header.log} (100%)
>  create mode 100644 tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
>
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index c3c524b79db8..b88db3f51dc5 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -338,9 +338,11 @@ def bubble_up_suite_errors(test_suites: Iterable[TestSuite]) -> TestStatus:
>  def parse_test_result(lines: LineStream) -> TestResult:
>         consume_non_diagnostic(lines)
>         if not lines or not parse_tap_header(lines):
> -               return TestResult(TestStatus.NO_TESTS, [], lines)
> +               return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
>         expected_test_suite_num = parse_test_plan(lines)
> -       if not expected_test_suite_num:
> +       if expected_test_suite_num == 0:
> +               return TestResult(TestStatus.NO_TESTS, [], lines)
> +       elif expected_test_suite_num is None:
>                 return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
>         test_suites = []
>         for i in range(1, expected_test_suite_num + 1):
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index bdae0e5f6197..75045aa0f8a1 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -157,8 +157,18 @@ class KUnitParserTest(unittest.TestCase):
>                         kunit_parser.TestStatus.FAILURE,
>                         result.status)
>
> +       def test_no_header(self):
> +               empty_log = test_data_path('test_is_test_passed-no_tests_run_no_header.log')
> +               with open(empty_log) as file:
> +                       result = kunit_parser.parse_run_tests(
> +                               kunit_parser.extract_tap_lines(file.readlines()))
> +               self.assertEqual(0, len(result.suites))
> +               self.assertEqual(
> +                       kunit_parser.TestStatus.FAILURE_TO_PARSE_TESTS,
> +                       result.status)
> +
>         def test_no_tests(self):
> -               empty_log = test_data_path('test_is_test_passed-no_tests_run.log')
> +               empty_log = test_data_path('test_is_test_passed-no_tests_run_with_header.log')
>                 with open(empty_log) as file:
>                         result = kunit_parser.parse_run_tests(
>                                 kunit_parser.extract_tap_lines(file.readlines()))
> @@ -173,7 +183,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('no tests run!'))
> +               print_mock.assert_any_call(StrContains('could not parse test results!'))
>                 print_mock.stop()
>                 file.close()
>
> @@ -309,7 +319,7 @@ class KUnitJsonTest(unittest.TestCase):
>                         result["sub_groups"][1]["test_cases"][0])
>
>         def test_no_tests_json(self):
> -               result = self._json_for('test_is_test_passed-no_tests_run.log')
> +               result = self._json_for('test_is_test_passed-no_tests_run_with_header.log')
>                 self.assertEqual(0, len(result['sub_groups']))
>
>  class StrContains(str):
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
> similarity index 100%
> rename from tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
> rename to tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
> new file mode 100644
> index 000000000000..5f48ee659d40
> --- /dev/null
> +++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
> @@ -0,0 +1,2 @@
> +TAP version 14
> +1..0
> --
> 2.32.0.93.g670b81a890-goog
>

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

* [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header
@ 2021-06-30 23:39 Rae Moar
  2021-07-01  0:48 ` Daniel Latypov
  0 siblings, 1 reply; 5+ messages in thread
From: Rae Moar @ 2021-06-30 23:39 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov, shuah
  Cc: kunit-dev, linux-kselftest, linux-kernel, Rae Moar

This patch addresses misleading error messages reported by kunit_tool in
two cases. First, in the case of TAP output having an incorrect header
format or missing a header, the parser used to output an error message of
'no tests run!'. Now the parser outputs an error message of 'could not
parse test results!'.

As an example:

Before:
$ ./tools/testing/kunit/kunit.py parse /dev/null
[ERROR] no tests run!
...

After:
$ ./tools/testing/kunit/kunit.py parse /dev/null
[ERROR] could not parse test results!
...

Second, in the case of TAP output with the correct header but no
tests, the parser used to output an error message of 'could not parse
test results!'. Now the parser outputs an error message of 'no tests
run!'.

As an example:

Before:
$ echo -e 'TAP version 14\n1..0' | ./tools/testing/kunit/kunit.py parse
[ERROR] could not parse test results!

After:
$ echo -e 'TAP version 14\n1..0' | ./tools/testing/kunit/kunit.py parse
[ERROR] no tests run!

Additionally, this patch also corrects the tests in kunit_tool_test.py
and adds a test to check the error in the case of TAP output with the
correct header but no tests (the log for this test was simplified from
the first version of this patch).

Signed-off-by: Rae Moar <rmoar@google.com>
---
 tools/testing/kunit/kunit_parser.py              |  6 ++++--
 tools/testing/kunit/kunit_tool_test.py           | 16 +++++++++++++---
 ...st_is_test_passed-no_tests_run_no_header.log} |  0
 ...t_is_test_passed-no_tests_run_with_header.log |  2 ++
 4 files changed, 19 insertions(+), 5 deletions(-)
 rename tools/testing/kunit/test_data/{test_is_test_passed-no_tests_run.log => test_is_test_passed-no_tests_run_no_header.log} (100%)
 create mode 100644 tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index c3c524b79db8..b88db3f51dc5 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -338,9 +338,11 @@ def bubble_up_suite_errors(test_suites: Iterable[TestSuite]) -> TestStatus:
 def parse_test_result(lines: LineStream) -> TestResult:
 	consume_non_diagnostic(lines)
 	if not lines or not parse_tap_header(lines):
-		return TestResult(TestStatus.NO_TESTS, [], lines)
+		return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
 	expected_test_suite_num = parse_test_plan(lines)
-	if not expected_test_suite_num:
+	if expected_test_suite_num == 0:
+		return TestResult(TestStatus.NO_TESTS, [], lines)
+	elif expected_test_suite_num is None:
 		return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
 	test_suites = []
 	for i in range(1, expected_test_suite_num + 1):
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index bdae0e5f6197..75045aa0f8a1 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -157,8 +157,18 @@ class KUnitParserTest(unittest.TestCase):
 			kunit_parser.TestStatus.FAILURE,
 			result.status)
 
+	def test_no_header(self):
+		empty_log = test_data_path('test_is_test_passed-no_tests_run_no_header.log')
+		with open(empty_log) as file:
+			result = kunit_parser.parse_run_tests(
+				kunit_parser.extract_tap_lines(file.readlines()))
+		self.assertEqual(0, len(result.suites))
+		self.assertEqual(
+			kunit_parser.TestStatus.FAILURE_TO_PARSE_TESTS,
+			result.status)
+
 	def test_no_tests(self):
-		empty_log = test_data_path('test_is_test_passed-no_tests_run.log')
+		empty_log = test_data_path('test_is_test_passed-no_tests_run_with_header.log')
 		with open(empty_log) as file:
 			result = kunit_parser.parse_run_tests(
 				kunit_parser.extract_tap_lines(file.readlines()))
@@ -173,7 +183,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('no tests run!'))
+		print_mock.assert_any_call(StrContains('could not parse test results!'))
 		print_mock.stop()
 		file.close()
 
@@ -309,7 +319,7 @@ class KUnitJsonTest(unittest.TestCase):
 			result["sub_groups"][1]["test_cases"][0])
 
 	def test_no_tests_json(self):
-		result = self._json_for('test_is_test_passed-no_tests_run.log')
+		result = self._json_for('test_is_test_passed-no_tests_run_with_header.log')
 		self.assertEqual(0, len(result['sub_groups']))
 
 class StrContains(str):
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
similarity index 100%
rename from tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
rename to tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_no_header.log
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
new file mode 100644
index 000000000000..5f48ee659d40
--- /dev/null
+++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_run_with_header.log
@@ -0,0 +1,2 @@
+TAP version 14
+1..0
-- 
2.32.0.93.g670b81a890-goog


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

end of thread, other threads:[~2021-07-01  0:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29 22:35 [PATCH] kunit: tool: Fix error messages for cases of no tests and wrong TAP header Rae Moar
2021-06-29 22:57 ` David Gow
2021-06-29 23:13 ` Daniel Latypov
2021-06-30 23:39 Rae Moar
2021-07-01  0:48 ` Daniel Latypov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).