kunit: tool: better handling of quasi-bool args (--json, --raw_output)

Problem:

What does this do?
$ kunit.py run --json
Well, it runs all the tests and prints test results out as JSON.

And next is
$ kunit.py run my-test-suite --json
This runs just `my-test-suite` and prints results out as JSON.

But what about?
$ kunit.py run --json my-test-suite
This runs all the tests and stores the json results in a "my-test-suite"
file.

Why:
--json, and now --raw_output are actually string flags. They just have a
default value. --json in particular takes the name of an output file.

It was intended that you'd do
$ kunit.py run --json=my_output_file my-test-suite
if you ever wanted to specify the value.

Workaround:
It doesn't seem like there's a way to make
https://docs.python.org/3/library/argparse.html only accept arg values
after a '='.

I believe that `--json` should "just work" regardless of where it is.
So this patch automatically rewrites a bare `--json` to `--json=stdout`.

That makes the examples above work the same way.
Add a regression test that can catch this for --raw_output.

Fixes: 6a499c9c42d0 ("kunit: tool: make --raw_output support only showing kunit output")
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Tested-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 5a93145..ac35c61 100755 (executable)
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -16,7 +16,7 @@ assert sys.version_info >= (3, 7), "Python version is too old"
 
 from collections import namedtuple
 from enum import Enum, auto
-from typing import Iterable
+from typing import Iterable, Sequence
 
 import kunit_config
 import kunit_json
@@ -186,6 +186,26 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
                                exec_result.elapsed_time))
        return parse_result
 
+# Problem:
+# $ kunit.py run --json
+# works as one would expect and prints the parsed test results as JSON.
+# $ kunit.py run --json suite_name
+# would *not* pass suite_name as the filter_glob and print as json.
+# argparse will consider it to be another way of writing
+# $ kunit.py run --json=suite_name
+# i.e. it would run all tests, and dump the json to a `suite_name` file.
+# So we hackily automatically rewrite --json => --json=stdout
+pseudo_bool_flag_defaults = {
+               '--json': 'stdout',
+               '--raw_output': 'kunit',
+}
+def massage_argv(argv: Sequence[str]) -> Sequence[str]:
+       def massage_arg(arg: str) -> str:
+               if arg not in pseudo_bool_flag_defaults:
+                       return arg
+               return  f'{arg}={pseudo_bool_flag_defaults[arg]}'
+       return list(map(massage_arg, argv))
+
 def add_common_opts(parser) -> None:
        parser.add_argument('--build_dir',
                            help='As in the make command, it specifies the build '
@@ -303,7 +323,7 @@ def main(argv, linux=None):
                                  help='Specifies the file to read results from.',
                                  type=str, nargs='?', metavar='input_file')
 
-       cli_args = parser.parse_args(argv)
+       cli_args = parser.parse_args(massage_argv(argv))
 
        if get_kernel_root_path():
                os.chdir(get_kernel_root_path())

diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 619c455..1edcc83 100755 (executable)
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -408,6 +408,14 @@ 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_does_not_take_positional_args(self):
+               # --raw_output is a string flag, 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', '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
                kunit.main(['exec', '--timeout', str(timeout)], self.linux_source_mock)