diff mbox series

kunit: tool: make --raw_output=kunit (aka --raw_output) preserve leading spaces

Message ID 20220810230258.3933707-1-dlatypov@google.com (mailing list archive)
State Accepted
Commit a15cfa39e8cf9bb20d755978c2f25a9c427bf7b2
Delegated to: Brendan Higgins
Headers show
Series kunit: tool: make --raw_output=kunit (aka --raw_output) preserve leading spaces | expand

Commit Message

Daniel Latypov Aug. 10, 2022, 11:02 p.m. UTC
With
$ kunit.py run --raw_output=all ...
you get the raw output from the kernel, e.g. something like
> TAP version 14
> 1..26
>     # Subtest: time_test_cases
>     1..1
>     ok 1 - time64_to_tm_test_date_range
> ok 1 - time_test_cases

But --raw_output=kunit or equivalently --raw_output, you get
> TAP version 14
> 1..26
> # Subtest: time_test_cases
> 1..1
> ok 1 - time64_to_tm_test_date_range
> ok 1 - time_test_cases

It looks less readable in my opinion, and it also isn't "raw output."

This is due to sharing code with kunit_parser.py, which wants to strip
leading whitespace since it uses anchored regexes.
We could update the kunit_parser.py code to tolerate leaading spaces,
but this patch takes the easier way out and adds a bool flag.

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


base-commit: aeb6e6ac18c73ec287b3b1e2c913520699358c13

Comments

David Gow Aug. 10, 2022, 11:11 p.m. UTC | #1
On Thu, Aug 11, 2022 at 7:03 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> With
> $ kunit.py run --raw_output=all ...
> you get the raw output from the kernel, e.g. something like
> > TAP version 14
> > 1..26
> >     # Subtest: time_test_cases
> >     1..1
> >     ok 1 - time64_to_tm_test_date_range
> > ok 1 - time_test_cases
>
> But --raw_output=kunit or equivalently --raw_output, you get
> > TAP version 14
> > 1..26
> > # Subtest: time_test_cases
> > 1..1
> > ok 1 - time64_to_tm_test_date_range
> > ok 1 - time_test_cases
>
> It looks less readable in my opinion, and it also isn't "raw output."
>
> This is due to sharing code with kunit_parser.py, which wants to strip
> leading whitespace since it uses anchored regexes.
> We could update the kunit_parser.py code to tolerate leaading spaces,
> but this patch takes the easier way out and adds a bool flag.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

Looks good to me. I think we'll probably want to update the parser
code to actually keep track of indentation at some point, as it could
help with some ambiguity which otherwise would exist when there are
nested tests without test plan lines.

For the moment, though, this is definitely an improvement.

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

Cheers,
-- David

>  tools/testing/kunit/kunit.py        |  2 +-
>  tools/testing/kunit/kunit_parser.py | 10 ++++++----
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index e132b0654029..161a3b1b0217 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -206,7 +206,7 @@ def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input
>                 if request.raw_output == 'all':
>                         pass
>                 elif request.raw_output == 'kunit':
> -                       output = kunit_parser.extract_tap_lines(output)
> +                       output = kunit_parser.extract_tap_lines(output, lstrip=False)
>                 for line in output:
>                         print(line.rstrip())
>
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index 12d3ec77f427..1ae873e3e341 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -218,7 +218,7 @@ TAP_START = re.compile(r'TAP version ([0-9]+)$')
>  KTAP_END = re.compile('(List of all partitions:|'
>         'Kernel panic - not syncing: VFS:|reboot: System halted)')
>
> -def extract_tap_lines(kernel_output: Iterable[str]) -> LineStream:
> +def extract_tap_lines(kernel_output: Iterable[str], lstrip=True) -> LineStream:
>         """Extracts KTAP lines from the kernel output."""
>         def isolate_ktap_output(kernel_output: Iterable[str]) \
>                         -> Iterator[Tuple[int, str]]:
> @@ -244,9 +244,11 @@ def extract_tap_lines(kernel_output: Iterable[str]) -> LineStream:
>                                 # stop extracting KTAP lines
>                                 break
>                         elif started:
> -                               # remove prefix and any indention and yield
> -                               # line with line number
> -                               line = line[prefix_len:].lstrip()
> +                               # remove the prefix and optionally any leading
> +                               # whitespace. Our parsing logic relies on this.
> +                               line = line[prefix_len:]
> +                               if lstrip:
> +                                       line = line.lstrip()
>                                 yield line_num, line
>         return LineStream(lines=isolate_ktap_output(kernel_output))
>
>
> base-commit: aeb6e6ac18c73ec287b3b1e2c913520699358c13
> --
> 2.37.1.559.g78731f0fdb-goog
>
diff mbox series

Patch

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index e132b0654029..161a3b1b0217 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -206,7 +206,7 @@  def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input
 		if request.raw_output == 'all':
 			pass
 		elif request.raw_output == 'kunit':
-			output = kunit_parser.extract_tap_lines(output)
+			output = kunit_parser.extract_tap_lines(output, lstrip=False)
 		for line in output:
 			print(line.rstrip())
 
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 12d3ec77f427..1ae873e3e341 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -218,7 +218,7 @@  TAP_START = re.compile(r'TAP version ([0-9]+)$')
 KTAP_END = re.compile('(List of all partitions:|'
 	'Kernel panic - not syncing: VFS:|reboot: System halted)')
 
-def extract_tap_lines(kernel_output: Iterable[str]) -> LineStream:
+def extract_tap_lines(kernel_output: Iterable[str], lstrip=True) -> LineStream:
 	"""Extracts KTAP lines from the kernel output."""
 	def isolate_ktap_output(kernel_output: Iterable[str]) \
 			-> Iterator[Tuple[int, str]]:
@@ -244,9 +244,11 @@  def extract_tap_lines(kernel_output: Iterable[str]) -> LineStream:
 				# stop extracting KTAP lines
 				break
 			elif started:
-				# remove prefix and any indention and yield
-				# line with line number
-				line = line[prefix_len:].lstrip()
+				# remove the prefix and optionally any leading
+				# whitespace. Our parsing logic relies on this.
+				line = line[prefix_len:]
+				if lstrip:
+					line = line.lstrip()
 				yield line_num, line
 	return LineStream(lines=isolate_ktap_output(kernel_output))