Skip to content

Commit

Permalink
kunit: tool: use dataclass instead of collections.namedtuple
Browse files Browse the repository at this point in the history 
namedtuple is a terse way of defining a collection of fields.
However, it does not allow us to annotate the type of these fields.
It also doesn't let us have any sort of inheritance between types.

Since commit df4b080 ("kunit: tool: Assert the version
requirement"), kunit.py has asserted that it's running on python >=3.7.

So in that case use a 3.7 feature, dataclasses, to replace these.

Changes in detail:
* Make KunitExecRequest contain all the fields needed for exec_tests
* Use inheritance to dedupe fields
  * also allows us to e.g. pass a KUnitRequest in as a KUnitParseRequest
  * this has changed around the order of some fields
* Use named arguments when constructing all request objects in kunit.py
  * This is to prevent accidentally mixing up fields, etc.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
  • Loading branch information
Daniel Latypov authored and Shuah Khan committed Dec 15, 2021
1 parent 7fa7ffc commit db16798
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 70 deletions.
139 changes: 72 additions & 67 deletions tools/testing/kunit/kunit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,57 @@

assert sys.version_info >= (3, 7), "Python version is too old"

from collections import namedtuple
from dataclasses import dataclass
from enum import Enum, auto
from typing import Iterable, Sequence, List
from typing import Any, Iterable, Sequence, List, Optional

import kunit_json
import kunit_kernel
import kunit_parser

KunitResult = namedtuple('KunitResult', ['status','result','elapsed_time'])

KunitConfigRequest = namedtuple('KunitConfigRequest',
['build_dir', 'make_options'])
KunitBuildRequest = namedtuple('KunitBuildRequest',
['jobs', 'build_dir', 'alltests',
'make_options'])
KunitExecRequest = namedtuple('KunitExecRequest',
['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', 'run_isolated', 'json', 'make_options'])

KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0]

class KunitStatus(Enum):
SUCCESS = auto()
CONFIG_FAILURE = auto()
BUILD_FAILURE = auto()
TEST_FAILURE = auto()

@dataclass
class KunitResult:
status: KunitStatus
result: Any
elapsed_time: float

@dataclass
class KunitConfigRequest:
build_dir: str
make_options: Optional[List[str]]

@dataclass
class KunitBuildRequest(KunitConfigRequest):
jobs: int
alltests: bool

@dataclass
class KunitParseRequest:
raw_output: Optional[str]
build_dir: str
json: Optional[str]

@dataclass
class KunitExecRequest(KunitParseRequest):
timeout: int
alltests: bool
filter_glob: str
kernel_args: Optional[List[str]]
run_isolated: Optional[str]

@dataclass
class KunitRequest(KunitExecRequest, KunitBuildRequest):
pass


KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0]

def get_kernel_root_path() -> str:
path = sys.argv[0] if not __file__ else __file__
parts = os.path.realpath(path).split('tools/testing/kunit')
Expand Down Expand Up @@ -121,8 +140,7 @@ def _suites_from_test_list(tests: List[str]) -> List[str]:



def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest,
parse_request: KunitParseRequest) -> KunitResult:
def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> KunitResult:
filter_globs = [request.filter_glob]
if request.run_isolated:
tests = _list_tests(linux, request)
Expand All @@ -147,7 +165,7 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest,
filter_glob=filter_glob,
build_dir=request.build_dir)

result = parse_tests(parse_request, run_result)
result = parse_tests(request, run_result)
# run_kernel() doesn't block on the kernel exiting.
# That only happens after we get the last line of output from `run_result`.
# So exec_time here actually contains parsing + execution time, which is fine.
Expand Down Expand Up @@ -217,27 +235,15 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
request: KunitRequest) -> KunitResult:
run_start = time.time()

config_request = KunitConfigRequest(request.build_dir,
request.make_options)
config_result = config_tests(linux, config_request)
config_result = config_tests(linux, request)
if config_result.status != KunitStatus.SUCCESS:
return config_result

build_request = KunitBuildRequest(request.jobs, request.build_dir,
request.alltests,
request.make_options)
build_result = build_tests(linux, build_request)
build_result = build_tests(linux, request)
if build_result.status != KunitStatus.SUCCESS:
return build_result

exec_request = KunitExecRequest(request.timeout, request.build_dir,
request.alltests, request.filter_glob,
request.kernel_args, request.run_isolated)
parse_request = KunitParseRequest(request.raw_output,
request.build_dir,
request.json)

exec_result = exec_tests(linux, exec_request, parse_request)
exec_result = exec_tests(linux, request)

