2022-08-10 23:32:12

by Daniel Latypov

[permalink] [raw]
Subject: [PATCH] kunit: tool: make --raw_output=kunit (aka --raw_output) preserve leading spaces

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 <[email protected]>
---
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


2022-08-11 00:04:50

by David Gow

[permalink] [raw]
Subject: Re: [PATCH] kunit: tool: make --raw_output=kunit (aka --raw_output) preserve leading spaces

On Thu, Aug 11, 2022 at 7:03 AM Daniel Latypov <[email protected]> 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 <[email protected]>
> ---

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 <[email protected]>

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
>


Attachments:
smime.p7s (3.91 kB)
S/MIME Cryptographic Signature