kunit: tool: surface and address more typing issues
The authors of this tool were more familiar with a different type-checker, https://github.com/google/pytype. That's open source, but mypy seems more prevalent (and runs faster). And unlike pytype, mypy doesn't try to infer types so it doesn't check unanotated functions. So annotate ~all functions in kunit tool to increase type-checking coverage. Note: per https://www.python.org/dev/peps/pep-0484/, `__init__()` should be annotated as `-> None`. Doing so makes mypy discover a number of new violations. Exclude main() since we reuse `request` for the different types of requests, which mypy isn't happy about. This commit fixes all but one error, where `TestSuite.status` might be None. Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: David Gow <davidgow@google.com> Tested-by: Brendan Higgins <brendanhiggins@google.com> Acked-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
This commit is contained in:
parent
8db50be262
commit
09641f7c7d
@ -43,9 +43,9 @@ class KunitStatus(Enum):
|
||||
BUILD_FAILURE = auto()
|
||||
TEST_FAILURE = auto()
|
||||
|
||||
def get_kernel_root_path():
|
||||
parts = sys.argv[0] if not __file__ else __file__
|
||||
parts = os.path.realpath(parts).split('tools/testing/kunit')
|
||||
def get_kernel_root_path() -> str:
|
||||
path = sys.argv[0] if not __file__ else __file__
|
||||
parts = os.path.realpath(path).split('tools/testing/kunit')
|
||||
if len(parts) != 2:
|
||||
sys.exit(1)
|
||||
return parts[0]
|
||||
@ -171,7 +171,7 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
|
||||
exec_result.elapsed_time))
|
||||
return parse_result
|
||||
|
||||
def add_common_opts(parser):
|
||||
def add_common_opts(parser) -> None:
|
||||
parser.add_argument('--build_dir',
|
||||
help='As in the make command, it specifies the build '
|
||||
'directory.',
|
||||
@ -183,13 +183,13 @@ def add_common_opts(parser):
|
||||
help='Run all KUnit tests through allyesconfig',
|
||||
action='store_true')
|
||||
|
||||
def add_build_opts(parser):
|
||||
def add_build_opts(parser) -> None:
|
||||
parser.add_argument('--jobs',
|
||||
help='As in the make command, "Specifies the number of '
|
||||
'jobs (commands) to run simultaneously."',
|
||||
type=int, default=8, metavar='jobs')
|
||||
|
||||
def add_exec_opts(parser):
|
||||
def add_exec_opts(parser) -> None:
|
||||
parser.add_argument('--timeout',
|
||||
help='maximum number of seconds to allow for all tests '
|
||||
'to run. This does not include time taken to build the '
|
||||
@ -198,7 +198,7 @@ def add_exec_opts(parser):
|
||||
default=300,
|
||||
metavar='timeout')
|
||||
|
||||
def add_parse_opts(parser):
|
||||
def add_parse_opts(parser) -> None:
|
||||
parser.add_argument('--raw_output', help='don\'t format output from kernel',
|
||||
action='store_true')
|
||||
parser.add_argument('--json',
|
||||
|
@ -8,6 +8,7 @@
|
||||
|
||||
import collections
|
||||
import re
|
||||
from typing import List, Set
|
||||
|
||||
CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$'
|
||||
CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$'
|
||||
@ -30,10 +31,10 @@ class KconfigParseError(Exception):
|
||||
class Kconfig(object):
|
||||
"""Represents defconfig or .config specified using the Kconfig language."""
|
||||
|
||||
def __init__(self):
|
||||
self._entries = []
|
||||
def __init__(self) -> None:
|
||||
self._entries = [] # type: List[KconfigEntry]
|
||||
|
||||
def entries(self):
|
||||
def entries(self) -> Set[KconfigEntry]:
|
||||
return set(self._entries)
|
||||
|
||||
def add_entry(self, entry: KconfigEntry) -> None:
|
||||
|
@ -13,7 +13,7 @@ import kunit_parser
|
||||
|
||||
from kunit_parser import TestStatus
|
||||
|
||||
def get_json_result(test_result, def_config, build_dir, json_path):
|
||||
def get_json_result(test_result, def_config, build_dir, json_path) -> str:
|
||||
sub_groups = []
|
||||
|
||||
# Each test suite is mapped to a KernelCI sub_group
|
||||
|
@ -11,6 +11,7 @@ import subprocess
|
||||
import os
|
||||
import shutil
|
||||
import signal
|
||||
from typing import Iterator
|
||||
|
||||
from contextlib import ExitStack
|
||||
|
||||
@ -39,7 +40,7 @@ class BuildError(Exception):
|
||||
class LinuxSourceTreeOperations(object):
|
||||
"""An abstraction over command line operations performed on a source tree."""
|
||||
|
||||
def make_mrproper(self):
|
||||
def make_mrproper(self) -> None:
|
||||
try:
|
||||
subprocess.check_output(['make', 'mrproper'], stderr=subprocess.STDOUT)
|
||||
except OSError as e:
|
||||
@ -47,7 +48,7 @@ class LinuxSourceTreeOperations(object):
|
||||
except subprocess.CalledProcessError as e:
|
||||
raise ConfigError(e.output.decode())
|
||||
|
||||
def make_olddefconfig(self, build_dir, make_options):
|
||||
def make_olddefconfig(self, build_dir, make_options) -> None:
|
||||
command = ['make', 'ARCH=um', 'olddefconfig']
|
||||
if make_options:
|
||||
command.extend(make_options)
|
||||
@ -60,7 +61,7 @@ class LinuxSourceTreeOperations(object):
|
||||
except subprocess.CalledProcessError as e:
|
||||
raise ConfigError(e.output.decode())
|
||||
|
||||
def make_allyesconfig(self, build_dir, make_options):
|
||||
def make_allyesconfig(self, build_dir, make_options) -> None:
|
||||
kunit_parser.print_with_timestamp(
|
||||
'Enabling all CONFIGs for UML...')
|
||||
command = ['make', 'ARCH=um', 'allyesconfig']
|
||||
@ -82,7 +83,7 @@ class LinuxSourceTreeOperations(object):
|
||||
kunit_parser.print_with_timestamp(
|
||||
'Starting Kernel with all configs takes a few minutes...')
|
||||
|
||||
def make(self, jobs, build_dir, make_options):
|
||||
def make(self, jobs, build_dir, make_options) -> None:
|
||||
command = ['make', 'ARCH=um', '--jobs=' + str(jobs)]
|
||||
if make_options:
|
||||
command.extend(make_options)
|
||||
@ -100,7 +101,7 @@ class LinuxSourceTreeOperations(object):
|
||||
if stderr: # likely only due to build warnings
|
||||
print(stderr.decode())
|
||||
|
||||
def linux_bin(self, params, timeout, build_dir):
|
||||
def linux_bin(self, params, timeout, build_dir) -> None:
|
||||
"""Runs the Linux UML binary. Must be named 'linux'."""
|
||||
linux_bin = get_file_path(build_dir, 'linux')
|
||||
outfile = get_outfile_path(build_dir)
|
||||
@ -110,23 +111,23 @@ class LinuxSourceTreeOperations(object):
|
||||
stderr=subprocess.STDOUT)
|
||||
process.wait(timeout)
|
||||
|
||||
def get_kconfig_path(build_dir):
|
||||
def get_kconfig_path(build_dir) -> str:
|
||||
return get_file_path(build_dir, KCONFIG_PATH)
|
||||
|
||||
def get_kunitconfig_path(build_dir):
|
||||
def get_kunitconfig_path(build_dir) -> str:
|
||||
return get_file_path(build_dir, KUNITCONFIG_PATH)
|
||||
|
||||
def get_outfile_path(build_dir):
|
||||
def get_outfile_path(build_dir) -> str:
|
||||
return get_file_path(build_dir, OUTFILE_PATH)
|
||||
|
||||
class LinuxSourceTree(object):
|
||||
"""Represents a Linux kernel source tree with KUnit tests."""
|
||||
|
||||
def __init__(self):
|
||||
def __init__(self) -> None:
|
||||
self._ops = LinuxSourceTreeOperations()
|
||||
signal.signal(signal.SIGINT, self.signal_handler)
|
||||
|
||||
def clean(self):
|
||||
def clean(self) -> bool:
|
||||
try:
|
||||
self._ops.make_mrproper()
|
||||
except ConfigError as e:
|
||||
@ -134,17 +135,17 @@ class LinuxSourceTree(object):
|
||||
return False
|
||||
return True
|
||||
|
||||
def create_kunitconfig(self, build_dir, defconfig=DEFAULT_KUNITCONFIG_PATH):
|
||||
def create_kunitconfig(self, build_dir, defconfig=DEFAULT_KUNITCONFIG_PATH) -> None:
|
||||
kunitconfig_path = get_kunitconfig_path(build_dir)
|
||||
if not os.path.exists(kunitconfig_path):
|
||||
shutil.copyfile(defconfig, kunitconfig_path)
|
||||
|
||||
def read_kunitconfig(self, build_dir):
|
||||
def read_kunitconfig(self, build_dir) -> None:
|
||||
kunitconfig_path = get_kunitconfig_path(build_dir)
|
||||
self._kconfig = kunit_config.Kconfig()
|
||||
self._kconfig.read_from_file(kunitconfig_path)
|
||||
|
||||
def validate_config(self, build_dir):
|
||||
def validate_config(self, build_dir) -> bool:
|
||||
kconfig_path = get_kconfig_path(build_dir)
|
||||
validated_kconfig = kunit_config.Kconfig()
|
||||
validated_kconfig.read_from_file(kconfig_path)
|
||||
@ -158,7 +159,7 @@ class LinuxSourceTree(object):
|
||||
return False
|
||||
return True
|
||||
|
||||
def build_config(self, build_dir, make_options):
|
||||
def build_config(self, build_dir, make_options) -> bool:
|
||||
kconfig_path = get_kconfig_path(build_dir)
|
||||
if build_dir and not os.path.exists(build_dir):
|
||||
os.mkdir(build_dir)
|
||||
@ -170,7 +171,7 @@ class LinuxSourceTree(object):
|
||||
return False
|
||||
return self.validate_config(build_dir)
|
||||
|
||||
def build_reconfig(self, build_dir, make_options):
|
||||
def build_reconfig(self, build_dir, make_options) -> bool:
|
||||
"""Creates a new .config if it is not a subset of the .kunitconfig."""
|
||||
kconfig_path = get_kconfig_path(build_dir)
|
||||
if os.path.exists(kconfig_path):
|
||||
@ -186,7 +187,7 @@ class LinuxSourceTree(object):
|
||||
print('Generating .config ...')
|
||||
return self.build_config(build_dir, make_options)
|
||||
|
||||
def build_um_kernel(self, alltests, jobs, build_dir, make_options):
|
||||
def build_um_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
|
||||
try:
|
||||
if alltests:
|
||||
self._ops.make_allyesconfig(build_dir, make_options)
|
||||
@ -197,7 +198,7 @@ class LinuxSourceTree(object):
|
||||
return False
|
||||
return self.validate_config(build_dir)
|
||||
|
||||
def run_kernel(self, args=[], build_dir='', timeout=None):
|
||||
def run_kernel(self, args=[], build_dir='', timeout=None) -> Iterator[str]:
|
||||
args.extend(['mem=1G', 'console=tty'])
|
||||
self._ops.linux_bin(args, timeout, build_dir)
|
||||
outfile = get_outfile_path(build_dir)
|
||||
@ -206,6 +207,6 @@ class LinuxSourceTree(object):
|
||||
for line in file:
|
||||
yield line
|
||||
|
||||
def signal_handler(self, sig, frame):
|
||||
def signal_handler(self, sig, frame) -> None:
|
||||
logging.error('Build interruption occurred. Cleaning console.')
|
||||
subprocess.call(['stty', 'sane'])
|
||||
|
@ -12,32 +12,32 @@ from collections import namedtuple
|
||||
from datetime import datetime
|
||||
from enum import Enum, auto
|
||||
from functools import reduce
|
||||
from typing import List, Optional, Tuple
|
||||
from typing import Iterator, List, Optional, Tuple
|
||||
|
||||
TestResult = namedtuple('TestResult', ['status','suites','log'])
|
||||
|
||||
class TestSuite(object):
|
||||
def __init__(self):
|
||||
self.status = None
|
||||
self.name = None
|
||||
self.cases = []
|
||||
def __init__(self) -> None:
|
||||
self.status = None # type: Optional[TestStatus]
|
||||
self.name = ''
|
||||
self.cases = [] # type: List[TestCase]
|
||||
|
||||
def __str__(self):
|
||||
return 'TestSuite(' + self.status + ',' + self.name + ',' + str(self.cases) + ')'
|
||||
def __str__(self) -> str:
|
||||
return 'TestSuite(' + str(self.status) + ',' + self.name + ',' + str(self.cases) + ')'
|
||||
|
||||
def __repr__(self):
|
||||
def __repr__(self) -> str:
|
||||
return str(self)
|
||||
|
||||
class TestCase(object):
|
||||
def __init__(self):
|
||||
self.status = None
|
||||
def __init__(self) -> None:
|
||||
self.status = None # type: Optional[TestStatus]
|
||||
self.name = ''
|
||||
self.log = []
|
||||
self.log = [] # type: List[str]
|
||||
|
||||
def __str__(self):
|
||||
return 'TestCase(' + self.status + ',' + self.name + ',' + str(self.log) + ')'
|
||||
def __str__(self) -> str:
|
||||
return 'TestCase(' + str(self.status) + ',' + self.name + ',' + str(self.log) + ')'
|
||||
|
||||
def __repr__(self):
|
||||
def __repr__(self) -> str:
|
||||
return str(self)
|
||||
|
||||
class TestStatus(Enum):
|
||||
@ -51,7 +51,7 @@ kunit_start_re = re.compile(r'TAP version [0-9]+$')
|
||||
kunit_end_re = re.compile('(List of all partitions:|'
|
||||
'Kernel panic - not syncing: VFS:)')
|
||||
|
||||
def isolate_kunit_output(kernel_output):
|
||||
def isolate_kunit_output(kernel_output) -> Iterator[str]:
|
||||
started = False
|
||||
for line in kernel_output:
|
||||
line = line.rstrip() # line always has a trailing \n
|
||||
@ -64,7 +64,7 @@ def isolate_kunit_output(kernel_output):
|
||||
elif started:
|
||||
yield line[prefix_len:] if prefix_len > 0 else line
|
||||
|
||||
def raw_output(kernel_output):
|
||||
def raw_output(kernel_output) -> None:
|
||||
for line in kernel_output:
|
||||
print(line.rstrip())
|
||||
|
||||
@ -72,26 +72,26 @@ DIVIDER = '=' * 60
|
||||
|
||||
RESET = '\033[0;0m'
|
||||
|
||||
def red(text):
|
||||
def red(text) -> str:
|
||||
return '\033[1;31m' + text + RESET
|
||||
|
||||
def yellow(text):
|
||||
def yellow(text) -> str:
|
||||
return '\033[1;33m' + text + RESET
|
||||
|
||||
def green(text):
|
||||
def green(text) -> str:
|
||||
return '\033[1;32m' + text + RESET
|
||||
|
||||
def print_with_timestamp(message):
|
||||
def print_with_timestamp(message) -> None:
|
||||
print('[%s] %s' % (datetime.now().strftime('%H:%M:%S'), message))
|
||||
|
||||
def format_suite_divider(message):
|
||||
def format_suite_divider(message) -> str:
|
||||
return '======== ' + message + ' ========'
|
||||
|
||||
def print_suite_divider(message):
|
||||
def print_suite_divider(message) -> None:
|
||||
print_with_timestamp(DIVIDER)
|
||||
print_with_timestamp(format_suite_divider(message))
|
||||
|
||||
def print_log(log):
|
||||
def print_log(log) -> None:
|
||||
for m in log:
|
||||
print_with_timestamp(m)
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user