* [PATCH 2/3] kunit: tool: factor exec + parse steps into a function
2021-09-09 0:10 [PATCH 0/3] kunit: allow running test suites/cases individually Daniel Latypov
2021-09-09 0:10 ` [PATCH 1/3] kunit: add 'kunit.action' param to allow listing out tests Daniel Latypov
@ 2021-09-09 0:10 ` Daniel Latypov
2021-09-09 0:10 ` [PATCH 3/3] kunit: tool: support running each suite/test separately Daniel Latypov
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Latypov @ 2021-09-09 0:10 UTC (permalink / raw)
To: brendanhiggins, davidgow
Cc: linux-kernel, kunit-dev, linux-kselftest, skhan, Daniel Latypov
Currently this code is copy-pasted between the normal "run" subcommand
and the "exec" subcommand.
Given we don't have any interest in just executing the tests without
giving the user any indication what happened (i.e. parsing the output),
make a function that does both this things and can be reused.
This will be useful when we allow more complicated ways of running
tests, e.g. invoking the kernel multiple times instead of just once,
etc.
We remove input_data from the ParseRequest so the callers don't have to
pass in a dummy value for this field. Named tuples are also immutable,
so if they did pass in a dummy, exec_tests() would need to make a copy
to call parse_tests().
Removing it also makes KunitParseRequest match the other *Request types,
as they only contain user arguments/flags, not data.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit.py | 44 ++++++++++++++++--------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 5a931456e718..55c25f7a715c 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -34,7 +34,7 @@ KunitExecRequest = namedtuple('KunitExecRequest',
['timeout', 'build_dir', 'alltests',
'filter_glob', 'kernel_args'])
KunitParseRequest = namedtuple('KunitParseRequest',
- ['raw_output', 'input_data', 'build_dir', 'json'])
+ ['raw_output', 'build_dir', 'json'])
KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs',
'build_dir', 'alltests', 'filter_glob',
'kernel_args', 'json', 'make_options'])
@@ -91,23 +91,25 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree,
'built kernel successfully',
build_end - build_start)
-def exec_tests(linux: kunit_kernel.LinuxSourceTree,
- request: KunitExecRequest) -> KunitResult:
+def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest,
+ parse_request: KunitParseRequest) -> KunitResult:
kunit_parser.print_with_timestamp('Starting KUnit Kernel ...')
test_start = time.time()
- result = linux.run_kernel(
+ run_result = linux.run_kernel(
args=request.kernel_args,
timeout=None if request.alltests else request.timeout,
- filter_glob=request.filter_glob,
+ filter_glob=request.filter_glob,
build_dir=request.build_dir)
test_end = time.time()
+ exec_time = test_end - test_start
- return KunitResult(KunitStatus.SUCCESS,
- result,
- test_end - test_start)
+ # Named tuples are immutable, so we rebuild them here manually
+ result = parse_tests(parse_request, run_result)
+
+ return KunitResult(status=result.status, result=result.result, elapsed_time=exec_time)
-def parse_tests(request: KunitParseRequest) -> KunitResult:
+def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> KunitResult:
parse_start = time.time()
test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS,
@@ -115,7 +117,7 @@ def parse_tests(request: KunitParseRequest) -> KunitResult:
'Tests not Parsed.')
if request.raw_output:
- output: Iterable[str] = request.input_data
+ output: Iterable[str] = input_data
if request.raw_output == 'all':
pass
elif request.raw_output == 'kunit':
@@ -126,7 +128,7 @@ def parse_tests(request: KunitParseRequest) -> KunitResult:
print(line.rstrip())
else:
- test_result = kunit_parser.parse_run_tests(request.input_data)
+ test_result = kunit_parser.parse_run_tests(input_data)
parse_end = time.time()
if request.json:
@@ -165,15 +167,11 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
exec_request = KunitExecRequest(request.timeout, request.build_dir,
request.alltests, request.filter_glob,
request.kernel_args)
- exec_result = exec_tests(linux, exec_request)
- if exec_result.status != KunitStatus.SUCCESS:
- return exec_result
-
parse_request = KunitParseRequest(request.raw_output,
- exec_result.result,
request.build_dir,
request.json)
- parse_result = parse_tests(parse_request)
+
+ exec_result = exec_tests(linux, exec_request, parse_request)
run_end = time.time()
@@ -184,7 +182,7 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
config_result.elapsed_time,
build_result.elapsed_time,
exec_result.elapsed_time))
- return parse_result
+ return exec_result
def add_common_opts(parser) -> None:
parser.add_argument('--build_dir',
@@ -382,15 +380,12 @@ def main(argv, linux=None):
cli_args.alltests,
cli_args.filter_glob,
cli_args.kernel_args)
- exec_result = exec_tests(linux, exec_request)
parse_request = KunitParseRequest(cli_args.raw_output,
- exec_result.result,
cli_args.build_dir,
cli_args.json)
- result = parse_tests(parse_request)
+ result = exec_tests(linux, exec_request, parse_request)
kunit_parser.print_with_timestamp((
- 'Elapsed time: %.3fs\n') % (
- exec_result.elapsed_time))
+ 'Elapsed time: %.3fs\n') % (result.elapsed_time))
if result.status != KunitStatus.SUCCESS:
sys.exit(1)
elif cli_args.subcommand == 'parse':
@@ -400,10 +395,9 @@ def main(argv, linux=None):
with open(cli_args.file, 'r') as f:
kunit_output = f.read().splitlines()
request = KunitParseRequest(cli_args.raw_output,
- kunit_output,
None,
cli_args.json)
- result = parse_tests(request)
+ result = parse_tests(request, kunit_output)
if result.status != KunitStatus.SUCCESS:
sys.exit(1)
else:
--
2.33.0.309.g3052b89438-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/3] kunit: tool: support running each suite/test separately
2021-09-09 0:10 [PATCH 0/3] kunit: allow running test suites/cases individually Daniel Latypov
2021-09-09 0:10 ` [PATCH 1/3] kunit: add 'kunit.action' param to allow listing out tests Daniel Latypov
2021-09-09 0:10 ` [PATCH 2/3] kunit: tool: factor exec + parse steps into a function Daniel Latypov
@ 2021-09-09 0:10 ` Daniel Latypov
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Latypov @ 2021-09-09 0:10 UTC (permalink / raw)
To: brendanhiggins, davidgow
Cc: linux-kernel, kunit-dev, linux-kselftest, skhan, Daniel Latypov
The new --run_isolated flag makes the tool boot the kernel once per
suite or test, preventing leftover state from one suite to impact the
other. This can be useful as a starting point to debugging test
hermeticity issues.
Note: it takes a lot longer, so people should not use it normally.
Consider the following very simplified example:
bool disable_something_for_test = false;
void function_being_tested() {
...
if (disable_something_for_test) return;
...
}
static void test_before(struct kunit *test)
{
disable_something_for_test = true;
function_being_tested();
/* oops, we forgot to reset it back to false */
}
static void test_after(struct kunit *test)
{
/* oops, now "fixing" test_before can cause test_after to fail! */
function_being_tested();
}
Presented like this, the issues are obvious, but it gets a lot more
complicated to track down as the amount of test setup and helper
functions increases.
Another use case is memory corruption. It might not be surface as an
failure/crash in the test case or suite that caused it. I've noticed in
kunit's own unit tests, the 3rd suite after might be the one to finally
crash after an out-of-bounds write, for example.
Example usage:
Per suite:
$ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit --run_isolated=suite
...
Starting KUnit Kernel (1/7)...
============================================================
======== [PASSED] kunit_executor_test ========
....
Testing complete. 5 tests run. 0 failed. 0 crashed. 0 skipped.
Starting KUnit Kernel (2/7)...
============================================================
======== [PASSED] kunit-try-catch-test ========
...
Per test:
$ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit --run_isolated=test
Starting KUnit Kernel (1/23)...
============================================================
======== [PASSED] kunit_executor_test ========
[PASSED] parse_filter_test
============================================================
Testing complete. 1 tests run. 0 failed. 0 crashed. 0 skipped.
Starting KUnit Kernel (2/23)...
============================================================
======== [PASSED] kunit_executor_test ========
[PASSED] filter_subsuite_test
...
It works with filters as well:
$ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit --run_isolated=suite example
...
Starting KUnit Kernel (1/1)...
============================================================
======== [PASSED] example ========
...
It also handles test filters, '*.*skip*' runs these 3 tests:
kunit_status.kunit_status_mark_skipped_test
example.example_skip_test
example.example_mark_skipped_test
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit.py | 91 ++++++++++++++++++++------
tools/testing/kunit/kunit_tool_test.py | 40 +++++++++++
2 files changed, 112 insertions(+), 19 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 55c25f7a715c..060655d60533 100755
--- 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, List
import kunit_config
import kunit_json
@@ -31,13 +31,13 @@ KunitBuildRequest = namedtuple('KunitBuildRequest',
['jobs', 'build_dir', 'alltests',
'make_options'])
KunitExecRequest = namedtuple('KunitExecRequest',
- ['timeout', 'build_dir', 'alltests',
- 'filter_glob', 'kernel_args'])
+ ['timeout', 'build_dir', 'alltests',
+ 'filter_glob', 'kernel_args', 'run_isolated'])
KunitParseRequest = namedtuple('KunitParseRequest',
['raw_output', 'build_dir', 'json'])
KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs',
'build_dir', 'alltests', 'filter_glob',
- 'kernel_args', 'json', 'make_options'])
+ 'kernel_args', 'run_isolated', 'json', 'make_options'])
KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0]
@@ -91,21 +91,66 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree,
'built kernel successfully',
build_end - build_start)
-def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest,
- parse_request: KunitParseRequest) -> KunitResult:
- kunit_parser.print_with_timestamp('Starting KUnit Kernel ...')
- test_start = time.time()
- run_result = linux.run_kernel(
- args=request.kernel_args,
- timeout=None if request.alltests else request.timeout,
- filter_glob=request.filter_glob,
- build_dir=request.build_dir)
+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=None if request.alltests else request.timeout,
+ filter_glob=request.filter_glob,
+ build_dir=request.build_dir)
+ output = kunit_parser.extract_tap_lines(output)
+ # Hack! Drop the TAP version header and top-level test plan.
+ output.pop()
+ output.pop()
+ return list(output)
+
+def _suites_from_test_list(tests: List[str]) -> List[str]:
+ """Extracts all the suites from an ordered list of tests."""
+ suites = [] # type: List[str]
+ for t in tests:
+ parts = t.split('.', maxsplit=2)
+ if len(parts) != 2:
+ raise ValueError(f'internal KUnit error, test name should be of the form "<suite>.<test>", got "{t}"')
+ suite, case = parts
+ if not suites or suites[-1] != suite:
+ suites.append(suite)
+ return suites
+
- test_end = time.time()
- exec_time = test_end - test_start
- # Named tuples are immutable, so we rebuild them here manually
- result = parse_tests(parse_request, run_result)
+def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest,
+ parse_request: KunitParseRequest) -> KunitResult:
+ filter_globs = [request.filter_glob]
+ if request.run_isolated:
+ tests = _list_tests(linux, request)
+ if request.run_isolated == 'test':
+ filter_globs = tests
+ if request.run_isolated == 'suite':
+ filter_globs = _suites_from_test_list(tests)
+ # Apply the test-part of the user's glob, if present.
+ if '.' in request.filter_glob:
+ test_glob = request.filter_glob.split('.', maxsplit=2)[1]
+ filter_globs = [g + '.'+ test_glob for g in filter_globs]
+
+ overall_status = kunit_parser.TestStatus.SUCCESS
+ exec_time = 0
+ for i, filter_glob in enumerate(filter_globs):
+ kunit_parser.print_with_timestamp('Starting KUnit Kernel ({}/{})...'.format(i+1, len(filter_globs)))
+
+ test_start = time.time()
+ run_result = linux.run_kernel(
+ args=request.kernel_args,
+ timeout=None if request.alltests else request.timeout,
+ filter_glob=filter_glob,
+ build_dir=request.build_dir)
+
+ test_end = time.time()
+ exec_time += test_end - test_start
+
+ result = parse_tests(parse_request, run_result)
+ overall_status = kunit_parser.max_status(overall_status, result.status)
return KunitResult(status=result.status, result=result.result, elapsed_time=exec_time)
@@ -166,7 +211,7 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
exec_request = KunitExecRequest(request.timeout, request.build_dir,
request.alltests, request.filter_glob,
- request.kernel_args)
+ request.kernel_args, request.run_isolated)
parse_request = KunitParseRequest(request.raw_output,
request.build_dir,
request.json)
@@ -251,6 +296,12 @@ def add_exec_opts(parser) -> None:
parser.add_argument('--kernel_args',
help='Kernel command-line parameters. Maybe be repeated',
action='append')
+ parser.add_argument('--run_isolated', help='If set, boot the kernel for each '
+ 'individual suite/test. This is can be useful for debugging '
+ 'a non-hermetic test, one that might pass/fail based on '
+ 'what ran before it.',
+ type=str,
+ choices=['suite', 'test']),
def add_parse_opts(parser) -> None:
parser.add_argument('--raw_output', help='If set don\'t format output from kernel. '
@@ -324,6 +375,7 @@ def main(argv, linux=None):
cli_args.alltests,
cli_args.filter_glob,
cli_args.kernel_args,
+ cli_args.run_isolated,
cli_args.json,
cli_args.make_options)
result = run_tests(linux, request)
@@ -379,7 +431,8 @@ def main(argv, linux=None):
cli_args.build_dir,
cli_args.alltests,
cli_args.filter_glob,
- cli_args.kernel_args)
+ cli_args.kernel_args,
+ cli_args.run_isolated)
parse_request = KunitParseRequest(cli_args.raw_output,
cli_args.build_dir,
cli_args.json)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 619c4554cbff..bcfdc6664c9d 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -477,6 +477,46 @@ class KUnitMainTest(unittest.TestCase):
args=['a=1','b=2'], build_dir='.kunit', filter_glob='', timeout=300)
self.print_mock.assert_any_call(StrContains('Testing complete.'))
+ def test_list_tests(self):
+ want = ['suite.test1', 'suite.test2', 'suite2.test1']
+ self.linux_source_mock.run_kernel.return_value = ['TAP version 14', '1..0'] + want
+
+ got = kunit._list_tests(self.linux_source_mock,
+ kunit.KunitExecRequest(300, '.kunit', False, 'suite*', None, 'suite'))
+
+ 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)
+
+
+ @mock.patch.object(kunit, '_list_tests')
+ def test_run_isolated_by_suite(self, mock_tests):
+ mock_tests.return_value = ['suite.test1', 'suite.test2', 'suite2.test1']
+ kunit.main(['exec', '--run_isolated=suite', 'suite*.test*'], self.linux_source_mock)
+
+ # Should respect the user's filter glob when listing tests.
+ mock_tests.assert_called_once_with(mock.ANY,
+ kunit.KunitExecRequest(300, '.kunit', False, 'suite*.test*', None, 'suite'))
+ 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.patch.object(kunit, '_list_tests')
+ def test_run_isolated_by_test(self, mock_tests):
+ mock_tests.return_value = ['suite.test1', 'suite.test2', 'suite2.test1']
+ kunit.main(['exec', '--run_isolated=test', 'suite*'], self.linux_source_mock)
+
+ # Should respect the user's filter glob when listing tests.
+ mock_tests.assert_called_once_with(mock.ANY,
+ kunit.KunitExecRequest(300, '.kunit', False, 'suite*', None, 'test'))
+ 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),
+ ])
+
if __name__ == '__main__':
unittest.main()
--
2.33.0.309.g3052b89438-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread