From: Rae Moar <rmoar@google.com>
To: David Gow <davidgow@google.com>
Cc: shuah@kernel.org, dlatypov@google.com, brendan.higgins@linux.dev,
linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
linux-kernel@vger.kernel.org, keescook@chromium.org,
linux-hardening@vger.kernel.org, jstultz@google.com,
tglx@linutronix.de, sboyd@kernel.org
Subject: Re: [RFC v1 4/6] kunit: tool: Add command line interface to filter and report attributes
Date: Tue, 13 Jun 2023 16:44:25 -0400 [thread overview]
Message-ID: <CA+GJov5sxHOug0kYYGcazPKrgS1FH81QPWMLjveN8_YO-OhxNg@mail.gmail.com> (raw)
In-Reply-To: <CABVgOS=rC+EtinxAcqJK_ATCXwvtfffEFZsG-nx=ig35pavHdQ@mail.gmail.com>
On Sat, Jun 10, 2023 at 4:29 AM David Gow <davidgow@google.com> wrote:
>
> On Sat, 10 Jun 2023 at 08:52, Rae Moar <rmoar@google.com> wrote:
> >
> > Add ability to use kunit.py to filter attributes and to 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. Note there
> > cannot be spaces within a filter.
>
> Within a filter's name, value, or the entire filter string. Is this a
> restriction we can remove?
>
Currently this implementation does not allow for spaces anywhere in
the filter string so for the example: "--filter speed=slow", there
cannot be spaces within "speed=slow".
I would be interested in removing this restriction by allowing the
user to use quotes if there are spaces.
I originally thought this would be potentially a future change.
However, it may be best to implement earlier as it would cause the
implementation to change quite a bit. The module_param_array may need
to be changed to be a string that is then parsed.
> >
> > As said in the previous patch, filters can have different operations: <, >,
> > <=, >=, !=, and =. Note that the characters < and > are often interpreted
> > by the shell, so they may need to be quoted or escaped.
> >
> > Example: --filter "speed>=normal" or –filter speed\>=normal
> >
> > This filter would run only the tests that have the speed faster than or
> > equal to normal.
> >
> > Add flag "--list_tests" to output a list of tests and their attributes
> > without running tests. This will be useful to see test attributes and which
> > tests will run with given filters.
>
> Please note that this comes from the kernel's kunit.action=list option.
Got it. Will do in the next version.
> >
> > Example of the output of these tests:
> > 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.
> >
>
> It's unrelated, so perhaps best split out into its own patch, but I'd
> love the option to list tests without the attributes as well. That
> would allow doing things like piping the list of tests to wc -l to
> count them, etc.
>
I really like this idea of allowing two options: list tests only and
then also include attributes. I wonder if I should include the tests
in the second option. My instinct would be yes (to show all tests not
just those with attributes) but let me know what you think.
Thanks!
-Rae
>
> > Signed-off-by: Rae Moar <rmoar@google.com>
> > ---
> > tools/testing/kunit/kunit.py | 34 +++++++++++++++++----
> > tools/testing/kunit/kunit_kernel.py | 6 ++--
> > tools/testing/kunit/kunit_tool_test.py | 41 +++++++++++++-------------
> > 3 files changed, 54 insertions(+), 27 deletions(-)
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index 3905c43369c3..661c39f7acf5 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -55,8 +55,10 @@ class KunitExecRequest(KunitParseRequest):
> > build_dir: str
> > timeout: int
> > filter_glob: str
> > + filter: Optional[List[str]]
> > kernel_args: Optional[List[str]]
> > run_isolated: Optional[str]
> > + list_tests: Optional[bool]
> >
> > @dataclass
> > class KunitRequest(KunitExecRequest, KunitBuildRequest):
> > @@ -100,7 +102,7 @@ def config_and_build_tests(linux: kunit_kernel.LinuxSourceTree,
> >
> > return build_tests(linux, request)
> >
> > -def _list_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> List[str]:
> > +def _list_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> Iterable[str]:
> > args = ['kunit.action=list']
> > if request.kernel_args:
> > args.extend(request.kernel_args)
> > @@ -108,13 +110,17 @@ def _list_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest)
> > 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 output
> > +
> > +def _get_tests(output: Iterable[str]) -> List[str]:
> > + return [l for l in output if re.match(r'^[^\s.]+\.[^\s.]+$', l)]
> >
> > def _suites_from_test_list(tests: List[str]) -> List[str]:
> > """Extracts all the suites from an ordered list of tests."""
> > @@ -132,8 +138,14 @@ def _suites_from_test_list(tests: List[str]) -> List[str]:
> >
> > 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.run_isolated:
> > - tests = _list_tests(linux, request)
> > + output = _list_tests(linux, request)
> > + tests = _get_tests(output)
> > if request.run_isolated == 'test':
> > filter_globs = tests
> > elif request.run_isolated == 'suite':
> > @@ -155,6 +167,7 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -
> > args=request.kernel_args,
> > timeout=request.timeout,
> > filter_glob=filter_glob,
> > + filter=request.filter,
> > build_dir=request.build_dir)
> >
> > _, test_result = parse_tests(request, metadata, run_result)
> > @@ -341,6 +354,11 @@ def add_exec_opts(parser: argparse.ArgumentParser) -> None:
> > nargs='?',
> > default='',
> > metavar='filter_glob')
> > + parser.add_argument('--filter',
> > + help='Filter which KUnit tests run by attributes'
> > + 'e.g. speed=fast or speed=>low',
> > + type=str,
> > + nargs='*')
> > parser.add_argument('--kernel_args',
> > help='Kernel command-line parameters. Maybe be repeated',
> > action='append', metavar='')
> > @@ -350,6 +368,8 @@ 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 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 +418,10 @@ 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,
> > kernel_args=cli_args.kernel_args,
> > - run_isolated=cli_args.run_isolated)
> > + run_isolated=cli_args.run_isolated,
> > + list_tests=cli_args.list_tests)
> > result = run_tests(linux, request)
> > if result.status != KunitStatus.SUCCESS:
> > sys.exit(1)
> > @@ -441,8 +463,10 @@ 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,
> > kernel_args=cli_args.kernel_args,
> > - run_isolated=cli_args.run_isolated)
> > + run_isolated=cli_args.run_isolated,
> > + list_tests=cli_args.list_tests)
> > 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..62cb8200f60e 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: Optional[List[str]]=None, 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=' + (','.join(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..4a7f3112d06c 100755
> > --- a/tools/testing/kunit/kunit_tool_test.py
> > +++ b/tools/testing/kunit/kunit_tool_test.py
> > @@ -14,6 +14,7 @@ import tempfile, shutil # Handling test_tmpdir
> > import itertools
> > import json
> > import os
> > +import re
> > import signal
> > import subprocess
> > from typing import Iterable
> > @@ -597,7 +598,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=None, timeout=300)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_run_passes_args_pass(self):
> > @@ -605,7 +606,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=None, timeout=300)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_exec_passes_args_fail(self):
> > @@ -629,7 +630,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=None, timeout=300)
> > self.print_mock.assert_any_call(StrContains(' 0 tests run!'))
> >
> > def test_exec_raw_output(self):
> > @@ -670,13 +671,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=None, 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=None, timeout=timeout)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_run_timeout(self):
> > @@ -684,7 +685,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=None, timeout=timeout)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_run_builddir(self):
> > @@ -692,7 +693,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=None, timeout=300)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_config_builddir(self):
> > @@ -710,7 +711,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=None, timeout=300)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_run_kunitconfig(self):
> > @@ -786,7 +787,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=None, timeout=300)
> > self.print_mock.assert_any_call(StrContains('Testing complete.'))
> >
> > def test_list_tests(self):
> > @@ -794,12 +795,13 @@ 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, None, 'suite', False))
> > + tests = kunit._get_tests(got)
> >
> > - self.assertEqual(got, want)
> > + self.assertEqual(tests, 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=None, timeout=300)
> >
> >
> > @mock.patch.object(kunit, '_list_tests')
> > @@ -809,10 +811,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, None, 'suite', 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=None, timeout=300),
> > + mock.call(args=None, build_dir='.kunit', filter_glob='suite2.test*', filter=None, timeout=300),
> > ])
> >
> > @mock.patch.object(kunit, '_list_tests')
> > @@ -822,13 +824,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, None, 'test', 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=None, timeout=300),
> > + mock.call(args=None, build_dir='.kunit', filter_glob='suite.test2', filter=None, timeout=300),
> > + mock.call(args=None, build_dir='.kunit', filter_glob='suite2.test1', filter=None, timeout=300),
> > ])
> >
> > -
> > if __name__ == '__main__':
> > unittest.main()
> > --
> > 2.41.0.162.gfafddb0af9-goog
> >
next prev parent reply other threads:[~2023-06-13 20:45 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-10 0:51 [RFC v1 0/6] kunit: Add test attributes API Rae Moar
2023-06-10 0:51 ` [RFC v1 1/6] kunit: Add test attributes API structure Rae Moar
2023-06-10 8:29 ` David Gow
2023-06-13 20:36 ` Rae Moar
2023-06-10 0:51 ` [RFC v1 2/6] kunit: Add speed attribute Rae Moar
2023-06-10 3:13 ` kernel test robot
2023-06-10 8:29 ` David Gow
2023-06-13 20:37 ` Rae Moar
2023-06-10 0:51 ` [RFC v1 3/6] kunit: Add ability to filter attributes Rae Moar
2023-06-10 3:57 ` kernel test robot
2023-06-10 8:29 ` David Gow
2023-06-13 20:42 ` Rae Moar
2023-06-13 20:26 ` Kees Cook
2023-06-13 20:58 ` Rae Moar
2023-06-10 0:51 ` [RFC v1 4/6] kunit: tool: Add command line interface to filter and report attributes Rae Moar
2023-06-10 8:29 ` David Gow
2023-06-13 20:44 ` Rae Moar [this message]
2023-06-10 0:51 ` [RFC v1 5/6] kunit: memcpy: Mark tests as slow using test attributes Rae Moar
2023-06-10 8:29 ` David Gow
2023-06-13 20:44 ` Rae Moar
2023-06-10 0:51 ` [RFC v1 6/6] kunit: time: Mark test " Rae Moar
2023-06-10 8:29 ` [RFC v1 0/6] kunit: Add test attributes API David Gow
2023-06-13 20:34 ` Rae Moar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+GJov5sxHOug0kYYGcazPKrgS1FH81QPWMLjveN8_YO-OhxNg@mail.gmail.com \
--to=rmoar@google.com \
--cc=brendan.higgins@linux.dev \
--cc=davidgow@google.com \
--cc=dlatypov@google.com \
--cc=jstultz@google.com \
--cc=keescook@chromium.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=sboyd@kernel.org \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.