Re: [RFC v2 5/9] kunit: tool: Add command line interface to filter and report attributes

[David Gow]

On Sat, 8 Jul 2023 at 05:10, Rae Moar <rmoar@xxxxxxxxxx> wrote:
>
> Add ability to kunit.py to filter attributes and report a list of tests
> including attributes without running tests.
>
> Add flag "--filter" to input filters on test attributes. Tests will be
> filtered out if they do not match all inputted filters.
>
> Example: --filter speed=slow (This filter would run only the tests that are
> marked as slow)
>
> Filters have operations: <, >, <=, >=, !=, and =. But note that the
> characters < and > are often interpreted by the shell, so they may need to
> be quoted or escaped.
>
> Example: --filter "speed>slow" or --filter speed\>slow (This filter would
> run only the tests that have the speed faster than slow.
>
> Additionally, multiple filters can be used.
>
> Example: --filter "speed=slow, module!=example" (This filter would run
> only the tests that have the speed slow and are not in the "example"
> module)
>
> Note if the user wants to skip filtered tests instead of not
> running/showing them use the "--filter_skip" flag instead.
>
> Expose the output of kunit.action=list option with flag "--list_tests" to
> output a list of tests. Additionally, add flag "--list_tests_attr" to
> output a list of tests and their attributes. These flags are useful to see
> tests and test attributes without needing to run tests.
>
> Example of the output of "--list_tests_attr":
>   example
>   example.test_1
>   example.test_2
>   # example.test_2.speed: slow
>
> This output includes a suite, example, with two test cases, test_1 and
> test_2. And in this instance test_2 has been marked as slow.
>
> Signed-off-by: Rae Moar <rmoar@xxxxxxxxxx>
> ---
>
> Changes since v1:
> - Change method for inputting filters to allow for spaces in filtering
>   values
> - Add option to skip filtered tests instead of not run or show them with
>   the --filter_skip flag
> - Separate the new feature to list tests and their attributes into both
>   --list_tests (lists just tests) and --list_tests_attr (lists all)
>
>  tools/testing/kunit/kunit.py           | 80 ++++++++++++++++++++++++--
>  tools/testing/kunit/kunit_kernel.py    |  6 +-
>  tools/testing/kunit/kunit_tool_test.py | 39 ++++++-------
>  3 files changed, 96 insertions(+), 29 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 3905c43369c3..6104e622ce20 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -55,8 +55,12 @@ class KunitExecRequest(KunitParseRequest):
>         build_dir: str
>         timeout: int
>         filter_glob: str
> +       filter: str
> +       filter_skip: str
>         kernel_args: Optional[List[str]]
>         run_isolated: Optional[str]
> +       list_tests: bool
> +       list_tests_attr: bool
>
>  @dataclass
>  class KunitRequest(KunitExecRequest, KunitBuildRequest):
> @@ -102,19 +106,39 @@ def config_and_build_tests(linux: kunit_kernel.LinuxSourceTree,
>
>  def _list_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> List[str]:
>         args = ['kunit.action=list']
> +
> +       if request.kernel_args:
> +               args.extend(request.kernel_args)
> +
> +       output = linux.run_kernel(args=args,
> +                          timeout=request.timeout,
> +                          filter_glob=request.filter_glob,
> +                          filter=request.filter,
> +                          build_dir=request.build_dir)
> +       lines = kunit_parser.extract_tap_lines(output)
> +       # Hack! Drop the dummy TAP version header that the executor prints out.
> +       lines.pop()
> +
> +       # Filter out any extraneous non-test output that might have gotten mixed in.
> +       return [l for l in output if re.match(r'^[^\s.]+\.[^\s.]+$', l)]
> +
> +def _list_tests_attr(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> Iterable[str]:
> +       args = ['kunit.action=list_attr']
> +
>         if request.kernel_args:
>                 args.extend(request.kernel_args)
>
>         output = linux.run_kernel(args=args,
>                            timeout=request.timeout,
>                            filter_glob=request.filter_glob,
> +                          filter=request.filter,
>                            build_dir=request.build_dir)
>         lines = kunit_parser.extract_tap_lines(output)
>         # Hack! Drop the dummy TAP version header that the executor prints out.
>         lines.pop()
>
>         # Filter out any extraneous non-test output that might have gotten mixed in.
> -       return [l for l in lines if re.match(r'^[^\s.]+\.[^\s.]+$', l)]
> +       return lines
>
>  def _suites_from_test_list(tests: List[str]) -> List[str]:
>         """Extracts all the suites from an ordered list of tests."""
> @@ -128,10 +152,18 @@ def _suites_from_test_list(tests: List[str]) -> List[str]:
>                         suites.append(suite)
>         return suites
>
> -
> -
>  def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> KunitResult:
>         filter_globs = [request.filter_glob]
> +       if request.list_tests:
> +               output = _list_tests(linux, request)
> +               for line in output:
> +                       print(line.rstrip())
> +               return KunitResult(status=KunitStatus.SUCCESS, elapsed_time=0.0)
> +       if request.list_tests_attr:
> +               attr_output = _list_tests_attr(linux, request)
> +               for line in attr_output:
> +                       print(line.rstrip())
> +               return KunitResult(status=KunitStatus.SUCCESS, elapsed_time=0.0)
>         if request.run_isolated:
>                 tests = _list_tests(linux, request)
>                 if request.run_isolated == 'test':
> @@ -145,6 +177,17 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -
>
>         metadata = kunit_json.Metadata(arch=linux.arch(), build_dir=request.build_dir, def_config='kunit_defconfig')
>
> +       filter = request.filter
> +       if request.filter_skip:
> +               args = ['kunit.filter_action=skip']
> +               filter = request.filter_skip
> +               if request.kernel_args:
> +                       args.extend(request.kernel_args)

What happens if both filter and filter_skip are set? We should
probably either make those mutually exclusive (error if both are set),
or expose filter_action directly instead.

> +       elif request.kernel_args:
> +               args = request.kernel_args
> +       else:
> +               args = None
> +
>         test_counts = kunit_parser.TestCounts()
>         exec_time = 0.0
>         for i, filter_glob in enumerate(filter_globs):
> @@ -152,9 +195,10 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -
>
>                 test_start = time.time()
>                 run_result = linux.run_kernel(
> -                       args=request.kernel_args,
> +                       args=args,
>                         timeout=request.timeout,
>                         filter_glob=filter_glob,
> +                       filter=filter,
>                         build_dir=request.build_dir)
>
>                 _, test_result = parse_tests(request, metadata, run_result)
> @@ -341,6 +385,18 @@ def add_exec_opts(parser: argparse.ArgumentParser) -> None:
>                             nargs='?',
>                             default='',
>                             metavar='filter_glob')
> +       parser.add_argument('--filter',
> +                           help='Filter KUnit tests with attributes, '
> +                               'filtered tests will not run, '
> +                           'e.g. speed=fast or speed=>low',

Neither fast or low are valid values for speed now.

> +                           type=str,
> +                               default='')
> +       parser.add_argument('--filter_skip',
> +                           help='Filter KUnit tests run with attributes, '
> +                               'filtered tests will be skipped, '
> +                           'e.g. speed=fast or speed=>low',

Neither fast or low are valid values for speed now.



> +                           type=str,
> +                               default='')
>         parser.add_argument('--kernel_args',
>                             help='Kernel command-line parameters. Maybe be repeated',
>                              action='append', metavar='')
> @@ -350,6 +406,10 @@ def add_exec_opts(parser: argparse.ArgumentParser) -> None:
>                             'what ran before it.',
>                             type=str,
>                             choices=['suite', 'test'])
> +       parser.add_argument('--list_tests', help='If set, list all tests',
> +                           action='store_true')
> +       parser.add_argument('--list_tests_attr', help='If set, list all tests and attributes.',
> +                           action='store_true')
>
>  def add_parse_opts(parser: argparse.ArgumentParser) -> None:
>         parser.add_argument('--raw_output', help='If set don\'t parse output from kernel. '
> @@ -398,8 +458,12 @@ def run_handler(cli_args: argparse.Namespace) -> None:
>                                         json=cli_args.json,
>                                         timeout=cli_args.timeout,
>                                         filter_glob=cli_args.filter_glob,
> +                                       filter=cli_args.filter,
> +                                       filter_skip=cli_args.filter_skip,
>                                         kernel_args=cli_args.kernel_args,
> -                                       run_isolated=cli_args.run_isolated)
> +                                       run_isolated=cli_args.run_isolated,
> +                                       list_tests=cli_args.list_tests,
> +                                       list_tests_attr=cli_args.list_tests_attr)
>         result = run_tests(linux, request)
>         if result.status != KunitStatus.SUCCESS:
>                 sys.exit(1)
> @@ -441,8 +505,12 @@ def exec_handler(cli_args: argparse.Namespace) -> None:
>                                         json=cli_args.json,
>                                         timeout=cli_args.timeout,
>                                         filter_glob=cli_args.filter_glob,
> +                                       filter=cli_args.filter,
> +                                       filter_skip=cli_args.filter_skip,
>                                         kernel_args=cli_args.kernel_args,
> -                                       run_isolated=cli_args.run_isolated)
> +                                       run_isolated=cli_args.run_isolated,
> +                                       list_tests=cli_args.list_tests,
> +                                       list_tests_attr=cli_args.list_tests_attr)
>         result = exec_tests(linux, exec_request)
>         stdout.print_with_timestamp((
>                 'Elapsed time: %.3fs\n') % (result.elapsed_time))
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 7f648802caf6..281f062a4767 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -330,11 +330,13 @@ class LinuxSourceTree:
>                         return False
>                 return self.validate_config(build_dir)
>
> -       def run_kernel(self, args: Optional[List[str]]=None, build_dir: str='', filter_glob: str='', timeout: Optional[int]=None) -> Iterator[str]:
> +       def run_kernel(self, args: Optional[List[str]]=None, build_dir: str='', filter_glob: str='', filter: str='', timeout: Optional[int]=None) -> Iterator[str]:
>                 if not args:
>                         args = []
>                 if filter_glob:
> -                       args.append('kunit.filter_glob='+filter_glob)
> +                       args.append('kunit.filter_glob=' + filter_glob)
> +               if filter:
> +                       args.append('kunit.filter="' + filter + '"')
>                 args.append('kunit.enable=1')
>
>                 process = self._ops.start(args, build_dir)
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index be35999bb84f..85a1fb72735e 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -597,7 +597,7 @@ class KUnitMainTest(unittest.TestCase):
>                 self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
>                 self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir='.kunit', filter_glob='', timeout=300)
> +                       args=None, build_dir='.kunit', filter_glob='', filter='', timeout=300)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_run_passes_args_pass(self):
> @@ -605,7 +605,7 @@ class KUnitMainTest(unittest.TestCase):
>                 self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
>                 self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir='.kunit', filter_glob='', timeout=300)
> +                       args=None, build_dir='.kunit', filter_glob='', filter='', timeout=300)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_exec_passes_args_fail(self):
> @@ -629,7 +629,7 @@ class KUnitMainTest(unittest.TestCase):
>                         kunit.main(['run'])
>                 self.assertEqual(e.exception.code, 1)
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir='.kunit', filter_glob='', timeout=300)
> +                       args=None, build_dir='.kunit', filter_glob='', filter='', timeout=300)
>                 self.print_mock.assert_any_call(StrContains(' 0 tests run!'))
>
>         def test_exec_raw_output(self):
> @@ -670,13 +670,13 @@ class KUnitMainTest(unittest.TestCase):
>                 self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
>                 kunit.main(['run', '--raw_output', 'filter_glob'])
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir='.kunit', filter_glob='filter_glob', timeout=300)
> +                       args=None, build_dir='.kunit', filter_glob='filter_glob', filter='', timeout=300)
>
>         def test_exec_timeout(self):
>                 timeout = 3453
>                 kunit.main(['exec', '--timeout', str(timeout)])
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir='.kunit', filter_glob='', timeout=timeout)
> +                       args=None, build_dir='.kunit', filter_glob='', filter='', timeout=timeout)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_run_timeout(self):
> @@ -684,7 +684,7 @@ class KUnitMainTest(unittest.TestCase):
>                 kunit.main(['run', '--timeout', str(timeout)])
>                 self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir='.kunit', filter_glob='', timeout=timeout)
> +                       args=None, build_dir='.kunit', filter_glob='', filter='', timeout=timeout)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_run_builddir(self):
> @@ -692,7 +692,7 @@ class KUnitMainTest(unittest.TestCase):
>                 kunit.main(['run', '--build_dir=.kunit'])
>                 self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir=build_dir, filter_glob='', timeout=300)
> +                       args=None, build_dir=build_dir, filter_glob='', filter='', timeout=300)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_config_builddir(self):
> @@ -710,7 +710,7 @@ class KUnitMainTest(unittest.TestCase):
>                 build_dir = '.kunit'
>                 kunit.main(['exec', '--build_dir', build_dir])
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=None, build_dir=build_dir, filter_glob='', timeout=300)
> +                       args=None, build_dir=build_dir, filter_glob='', filter='', timeout=300)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_run_kunitconfig(self):
> @@ -786,7 +786,7 @@ class KUnitMainTest(unittest.TestCase):
>                 kunit.main(['run', '--kernel_args=a=1', '--kernel_args=b=2'])
>                 self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                     args=['a=1','b=2'], build_dir='.kunit', filter_glob='', timeout=300)
> +                     args=['a=1','b=2'], build_dir='.kunit', filter_glob='', filter='', timeout=300)
>                 self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
>         def test_list_tests(self):
> @@ -794,13 +794,11 @@ class KUnitMainTest(unittest.TestCase):
>                 self.linux_source_mock.run_kernel.return_value = ['TAP version 14', 'init: random output'] + want
>
>                 got = kunit._list_tests(self.linux_source_mock,
> -                                    kunit.KunitExecRequest(None, None, '.kunit', 300, 'suite*', None, 'suite'))
> -
> +                                    kunit.KunitExecRequest(None, None, '.kunit', 300, 'suite*', '', '', None, 'suite', False, False))
>                 self.assertEqual(got, want)
>                 # Should respect the user's filter glob when listing tests.
>                 self.linux_source_mock.run_kernel.assert_called_once_with(
> -                       args=['kunit.action=list'], build_dir='.kunit', filter_glob='suite*', timeout=300)
> -
> +                       args=['kunit.action=list'], build_dir='.kunit', filter_glob='suite*', filter='', timeout=300)
>
>         @mock.patch.object(kunit, '_list_tests')
>         def test_run_isolated_by_suite(self, mock_tests):
> @@ -809,10 +807,10 @@ class KUnitMainTest(unittest.TestCase):
>
>                 # Should respect the user's filter glob when listing tests.
>                 mock_tests.assert_called_once_with(mock.ANY,
> -                                    kunit.KunitExecRequest(None, None, '.kunit', 300, 'suite*.test*', None, 'suite'))
> +                                    kunit.KunitExecRequest(None, None, '.kunit', 300, 'suite*.test*', '', '', None, 'suite', False, False))
>                 self.linux_source_mock.run_kernel.assert_has_calls([
> -                       mock.call(args=None, build_dir='.kunit', filter_glob='suite.test*', timeout=300),
> -                       mock.call(args=None, build_dir='.kunit', filter_glob='suite2.test*', timeout=300),
> +                       mock.call(args=None, build_dir='.kunit', filter_glob='suite.test*', filter='', timeout=300),
> +                       mock.call(args=None, build_dir='.kunit', filter_glob='suite2.test*', filter='', timeout=300),
>                 ])
>
>         @mock.patch.object(kunit, '_list_tests')
> @@ -822,13 +820,12 @@ class KUnitMainTest(unittest.TestCase):
>
>                 # Should respect the user's filter glob when listing tests.
>                 mock_tests.assert_called_once_with(mock.ANY,
> -                                    kunit.KunitExecRequest(None, None, '.kunit', 300, 'suite*', None, 'test'))
> +                                    kunit.KunitExecRequest(None, None, '.kunit', 300, 'suite*', '', '', None, 'test', False, False))
>                 self.linux_source_mock.run_kernel.assert_has_calls([
> -                       mock.call(args=None, build_dir='.kunit', filter_glob='suite.test1', timeout=300),
> -                       mock.call(args=None, build_dir='.kunit', filter_glob='suite.test2', timeout=300),
> -                       mock.call(args=None, build_dir='.kunit', filter_glob='suite2.test1', timeout=300),
> +                       mock.call(args=None, build_dir='.kunit', filter_glob='suite.test1', filter='', timeout=300),
> +                       mock.call(args=None, build_dir='.kunit', filter_glob='suite.test2', filter='', timeout=300),
> +                       mock.call(args=None, build_dir='.kunit', filter_glob='suite2.test1', filter='', timeout=300),
>                 ])
>
> -
>  if __name__ == '__main__':
>         unittest.main()
> --
> 2.41.0.255.g8b1d071c50-goog
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature