From 695e26030858b27648ca107b77095fed53377b4b Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Thu, 16 Mar 2023 15:06:36 -0700 Subject: kunit: tool: add subscripts for type annotations where appropriate E.g. for subprocess.Popen, it can be opened in `text=True` mode where it returns strings, or `text=False` where it returns bytes. To differentiate, you can annotate types as `Popen[str]` or `Popen[bytes]`. This patch should add subscripts in all the places we were missing them. Reported-by: Johannes Berg Link: https://lore.kernel.org/linux-kselftest/20230315105055.9b2be0153625.I7a2cb99b95dff216c0feed4604255275e0b156a7@changeid/ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_kernel.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'tools/testing/kunit/kunit_kernel.py') diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 53e90c335834..e6fc8fcb071a 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -92,7 +92,7 @@ class LinuxSourceTreeOperations: if stderr: # likely only due to build warnings print(stderr.decode()) - def start(self, params: List[str], build_dir: str) -> subprocess.Popen: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: raise RuntimeError('not implemented!') @@ -112,7 +112,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): kconfig.merge_in_entries(base_kunitconfig) return kconfig - def start(self, params: List[str], build_dir: str) -> subprocess.Popen: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: kernel_path = os.path.join(build_dir, self._kernel_path) qemu_command = ['qemu-system-' + self._qemu_arch, '-nodefaults', @@ -141,7 +141,7 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): kconfig.merge_in_entries(base_kunitconfig) return kconfig - def start(self, params: List[str], build_dir: str) -> subprocess.Popen: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: """Runs the Linux UML binary. Must be named 'linux'.""" linux_bin = os.path.join(build_dir, 'linux') params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt']) -- cgit v1.2.3 From 126901ba3499880c9ed033633817cf7493120fda Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Thu, 16 Mar 2023 15:06:37 -0700 Subject: kunit: tool: remove unused imports and variables We don't run a linter regularly over kunit.py code (the default settings on most don't like kernel style, e.g. tabs) so some of these imports didn't get removed when they stopped being used. Signed-off-by: Daniel Latypov Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit.py | 2 +- tools/testing/kunit/kunit_config.py | 2 +- tools/testing/kunit/kunit_kernel.py | 1 - tools/testing/kunit/kunit_parser.py | 1 - tools/testing/kunit/kunit_tool_test.py | 2 +- 5 files changed, 3 insertions(+), 5 deletions(-) (limited to 'tools/testing/kunit/kunit_kernel.py') diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 741f15420467..52853634ba23 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -123,7 +123,7 @@ def _suites_from_test_list(tests: List[str]) -> List[str]: parts = t.split('.', maxsplit=2) if len(parts) != 2: raise ValueError(f'internal KUnit error, test name should be of the form ".", got "{t}"') - suite, case = parts + suite, _ = parts if not suites or suites[-1] != suite: suites.append(suite) return suites diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py index 48b5f34b2e5d..9f76d7b89617 100644 --- a/tools/testing/kunit/kunit_config.py +++ b/tools/testing/kunit/kunit_config.py @@ -8,7 +8,7 @@ from dataclasses import dataclass import re -from typing import Dict, Iterable, List, Set, Tuple +from typing import Dict, Iterable, List, Tuple CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$' CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$' diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index e6fc8fcb071a..775842b912d8 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -18,7 +18,6 @@ import threading from typing import Iterator, List, Optional, Tuple import kunit_config -from kunit_printer import stdout import qemu_config KCONFIG_PATH = '.config' diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index a225799f6b1b..fbc094f0567e 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -12,7 +12,6 @@ from __future__ import annotations from dataclasses import dataclass import re -import sys import textwrap from enum import Enum, auto diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index 0c2190514103..be35999bb84f 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -328,7 +328,7 @@ class KUnitParserTest(unittest.TestCase): def test_parse_subtest_header(self): ktap_log = test_data_path('test_parse_subtest_header.log') with open(ktap_log) as file: - result = kunit_parser.parse_run_tests(file.readlines()) + kunit_parser.parse_run_tests(file.readlines()) self.print_mock.assert_any_call(StrContains('suite (1 subtest)')) def test_show_test_output_on_failure(self): -- cgit v1.2.3 From 1da2e6220e1115930694c649605534baf6fa3dea Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Thu, 16 Mar 2023 15:06:38 -0700 Subject: kunit: tool: fix pre-existing `mypy --strict` errors and update run_checks.py Basically, get this command to be happy and make run_checks.py happy $ mypy --strict --exclude '_test.py$' --exclude qemu_configs/ ./tools/testing/kunit/ Primarily the changes are * add `-> None` return type annotations * add all the missing argument type annotations Previously, we had false positives from mypy in `main()`, see commit 09641f7c7d8f ("kunit: tool: surface and address more typing issues"). But after commit 2dc9d6ca52a4 ("kunit: kunit.py extract handlers") refactored things, the variable name reuse mypy hated is gone. Note: mypy complains we don't annotate the types the unused args in our signal handler. That's silly. But to make it happy, I've copy-pasted an appropriate annotation from https://github.com/python/typing/discussions/1042#discussioncomment-2013595. Reported-by: Johannes Berg Link: https://lore.kernel.org/linux-kselftest/9a172b50457f4074af41fe1dc8e55dcaf4795d7e.camel@sipsolutions.net/ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit.py | 24 ++++++++++++------------ tools/testing/kunit/kunit_config.py | 4 ++-- tools/testing/kunit/kunit_kernel.py | 29 +++++++++++++++-------------- tools/testing/kunit/run_checks.py | 4 ++-- 4 files changed, 31 insertions(+), 30 deletions(-) (limited to 'tools/testing/kunit/kunit_kernel.py') diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 52853634ba23..3905c43369c3 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -269,7 +269,7 @@ def massage_argv(argv: Sequence[str]) -> Sequence[str]: def get_default_jobs() -> int: return len(os.sched_getaffinity(0)) -def add_common_opts(parser) -> None: +def add_common_opts(parser: argparse.ArgumentParser) -> None: parser.add_argument('--build_dir', help='As in the make command, it specifies the build ' 'directory.', @@ -320,13 +320,13 @@ def add_common_opts(parser) -> None: help='Additional QEMU arguments, e.g. "-smp 8"', action='append', metavar='') -def add_build_opts(parser) -> None: +def add_build_opts(parser: argparse.ArgumentParser) -> None: parser.add_argument('--jobs', help='As in the make command, "Specifies the number of ' 'jobs (commands) to run simultaneously."', type=int, default=get_default_jobs(), metavar='N') -def add_exec_opts(parser) -> None: +def add_exec_opts(parser: argparse.ArgumentParser) -> 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 ' @@ -351,7 +351,7 @@ def add_exec_opts(parser) -> None: type=str, choices=['suite', 'test']) -def add_parse_opts(parser) -> None: +def add_parse_opts(parser: argparse.ArgumentParser) -> None: parser.add_argument('--raw_output', help='If set don\'t parse output from kernel. ' 'By default, filters to just KUnit output. Use ' '--raw_output=all to show everything', @@ -386,7 +386,7 @@ def tree_from_args(cli_args: argparse.Namespace) -> kunit_kernel.LinuxSourceTree extra_qemu_args=qemu_args) -def run_handler(cli_args): +def run_handler(cli_args: argparse.Namespace) -> None: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) @@ -405,7 +405,7 @@ def run_handler(cli_args): sys.exit(1) -def config_handler(cli_args): +def config_handler(cli_args: argparse.Namespace) -> None: if cli_args.build_dir and ( not os.path.exists(cli_args.build_dir)): os.mkdir(cli_args.build_dir) @@ -421,7 +421,7 @@ def config_handler(cli_args): sys.exit(1) -def build_handler(cli_args): +def build_handler(cli_args: argparse.Namespace) -> None: linux = tree_from_args(cli_args) request = KunitBuildRequest(build_dir=cli_args.build_dir, make_options=cli_args.make_options, @@ -434,7 +434,7 @@ def build_handler(cli_args): sys.exit(1) -def exec_handler(cli_args): +def exec_handler(cli_args: argparse.Namespace) -> None: linux = tree_from_args(cli_args) exec_request = KunitExecRequest(raw_output=cli_args.raw_output, build_dir=cli_args.build_dir, @@ -450,10 +450,10 @@ def exec_handler(cli_args): sys.exit(1) -def parse_handler(cli_args): +def parse_handler(cli_args: argparse.Namespace) -> None: if cli_args.file is None: - sys.stdin.reconfigure(errors='backslashreplace') # pytype: disable=attribute-error - kunit_output = sys.stdin + sys.stdin.reconfigure(errors='backslashreplace') # type: ignore + kunit_output = sys.stdin # type: Iterable[str] else: with open(cli_args.file, 'r', errors='backslashreplace') as f: kunit_output = f.read().splitlines() @@ -475,7 +475,7 @@ subcommand_handlers_map = { } -def main(argv): +def main(argv: Sequence[str]) -> None: parser = argparse.ArgumentParser( description='Helps writing and running KUnit tests.') subparser = parser.add_subparsers(dest='subcommand') diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py index 9f76d7b89617..eb5dd01210b1 100644 --- a/tools/testing/kunit/kunit_config.py +++ b/tools/testing/kunit/kunit_config.py @@ -8,7 +8,7 @@ from dataclasses import dataclass import re -from typing import Dict, Iterable, List, Tuple +from typing import Any, Dict, Iterable, List, Tuple CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$' CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$' @@ -34,7 +34,7 @@ class Kconfig: def __init__(self) -> None: self._entries = {} # type: Dict[str, str] - def __eq__(self, other) -> bool: + def __eq__(self, other: Any) -> bool: if not isinstance(other, self.__class__): return False return self._entries == other._entries diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 775842b912d8..a3321a991f11 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -16,6 +16,7 @@ import shutil import signal import threading from typing import Iterator, List, Optional, Tuple +from types import FrameType import kunit_config import qemu_config @@ -56,7 +57,7 @@ class LinuxSourceTreeOperations: def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: return base_kunitconfig - def make_olddefconfig(self, build_dir: str, make_options) -> None: + def make_olddefconfig(self, build_dir: str, make_options: Optional[List[str]]) -> None: command = ['make', 'ARCH=' + self._linux_arch, 'O=' + build_dir, 'olddefconfig'] if self._cross_compile: command += ['CROSS_COMPILE=' + self._cross_compile] @@ -70,7 +71,7 @@ class LinuxSourceTreeOperations: except subprocess.CalledProcessError as e: raise ConfigError(e.output.decode()) - def make(self, jobs, build_dir: str, make_options) -> None: + def make(self, jobs: int, build_dir: str, make_options: Optional[List[str]]) -> None: command = ['make', 'ARCH=' + self._linux_arch, 'O=' + build_dir, '--jobs=' + str(jobs)] if make_options: command.extend(make_options) @@ -132,7 +133,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): """An abstraction over command line operations performed on a source tree.""" - def __init__(self, cross_compile=None): + def __init__(self, cross_compile: Optional[str]=None): super().__init__(linux_arch='um', cross_compile=cross_compile) def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: @@ -215,7 +216,7 @@ def _get_qemu_ops(config_path: str, if not hasattr(config, 'QEMU_ARCH'): raise ValueError('qemu_config module missing "QEMU_ARCH": ' + config_path) - params: qemu_config.QemuArchParams = config.QEMU_ARCH # type: ignore + params: qemu_config.QemuArchParams = config.QEMU_ARCH if extra_qemu_args: params.extra_qemu_params.extend(extra_qemu_args) return params.linux_arch, LinuxSourceTreeOperationsQemu( @@ -229,10 +230,10 @@ class LinuxSourceTree: build_dir: str, kunitconfig_paths: Optional[List[str]]=None, kconfig_add: Optional[List[str]]=None, - arch=None, - cross_compile=None, - qemu_config_path=None, - extra_qemu_args=None) -> None: + arch: Optional[str]=None, + cross_compile: Optional[str]=None, + qemu_config_path: Optional[str]=None, + extra_qemu_args: Optional[List[str]]=None) -> None: signal.signal(signal.SIGINT, self.signal_handler) if qemu_config_path: self._arch, self._ops = _get_qemu_ops(qemu_config_path, extra_qemu_args, cross_compile) @@ -275,7 +276,7 @@ class LinuxSourceTree: logging.error(message) return False - def build_config(self, build_dir: str, make_options) -> bool: + def build_config(self, build_dir: str, make_options: Optional[List[str]]) -> bool: kconfig_path = get_kconfig_path(build_dir) if build_dir and not os.path.exists(build_dir): os.mkdir(build_dir) @@ -303,7 +304,7 @@ class LinuxSourceTree: old_kconfig = kunit_config.parse_file(old_path) return old_kconfig != self._kconfig - def build_reconfig(self, build_dir: str, make_options) -> bool: + def build_reconfig(self, build_dir: str, make_options: Optional[List[str]]) -> bool: """Creates a new .config if it is not a subset of the .kunitconfig.""" kconfig_path = get_kconfig_path(build_dir) if not os.path.exists(kconfig_path): @@ -319,7 +320,7 @@ class LinuxSourceTree: os.remove(kconfig_path) return self.build_config(build_dir, make_options) - def build_kernel(self, jobs, build_dir: str, make_options) -> bool: + def build_kernel(self, jobs: int, build_dir: str, make_options: Optional[List[str]]) -> bool: try: self._ops.make_olddefconfig(build_dir, make_options) self._ops.make(jobs, build_dir, make_options) @@ -328,7 +329,7 @@ class LinuxSourceTree: return False return self.validate_config(build_dir) - def run_kernel(self, args=None, build_dir='', filter_glob='', timeout=None) -> Iterator[str]: + def run_kernel(self, args: Optional[List[str]]=None, build_dir: str='', filter_glob: str='', timeout: Optional[int]=None) -> Iterator[str]: if not args: args = [] if filter_glob: @@ -339,7 +340,7 @@ class LinuxSourceTree: assert process.stdout is not None # tell mypy it's set # Enforce the timeout in a background thread. - def _wait_proc(): + def _wait_proc() -> None: try: process.wait(timeout=timeout) except Exception as e: @@ -365,6 +366,6 @@ class LinuxSourceTree: waiter.join() subprocess.call(['stty', 'sane']) - def signal_handler(self, unused_sig, unused_frame) -> None: + def signal_handler(self, unused_sig: int, unused_frame: Optional[FrameType]) -> None: logging.error('Build interruption occurred. Cleaning console.') subprocess.call(['stty', 'sane']) diff --git a/tools/testing/kunit/run_checks.py b/tools/testing/kunit/run_checks.py index 61cece1684df..8208c3b3135e 100755 --- a/tools/testing/kunit/run_checks.py +++ b/tools/testing/kunit/run_checks.py @@ -23,7 +23,7 @@ commands: Dict[str, Sequence[str]] = { 'kunit_tool_test.py': ['./kunit_tool_test.py'], 'kunit smoke test': ['./kunit.py', 'run', '--kunitconfig=lib/kunit', '--build_dir=kunit_run_checks'], 'pytype': ['/bin/sh', '-c', 'pytype *.py'], - 'mypy': ['/bin/sh', '-c', 'mypy *.py'], + 'mypy': ['mypy', '--strict', '--exclude', '_test.py$', '--exclude', 'qemu_configs/', '.'], } # The user might not have mypy or pytype installed, skip them if so. @@ -73,7 +73,7 @@ def main(argv: Sequence[str]) -> None: sys.exit(1) -def run_cmd(argv: Sequence[str]): +def run_cmd(argv: Sequence[str]) -> None: subprocess.check_output(argv, stderr=subprocess.STDOUT, cwd=ABS_TOOL_PATH, timeout=TIMEOUT) -- cgit v1.2.3 From 5ffb8629b133fecf7cdd36134ae6d6a1efb46eb5 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 28 Feb 2023 11:31:02 +0100 Subject: kunit: tool: Add support for overriding the QEMU serial port On some platforms, the console is not the first serial port. To make this work, the first serial port in QEMU must be set to "null". Add support for this by adding an optional "serial" parameter, which defaults to "stdio", and can be overridden by platform-specific configuration. Signed-off-by: Geert Uytterhoeven Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_kernel.py | 3 ++- tools/testing/kunit/qemu_config.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'tools/testing/kunit/kunit_kernel.py') diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index a3321a991f11..f01f94106129 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -106,6 +106,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): self._kernel_path = qemu_arch_params.kernel_path self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' self._extra_qemu_params = qemu_arch_params.extra_qemu_params + self._serial = qemu_arch_params.serial def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: kconfig = kunit_config.parse_from_string(self._kconfig) @@ -121,7 +122,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): '-append', ' '.join(params + [self._kernel_command_line]), '-no-reboot', '-nographic', - '-serial', 'stdio'] + self._extra_qemu_params + '-serial', self._serial] + self._extra_qemu_params # Note: shlex.join() does what we want, but requires python 3.8+. print('Running tests with:\n$', ' '.join(shlex.quote(arg) for arg in qemu_command)) return subprocess.Popen(qemu_command, diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py index 0b6a80398ccc..b1fba9016eed 100644 --- a/tools/testing/kunit/qemu_config.py +++ b/tools/testing/kunit/qemu_config.py @@ -17,3 +17,4 @@ class QemuArchParams: kernel_path: str kernel_command_line: str extra_qemu_params: List[str] + serial: str = 'stdio' -- cgit v1.2.3