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@go...
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
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@go...
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
linux-kselftest-mirror@lists.linaro.org