* [PATCH v3 1/3] kunit: tool: surface and address more typing issues
@ 2021-01-07 23:48 Daniel Latypov
2021-01-07 23:48 ` [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status Daniel Latypov
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Daniel Latypov @ 2021-01-07 23:48 UTC (permalink / raw)
To: davidgow, brendanhiggins
Cc: linux-kernel, linux-kselftest, skhan, Daniel Latypov
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>
---
tools/testing/kunit/kunit.py | 14 ++++-----
tools/testing/kunit/kunit_config.py | 7 +++--
tools/testing/kunit/kunit_json.py | 2 +-
tools/testing/kunit/kunit_kernel.py | 37 ++++++++++++-----------
tools/testing/kunit/kunit_parser.py | 46 ++++++++++++++---------------
5 files changed, 54 insertions(+), 52 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 21516e293d17..5521e0a8201e 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -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',
diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
index 02ffc3a3e5dc..bdd60230764b 100644
--- a/tools/testing/kunit/kunit_config.py
+++ b/tools/testing/kunit/kunit_config.py
@@ -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:
diff --git a/tools/testing/kunit/kunit_json.py b/tools/testing/kunit/kunit_json.py
index 624b31b2dbd6..f5cca5c38cac 100644
--- a/tools/testing/kunit/kunit_json.py
+++ b/tools/testing/kunit/kunit_json.py
@@ -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
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 57c1724b7e5d..db7ed84ea410 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -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'])
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'])
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 6614ec4d0898..8b5eb9507765 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -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)
base-commit: f5e6c330254ae691f6d7befe61c786eb5056007e
--
2.29.2.729.g45daf8777d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status
2021-01-07 23:48 [PATCH v3 1/3] kunit: tool: surface and address more typing issues Daniel Latypov
@ 2021-01-07 23:48 ` Daniel Latypov
2021-01-11 21:19 ` Brendan Higgins
2021-01-07 23:48 ` [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional Daniel Latypov
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Daniel Latypov @ 2021-01-07 23:48 UTC (permalink / raw)
To: davidgow, brendanhiggins
Cc: linux-kernel, linux-kselftest, skhan, Daniel Latypov
The code to handle aggregating statuses didn't check that the status
actually got set to some non-None value.
Default the value to SUCCESS instead of adding a bunch of `is None`
checks.
This sorta follows the precedent in commit 3fc48259d525 ("kunit: Don't
fail test suites if one of them is empty").
Also slightly simplify the code and add type annotations.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
---
tools/testing/kunit/kunit_parser.py | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 8b5eb9507765..2e4cb5206fa7 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -12,13 +12,13 @@ from collections import namedtuple
from datetime import datetime
from enum import Enum, auto
from functools import reduce
-from typing import Iterator, List, Optional, Tuple
+from typing import Iterable, Iterator, List, Optional, Tuple
TestResult = namedtuple('TestResult', ['status','suites','log'])
class TestSuite(object):
def __init__(self) -> None:
- self.status = None # type: Optional[TestStatus]
+ self.status = TestStatus.SUCCESS
self.name = ''
self.cases = [] # type: List[TestCase]
@@ -30,7 +30,7 @@ class TestSuite(object):
class TestCase(object):
def __init__(self) -> None:
- self.status = None # type: Optional[TestStatus]
+ self.status = TestStatus.SUCCESS
self.name = ''
self.log = [] # type: List[str]
@@ -224,12 +224,11 @@ def parse_ok_not_ok_test_suite(lines: List[str],
else:
return False
-def bubble_up_errors(to_status, status_container_list) -> TestStatus:
- status_list = map(to_status, status_container_list)
- return reduce(max_status, status_list, TestStatus.SUCCESS)
+def bubble_up_errors(statuses: Iterable[TestStatus]) -> TestStatus:
+ return reduce(max_status, statuses, TestStatus.SUCCESS)
def bubble_up_test_case_errors(test_suite: TestSuite) -> TestStatus:
- max_test_case_status = bubble_up_errors(lambda x: x.status, test_suite.cases)
+ max_test_case_status = bubble_up_errors(x.status for x in test_suite.cases)
return max_status(max_test_case_status, test_suite.status)
def parse_test_suite(lines: List[str], expected_suite_index: int) -> Optional[TestSuite]:
@@ -282,8 +281,8 @@ def parse_test_plan(lines: List[str]) -> Optional[int]:
else:
return None
-def bubble_up_suite_errors(test_suite_list: List[TestSuite]) -> TestStatus:
- return bubble_up_errors(lambda x: x.status, test_suite_list)
+def bubble_up_suite_errors(test_suites: Iterable[TestSuite]) -> TestStatus:
+ return bubble_up_errors(x.status for x in test_suites)
def parse_test_result(lines: List[str]) -> TestResult:
consume_non_diagnositic(lines)
--
2.29.2.729.g45daf8777d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional
2021-01-07 23:48 [PATCH v3 1/3] kunit: tool: surface and address more typing issues Daniel Latypov
2021-01-07 23:48 ` [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status Daniel Latypov
@ 2021-01-07 23:48 ` Daniel Latypov
2021-01-11 21:24 ` Brendan Higgins
2021-01-11 21:15 ` [PATCH v3 1/3] kunit: tool: surface and address more typing issues Brendan Higgins
2021-01-15 0:32 ` Brendan Higgins
3 siblings, 1 reply; 7+ messages in thread
From: Daniel Latypov @ 2021-01-07 23:48 UTC (permalink / raw)
To: davidgow, brendanhiggins
Cc: linux-kernel, linux-kselftest, skhan, Daniel Latypov
LinuxSourceTree will unceremoniously crash if the user doesn't call
read_kunitconfig() first in a number of functions.
And currently every place we create an instance, the caller also calls
create_kunitconfig() and read_kunitconfig().
Move these instead into __init__() so they can't be forgotten and to
reduce copy-paste.
The https://github.com/google/pytype type-checker complained that
_config wasn't initialized. With this, kunit_tool now type checks
under both pytype and mypy.
Add an optional boolean that can be used to disable this for use cases
in the future where we might not need/want to load the config.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit.py | 20 ++++----------------
tools/testing/kunit/kunit_kernel.py | 25 +++++++++++++------------
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 5521e0a8201e..e808a47c839b 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -256,10 +256,7 @@ def main(argv, linux=None):
os.mkdir(cli_args.build_dir)
if not linux:
- linux = kunit_kernel.LinuxSourceTree()
-
- linux.create_kunitconfig(cli_args.build_dir)
- linux.read_kunitconfig(cli_args.build_dir)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
request = KunitRequest(cli_args.raw_output,
cli_args.timeout,
@@ -277,10 +274,7 @@ def main(argv, linux=None):
os.mkdir(cli_args.build_dir)
if not linux:
- linux = kunit_kernel.LinuxSourceTree()
-
- linux.create_kunitconfig(cli_args.build_dir)
- linux.read_kunitconfig(cli_args.build_dir)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
request = KunitConfigRequest(cli_args.build_dir,
cli_args.make_options)
@@ -292,10 +286,7 @@ def main(argv, linux=None):
sys.exit(1)
elif cli_args.subcommand == 'build':
if not linux:
- linux = kunit_kernel.LinuxSourceTree()
-
- linux.create_kunitconfig(cli_args.build_dir)
- linux.read_kunitconfig(cli_args.build_dir)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
request = KunitBuildRequest(cli_args.jobs,
cli_args.build_dir,
@@ -309,10 +300,7 @@ def main(argv, linux=None):
sys.exit(1)
elif cli_args.subcommand == 'exec':
if not linux:
- linux = kunit_kernel.LinuxSourceTree()
-
- linux.create_kunitconfig(cli_args.build_dir)
- linux.read_kunitconfig(cli_args.build_dir)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
exec_request = KunitExecRequest(cli_args.timeout,
cli_args.build_dir,
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index db7ed84ea410..bf7a784ac6eb 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -123,28 +123,29 @@ def get_outfile_path(build_dir) -> str:
class LinuxSourceTree(object):
"""Represents a Linux kernel source tree with KUnit tests."""
- def __init__(self) -> None:
- self._ops = LinuxSourceTreeOperations()
+ def __init__(self, build_dir: str, load_config=True, defconfig=DEFAULT_KUNITCONFIG_PATH) -> None:
signal.signal(signal.SIGINT, self.signal_handler)
- def clean(self) -> bool:
- try:
- self._ops.make_mrproper()
- except ConfigError as e:
- logging.error(e)
- return False
- return True
+ self._ops = LinuxSourceTreeOperations()
+
+ if not load_config:
+ return
- 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) -> None:
- kunitconfig_path = get_kunitconfig_path(build_dir)
self._kconfig = kunit_config.Kconfig()
self._kconfig.read_from_file(kunitconfig_path)
+ def clean(self) -> bool:
+ try:
+ self._ops.make_mrproper()
+ except ConfigError as e:
+ logging.error(e)
+ return False
+ return True
+
def validate_config(self, build_dir) -> bool:
kconfig_path = get_kconfig_path(build_dir)
validated_kconfig = kunit_config.Kconfig()
--
2.29.2.729.g45daf8777d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] kunit: tool: surface and address more typing issues
2021-01-07 23:48 [PATCH v3 1/3] kunit: tool: surface and address more typing issues Daniel Latypov
2021-01-07 23:48 ` [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status Daniel Latypov
2021-01-07 23:48 ` [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional Daniel Latypov
@ 2021-01-11 21:15 ` Brendan Higgins
2021-01-15 0:32 ` Brendan Higgins
3 siblings, 0 replies; 7+ messages in thread
From: Brendan Higgins @ 2021-01-11 21:15 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> 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>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status
2021-01-07 23:48 ` [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status Daniel Latypov
@ 2021-01-11 21:19 ` Brendan Higgins
0 siblings, 0 replies; 7+ messages in thread
From: Brendan Higgins @ 2021-01-11 21:19 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> The code to handle aggregating statuses didn't check that the status
> actually got set to some non-None value.
> Default the value to SUCCESS instead of adding a bunch of `is None`
> checks.
>
> This sorta follows the precedent in commit 3fc48259d525 ("kunit: Don't
> fail test suites if one of them is empty").
>
> Also slightly simplify the code and add type annotations.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional
2021-01-07 23:48 ` [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional Daniel Latypov
@ 2021-01-11 21:24 ` Brendan Higgins
0 siblings, 0 replies; 7+ messages in thread
From: Brendan Higgins @ 2021-01-11 21:24 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> LinuxSourceTree will unceremoniously crash if the user doesn't call
> read_kunitconfig() first in a number of functions.
>
> And currently every place we create an instance, the caller also calls
> create_kunitconfig() and read_kunitconfig().
> Move these instead into __init__() so they can't be forgotten and to
> reduce copy-paste.
>
> The https://github.com/google/pytype type-checker complained that
> _config wasn't initialized. With this, kunit_tool now type checks
> under both pytype and mypy.
>
> Add an optional boolean that can be used to disable this for use cases
> in the future where we might not need/want to load the config.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
Very nice! This makes the code much more readable!
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] kunit: tool: surface and address more typing issues
2021-01-07 23:48 [PATCH v3 1/3] kunit: tool: surface and address more typing issues Daniel Latypov
` (2 preceding siblings ...)
2021-01-11 21:15 ` [PATCH v3 1/3] kunit: tool: surface and address more typing issues Brendan Higgins
@ 2021-01-15 0:32 ` Brendan Higgins
3 siblings, 0 replies; 7+ messages in thread
From: Brendan Higgins @ 2021-01-15 0:32 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> 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>
Hey, I just tried applying this series as a pre-check before sending
it off to Shuah and it no longer applies to torvalds/master.
Please fix and resend.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-15 0:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07 23:48 [PATCH v3 1/3] kunit: tool: surface and address more typing issues Daniel Latypov
2021-01-07 23:48 ` [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status Daniel Latypov
2021-01-11 21:19 ` Brendan Higgins
2021-01-07 23:48 ` [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional Daniel Latypov
2021-01-11 21:24 ` Brendan Higgins
2021-01-11 21:15 ` [PATCH v3 1/3] kunit: tool: surface and address more typing issues Brendan Higgins
2021-01-15 0:32 ` Brendan Higgins
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.