| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+GJov5sxHOug0kYYGcazPKrgS1FH81QPWMLjveN8_YO-OhxNg@mail.gmail.com>
Date: Tue, 13 Jun 2023 16:44:25 -0400
From: Rae Moar <rmoar@...gle.com>
To: David Gow <davidgow@...gle.com>
Cc: shuah@...nel.org, dlatypov@...gle.com, brendan.higgins@...ux.dev,
linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
linux-kernel@...r.kernel.org, keescook@...omium.org,
linux-hardening@...r.kernel.org, jstultz@...gle.com,
tglx@...utronix.de, sboyd@...nel.org
Subject: Re: [RFC v1 4/6] kunit: tool: Add command line interface to filter
and report attributes
On Sat, Jun 10, 2023 at 4:29 AM David Gow <davidgow@...gle.com> wrote:
>
> On Sat, 10 Jun 2023 at 08:52, Rae Moar <rmoar@...gle.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@...gle.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
> >
Powered by blists - more mailing lists