run_end = time.time()

Expand Down Expand Up @@ -413,16 +419,16 @@ def main(argv, linux=None):
cross_compile=cli_args.cross_compile,
qemu_config_path=cli_args.qemu_config)

request = KunitRequest(cli_args.raw_output,
cli_args.timeout,
cli_args.jobs,
cli_args.build_dir,
cli_args.alltests,
cli_args.filter_glob,
cli_args.kernel_args,
cli_args.run_isolated,
cli_args.json,
cli_args.make_options)
request = KunitRequest(build_dir=cli_args.build_dir,
make_options=cli_args.make_options,
jobs=cli_args.jobs,
alltests=cli_args.alltests,
raw_output=cli_args.raw_output,
json=cli_args.json,
timeout=cli_args.timeout,
filter_glob=cli_args.filter_glob,
kernel_args=cli_args.kernel_args,
run_isolated=cli_args.run_isolated)
result = run_tests(linux, request)
if result.status != KunitStatus.SUCCESS:
sys.exit(1)
Expand All @@ -439,8 +445,8 @@ def main(argv, linux=None):
cross_compile=cli_args.cross_compile,
qemu_config_path=cli_args.qemu_config)

request = KunitConfigRequest(cli_args.build_dir,
cli_args.make_options)
request = KunitConfigRequest(build_dir=cli_args.build_dir,
make_options=cli_args.make_options)
result = config_tests(linux, request)
kunit_parser.print_with_timestamp((
'Elapsed time: %.3fs\n') % (
Expand All @@ -456,10 +462,10 @@ def main(argv, linux=None):
cross_compile=cli_args.cross_compile,
qemu_config_path=cli_args.qemu_config)

request = KunitBuildRequest(cli_args.jobs,
cli_args.build_dir,
cli_args.alltests,
cli_args.make_options)
request = KunitBuildRequest(build_dir=cli_args.build_dir,
make_options=cli_args.make_options,
jobs=cli_args.jobs,
alltests=cli_args.alltests)
result = build_tests(linux, request)
kunit_parser.print_with_timestamp((
'Elapsed time: %.3fs\n') % (
Expand All @@ -475,16 +481,15 @@ def main(argv, linux=None):
cross_compile=cli_args.cross_compile,
qemu_config_path=cli_args.qemu_config)

exec_request = KunitExecRequest(cli_args.timeout,
cli_args.build_dir,
cli_args.alltests,
cli_args.filter_glob,
cli_args.kernel_args,
cli_args.run_isolated)
parse_request = KunitParseRequest(cli_args.raw_output,
cli_args.build_dir,
cli_args.json)
result = exec_tests(linux, exec_request, parse_request)
exec_request = KunitExecRequest(raw_output=cli_args.raw_output,
build_dir=cli_args.build_dir,
json=cli_args.json,
timeout=cli_args.timeout,
alltests=cli_args.alltests,
filter_glob=cli_args.filter_glob,
kernel_args=cli_args.kernel_args,
run_isolated=cli_args.run_isolated)
result = exec_tests(linux, exec_request)
kunit_parser.print_with_timestamp((
'Elapsed time: %.3fs\n') % (result.elapsed_time))
if result.status != KunitStatus.SUCCESS:
Expand All @@ -496,9 +501,9 @@ def main(argv, linux=None):
else:
with open(cli_args.file, 'r', errors='backslashreplace') as f:
kunit_output = f.read().splitlines()
request = KunitParseRequest(cli_args.raw_output,
None,
cli_args.json)
request = KunitParseRequest(raw_output=cli_args.raw_output,
build_dir='',
json=cli_args.json)
result = parse_tests(request, kunit_output)
if result.status != KunitStatus.SUCCESS:
sys.exit(1)
Expand Down
6 changes: 3 additions & 3 deletions tools/testing/kunit/kunit_tool_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ def test_list_tests(self):
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(300, '.kunit', False, 'suite*', None, 'suite'))
kunit.KunitExecRequest(None, '.kunit', None, 300, False, 'suite*', None, 'suite'))

self.assertEqual(got, want)
# Should respect the user's filter glob when listing tests.
Expand All @@ -706,7 +706,7 @@ def test_run_isolated_by_suite(self, mock_tests):

# 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'))
kunit.KunitExecRequest(None, '.kunit', None, 300, 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),
Expand All @@ -719,7 +719,7 @@ def test_run_isolated_by_test(self, mock_tests):

# 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'))
kunit.KunitExecRequest(None, '.kunit', None, 300, 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),
Expand Down

0 comments on commit db16798

Please sign in to comment.