* [PATCH] kunit: tool: make --raw_output only support showing kunit output
@ 2021-09-16 22:39 Daniel Latypov
2021-09-21 16:19 ` David Gow
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Latypov @ 2021-09-16 22:39 UTC (permalink / raw)
To: brendanhiggins, davidgow
Cc: linux-kernel, kunit-dev, linux-kselftest, skhan, Daniel Latypov
Commit 6a499c9c42d0 ("kunit: tool: make --raw_output support only
showing kunit output") made --raw_output a string-typed argument.
Passing --raw_output=kunit would make it only show KUnit-related output
and not everything.
However, converting it to a string-typed argument had side effects.
These calls used to work:
$ kunit.py run --raw_output
$ kunit.py run --raw_output suite_filter
$ kunit.py run suite_filter --raw_output
But now the second is actually parsed as
$ kunit.py run --raw_output=suite_filter
So the order you add in --raw_output now matters and command lines that
used to work might not anymore.
Change --raw_output back to a boolean flag, but change its behavior to
match that of the former --raw_output=kunit.
The assumption is that this is what most people wanted to see anyways.
To get the old behavior, users can simply do:
$ kunit.py run >/dev/null; cat .kunit/test.log
They don't have any easy way of getting the --raw_output=kunit behavior.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
Meta: this is an alternative to
https://lore.kernel.org/linux-kselftest/20210903161405.1861312-1-dlatypov@google.com/
I'd slightly prefer that approach, but if we're fine with giving up the
old --raw_output semantics entirely, this would be cleaner.
I'd also assume that most people would prefer the new semantics, but I'm
not sure of that.
---
Documentation/dev-tools/kunit/kunit-tool.rst | 7 -------
tools/testing/kunit/kunit.py | 12 +++---------
tools/testing/kunit/kunit_tool_test.py | 13 ++++++-------
3 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/Documentation/dev-tools/kunit/kunit-tool.rst b/Documentation/dev-tools/kunit/kunit-tool.rst
index ae52e0f489f9..03404746f1f6 100644
--- a/Documentation/dev-tools/kunit/kunit-tool.rst
+++ b/Documentation/dev-tools/kunit/kunit-tool.rst
@@ -114,13 +114,6 @@ results in TAP format, you can pass the ``--raw_output`` argument.
./tools/testing/kunit/kunit.py run --raw_output
-The raw output from test runs may contain other, non-KUnit kernel log
-lines. You can see just KUnit output with ``--raw_output=kunit``:
-
-.. code-block:: bash
-
- ./tools/testing/kunit/kunit.py run --raw_output=kunit
-
If you have KUnit results in their raw TAP format, you can parse them and print
the human-readable summary with the ``parse`` command for kunit_tool. This
accepts a filename for an argument, or will read from standard input.
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 5a931456e718..3626a56472b5 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -115,13 +115,7 @@ def parse_tests(request: KunitParseRequest) -> KunitResult:
'Tests not Parsed.')
if request.raw_output:
- output: Iterable[str] = request.input_data
- if request.raw_output == 'all':
- pass
- elif request.raw_output == 'kunit':
- output = kunit_parser.extract_tap_lines(output)
- else:
- print(f'Unknown --raw_output option "{request.raw_output}"', file=sys.stderr)
+ output = kunit_parser.extract_tap_lines(request.input_data)
for line in output:
print(line.rstrip())
@@ -256,8 +250,8 @@ def add_exec_opts(parser) -> None:
def add_parse_opts(parser) -> None:
parser.add_argument('--raw_output', help='If set don\'t format output from kernel. '
- 'If set to --raw_output=kunit, filters to just KUnit output.',
- type=str, nargs='?', const='all', default=None)
+ 'It will only show output from KUnit.',
+ action='store_true')
parser.add_argument('--json',
nargs='?',
help='Stores test results in a JSON, and either '
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 619c4554cbff..55ed3dac31ee 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -399,14 +399,13 @@ class KUnitMainTest(unittest.TestCase):
self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
- def test_run_raw_output_kunit(self):
+ def test_run_raw_output_does_not_take_positional_args(self):
+ # --raw_output might eventually support an argument, but we don't want it
+ # to consume any positional arguments, only ones after an '='.
self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
- kunit.main(['run', '--raw_output=kunit'], self.linux_source_mock)
- self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
- self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
- for call in self.print_mock.call_args_list:
- self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
- self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
+ kunit.main(['run', '--raw_output', 'filter_glob'], self.linux_source_mock)
+ self.linux_source_mock.run_kernel.assert_called_once_with(
+ args=None, build_dir='.kunit', filter_glob='filter_glob', timeout=300)
def test_exec_timeout(self):
timeout = 3453
base-commit: 316346243be6df12799c0b64b788e06bad97c30b
--
2.33.0.464.g1972c5931b-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] kunit: tool: make --raw_output only support showing kunit output
2021-09-16 22:39 [PATCH] kunit: tool: make --raw_output only support showing kunit output Daniel Latypov
@ 2021-09-21 16:19 ` David Gow
0 siblings, 0 replies; 2+ messages in thread
From: David Gow @ 2021-09-21 16:19 UTC (permalink / raw)
To: Daniel Latypov
Cc: Brendan Higgins, Linux Kernel Mailing List, KUnit Development,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Fri, Sep 17, 2021 at 6:39 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Commit 6a499c9c42d0 ("kunit: tool: make --raw_output support only
> showing kunit output") made --raw_output a string-typed argument.
> Passing --raw_output=kunit would make it only show KUnit-related output
> and not everything.
>
> However, converting it to a string-typed argument had side effects.
>
> These calls used to work:
> $ kunit.py run --raw_output
> $ kunit.py run --raw_output suite_filter
> $ kunit.py run suite_filter --raw_output
>
> But now the second is actually parsed as
> $ kunit.py run --raw_output=suite_filter
>
> So the order you add in --raw_output now matters and command lines that
> used to work might not anymore.
>
> Change --raw_output back to a boolean flag, but change its behavior to
> match that of the former --raw_output=kunit.
> The assumption is that this is what most people wanted to see anyways.
>
> To get the old behavior, users can simply do:
> $ kunit.py run >/dev/null; cat .kunit/test.log
> They don't have any easy way of getting the --raw_output=kunit behavior.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
>
> Meta: this is an alternative to
> https://lore.kernel.org/linux-kselftest/20210903161405.1861312-1-dlatypov@google.com/
>
> I'd slightly prefer that approach, but if we're fine with giving up the
> old --raw_output semantics entirely, this would be cleaner.
> I'd also assume that most people would prefer the new semantics, but I'm
> not sure of that.
>
> ---
Thanks. I'm happy with either approach, but this is the one I properly
understand. If you'd rather push the other one, I agree that it's
better from a user perspective, so I'm okay with that: it's just a bit
beyond my comfort zone Python-hacks wise.
If we do go with this one, and I need the whole output, just running
the UML 'linux' binary is another option, which I've used in the past.
That's a bit trickier for qemu though: maybe there's some benefit in
having a --dry-run option for kunit.py run which just prints the
command used to execute the kernel. That's obviously beyond the scope
of this, though.
Regardless, this is
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> Documentation/dev-tools/kunit/kunit-tool.rst | 7 -------
> tools/testing/kunit/kunit.py | 12 +++---------
> tools/testing/kunit/kunit_tool_test.py | 13 ++++++-------
> 3 files changed, 9 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/kunit-tool.rst b/Documentation/dev-tools/kunit/kunit-tool.rst
> index ae52e0f489f9..03404746f1f6 100644
> --- a/Documentation/dev-tools/kunit/kunit-tool.rst
> +++ b/Documentation/dev-tools/kunit/kunit-tool.rst
> @@ -114,13 +114,6 @@ results in TAP format, you can pass the ``--raw_output`` argument.
>
> ./tools/testing/kunit/kunit.py run --raw_output
>
> -The raw output from test runs may contain other, non-KUnit kernel log
> -lines. You can see just KUnit output with ``--raw_output=kunit``:
> -
> -.. code-block:: bash
> -
> - ./tools/testing/kunit/kunit.py run --raw_output=kunit
> -
> If you have KUnit results in their raw TAP format, you can parse them and print
> the human-readable summary with the ``parse`` command for kunit_tool. This
> accepts a filename for an argument, or will read from standard input.
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 5a931456e718..3626a56472b5 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -115,13 +115,7 @@ def parse_tests(request: KunitParseRequest) -> KunitResult:
> 'Tests not Parsed.')
>
> if request.raw_output:
> - output: Iterable[str] = request.input_data
> - if request.raw_output == 'all':
> - pass
> - elif request.raw_output == 'kunit':
> - output = kunit_parser.extract_tap_lines(output)
> - else:
> - print(f'Unknown --raw_output option "{request.raw_output}"', file=sys.stderr)
> + output = kunit_parser.extract_tap_lines(request.input_data)
> for line in output:
> print(line.rstrip())
>
> @@ -256,8 +250,8 @@ def add_exec_opts(parser) -> None:
>
> def add_parse_opts(parser) -> None:
> parser.add_argument('--raw_output', help='If set don\'t format output from kernel. '
> - 'If set to --raw_output=kunit, filters to just KUnit output.',
> - type=str, nargs='?', const='all', default=None)
> + 'It will only show output from KUnit.',
> + action='store_true')
> parser.add_argument('--json',
> nargs='?',
> help='Stores test results in a JSON, and either '
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 619c4554cbff..55ed3dac31ee 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -399,14 +399,13 @@ class KUnitMainTest(unittest.TestCase):
> self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
> self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
>
> - def test_run_raw_output_kunit(self):
> + def test_run_raw_output_does_not_take_positional_args(self):
> + # --raw_output might eventually support an argument, but we don't want it
> + # to consume any positional arguments, only ones after an '='.
> self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
> - kunit.main(['run', '--raw_output=kunit'], self.linux_source_mock)
> - self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
> - self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
> - for call in self.print_mock.call_args_list:
> - self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
> - self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
> + kunit.main(['run', '--raw_output', 'filter_glob'], self.linux_source_mock)
> + self.linux_source_mock.run_kernel.assert_called_once_with(
> + args=None, build_dir='.kunit', filter_glob='filter_glob', timeout=300)
>
> def test_exec_timeout(self):
> timeout = 3453
>
> base-commit: 316346243be6df12799c0b64b788e06bad97c30b
> --
> 2.33.0.464.g1972c5931b-goog
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-21 16:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 22:39 [PATCH] kunit: tool: make --raw_output only support showing kunit output Daniel Latypov
2021-09-21 16:19 ` David Gow
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).