From a1666863fdf1861188e1378a6dde2a19b1efa1c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 09:56:42 +0200 Subject: [PATCH 1/7] nixos/test-driver: convert to pyproject from setup.py This also makes configuration available if you just run those tools locally. Also use ruff instead of pylint because it's faster and more comprehensive. --- nixos/lib/test-driver/default.nix | 20 ++++++++-------- nixos/lib/test-driver/pyproject.toml | 35 ++++++++++++++++++++++++++++ nixos/lib/test-driver/setup.py | 14 ----------- 3 files changed, 45 insertions(+), 24 deletions(-) create mode 100644 nixos/lib/test-driver/pyproject.toml delete mode 100644 nixos/lib/test-driver/setup.py diff --git a/nixos/lib/test-driver/default.nix b/nixos/lib/test-driver/default.nix index 33313059fff7..6e01e00b4355 100644 --- a/nixos/lib/test-driver/default.nix +++ b/nixos/lib/test-driver/default.nix @@ -4,19 +4,20 @@ , qemu_pkg ? qemu_test , coreutils , imagemagick_light -, libtiff , netpbm , qemu_test , socat +, ruff , tesseract4 , vde2 , extraPythonPackages ? (_ : []) }: -python3Packages.buildPythonApplication rec { +python3Packages.buildPythonApplication { pname = "nixos-test-driver"; version = "1.1"; src = ./.; + format = "pyproject"; propagatedBuildInputs = [ coreutils @@ -31,14 +32,13 @@ python3Packages.buildPythonApplication rec { ++ extraPythonPackages python3Packages; doCheck = true; - nativeCheckInputs = with python3Packages; [ mypy pylint black ]; + nativeCheckInputs = with python3Packages; [ mypy ruff black ]; checkPhase = '' - mypy --disallow-untyped-defs \ - --no-implicit-optional \ - --pretty \ - --no-color-output \ - --ignore-missing-imports ${src}/test_driver - pylint --errors-only --enable=unused-import ${src}/test_driver - black --check --diff ${src}/test_driver + echo -e "\x1b[32m## run mypy\x1b[0m" + mypy test_driver extract-docstrings.py + echo -e "\x1b[32m## run ruff\x1b[0m" + ruff . + echo -e "\x1b[32m## run black\x1b[0m" + black --check --diff . ''; } diff --git a/nixos/lib/test-driver/pyproject.toml b/nixos/lib/test-driver/pyproject.toml new file mode 100644 index 000000000000..68921bc9f850 --- /dev/null +++ b/nixos/lib/test-driver/pyproject.toml @@ -0,0 +1,35 @@ +[build-system] +requires = ["setuptools"] +build-backend = "setuptools.build_meta" + +[project] +name = "nixos-test-driver" +version = "0.0.0" + +[project.scripts] +nixos-test-driver = "test_driver:main" +generate-driver-symbols = "test_driver:generate_driver_symbols" + +[tool.setuptools.packages] +find = {} + +[tool.setuptools.package-data] +test_driver = ["py.typed"] + +[tool.ruff] +line-length = 88 + +select = ["E", "F", "I", "U", "N"] +ignore = ["E501"] + +[tool.black] +line-length = 88 +target-version = ['py39'] +include = '\.pyi?$' + +[tool.mypy] +python_version = "3.10" +warn_redundant_casts = true +disallow_untyped_calls = true +disallow_untyped_defs = true +no_implicit_optional = true diff --git a/nixos/lib/test-driver/setup.py b/nixos/lib/test-driver/setup.py deleted file mode 100644 index 1719b988db68..000000000000 --- a/nixos/lib/test-driver/setup.py +++ /dev/null @@ -1,14 +0,0 @@ -from setuptools import setup, find_packages - -setup( - name="nixos-test-driver", - version='1.1', - packages=find_packages(), - package_data={"test_driver": ["py.typed"]}, - entry_points={ - "console_scripts": [ - "nixos-test-driver=test_driver:main", - "generate-driver-symbols=test_driver:generate_driver_symbols" - ] - }, -) From 9ac9e8407fbe084fb6667bdd3bd0c569ea454271 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 09:59:17 +0200 Subject: [PATCH 2/7] nixos/test-driver: fix type errors in extract-docstrings --- nixos/lib/test-driver/extract-docstrings.py | 42 ++++++++++++--------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/nixos/lib/test-driver/extract-docstrings.py b/nixos/lib/test-driver/extract-docstrings.py index 5aec4c89a9d7..a12e586882a6 100644 --- a/nixos/lib/test-driver/extract-docstrings.py +++ b/nixos/lib/test-driver/extract-docstrings.py @@ -1,5 +1,6 @@ import ast import sys +from pathlib import Path """ This program takes all the Machine class methods and prints its methods in @@ -40,27 +41,34 @@ some_function(param1, param2) """ -assert len(sys.argv) == 2 -with open(sys.argv[1], "r") as f: - module = ast.parse(f.read()) +def main() -> None: + if len(sys.argv) != 2: + print(f"Usage: {sys.argv[0]} ") + sys.exit(1) -class_definitions = (node for node in module.body if isinstance(node, ast.ClassDef)) + module = ast.parse(Path(sys.argv[1]).read_text()) -machine_class = next(filter(lambda x: x.name == "Machine", class_definitions)) -assert machine_class is not None + class_definitions = (node for node in module.body if isinstance(node, ast.ClassDef)) -function_definitions = [ - node for node in machine_class.body if isinstance(node, ast.FunctionDef) -] -function_definitions.sort(key=lambda x: x.name) + machine_class = next(filter(lambda x: x.name == "Machine", class_definitions)) + assert machine_class is not None -for f in function_definitions: - docstr = ast.get_docstring(f) - if docstr is not None: - args = ", ".join((a.arg for a in f.args.args[1:])) - args = f"({args})" + function_definitions = [ + node for node in machine_class.body if isinstance(node, ast.FunctionDef) + ] + function_definitions.sort(key=lambda x: x.name) - docstr = "\n".join((f" {l}" for l in docstr.strip().splitlines())) + for f in function_definitions: + docstr = ast.get_docstring(f) + if docstr is not None: + args = ", ".join(a.arg for a in f.args.args[1:]) + args = f"({args})" - print(f"{f.name}{args}\n\n:{docstr[1:]}\n") + docstr = "\n".join(f" {l}" for l in docstr.strip().splitlines()) + + print(f"{f.name}{args}\n\n:{docstr[1:]}\n") + + +if __name__ == "__main__": + main() From d7465572601648cadf292b23baf962afc0b0760a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 10:13:51 +0200 Subject: [PATCH 3/7] nixos/test-driver: add shell.nix this useful for local development --- nixos/lib/test-driver/shell.nix | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 nixos/lib/test-driver/shell.nix diff --git a/nixos/lib/test-driver/shell.nix b/nixos/lib/test-driver/shell.nix new file mode 100644 index 000000000000..367bbad556c0 --- /dev/null +++ b/nixos/lib/test-driver/shell.nix @@ -0,0 +1,2 @@ +with import ../../.. {}; +pkgs.callPackage ./default.nix {} From 93b1fa09d52970b449b36b8649b2831dad08604e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 10:14:56 +0200 Subject: [PATCH 4/7] nixos/test-driver: disable typecheck for dependencies where don't have typing --- nixos/lib/test-driver/pyproject.toml | 9 +++++++++ nixos/lib/test-driver/test_driver/logger.py | 3 +++ 2 files changed, 12 insertions(+) diff --git a/nixos/lib/test-driver/pyproject.toml b/nixos/lib/test-driver/pyproject.toml index 68921bc9f850..8638f14dfdae 100644 --- a/nixos/lib/test-driver/pyproject.toml +++ b/nixos/lib/test-driver/pyproject.toml @@ -22,6 +22,15 @@ line-length = 88 select = ["E", "F", "I", "U", "N"] ignore = ["E501"] +# xxx: we can import https://pypi.org/project/types-colorama/ here +[[tool.mypy.overrides]] +module = "colorama.*" +ignore_missing_imports = true + +[[tool.mypy.overrides]] +module = "ptpython.*" +ignore_missing_imports = true + [tool.black] line-length = 88 target-version = ['py39'] diff --git a/nixos/lib/test-driver/test_driver/logger.py b/nixos/lib/test-driver/test_driver/logger.py index e6182ff7c761..ea14c50dc517 100644 --- a/nixos/lib/test-driver/test_driver/logger.py +++ b/nixos/lib/test-driver/test_driver/logger.py @@ -1,3 +1,6 @@ +# mypy: disable-error-code="no-untyped-call" +# drop the above line when mypy is upgraded to include +# https://github.com/python/typeshed/commit/49b717ca52bf0781a538b04c0d76a5513f7119b8 from colorama import Style, Fore from contextlib import contextmanager from typing import Any, Dict, Iterator From a1f01abe53e9b20965df7b6c929bc2a129052551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 10:18:47 +0200 Subject: [PATCH 5/7] nixos/test-driver: apply ruff fixes & suggestions --- nixos/lib/test-driver/extract-docstrings.py | 10 +++++----- nixos/lib/test-driver/test_driver/__init__.py | 11 +++++------ nixos/lib/test-driver/test_driver/driver.py | 8 ++++---- nixos/lib/test-driver/test_driver/logger.py | 11 ++++++----- nixos/lib/test-driver/test_driver/machine.py | 12 ++++++------ .../lib/test-driver/test_driver/polling_condition.py | 4 ++-- nixos/lib/test-driver/test_driver/vlan.py | 2 +- 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/nixos/lib/test-driver/extract-docstrings.py b/nixos/lib/test-driver/extract-docstrings.py index a12e586882a6..64850ca711f3 100644 --- a/nixos/lib/test-driver/extract-docstrings.py +++ b/nixos/lib/test-driver/extract-docstrings.py @@ -59,15 +59,15 @@ def main() -> None: ] function_definitions.sort(key=lambda x: x.name) - for f in function_definitions: - docstr = ast.get_docstring(f) + for function in function_definitions: + docstr = ast.get_docstring(function) if docstr is not None: - args = ", ".join(a.arg for a in f.args.args[1:]) + args = ", ".join(a.arg for a in function.args.args[1:]) args = f"({args})" - docstr = "\n".join(f" {l}" for l in docstr.strip().splitlines()) + docstr = "\n".join(f" {line}" for line in docstr.strip().splitlines()) - print(f"{f.name}{args}\n\n:{docstr[1:]}\n") + print(f"{function.name}{args}\n\n:{docstr[1:]}\n") if __name__ == "__main__": diff --git a/nixos/lib/test-driver/test_driver/__init__.py b/nixos/lib/test-driver/test_driver/__init__.py index c90e3d9e1cdb..371719d7a988 100755 --- a/nixos/lib/test-driver/test_driver/__init__.py +++ b/nixos/lib/test-driver/test_driver/__init__.py @@ -1,11 +1,12 @@ -from pathlib import Path import argparse -import ptpython.repl import os import time +from pathlib import Path + +import ptpython.repl -from test_driver.logger import rootlog from test_driver.driver import Driver +from test_driver.logger import rootlog class EnvDefault(argparse.Action): @@ -25,9 +26,7 @@ class EnvDefault(argparse.Action): ) if required and default: required = False - super(EnvDefault, self).__init__( - default=default, required=required, nargs=nargs, **kwargs - ) + super().__init__(default=default, required=required, nargs=nargs, **kwargs) def __call__(self, parser, namespace, values, option_string=None): # type: ignore setattr(namespace, self.dest, values) diff --git a/nixos/lib/test-driver/test_driver/driver.py b/nixos/lib/test-driver/test_driver/driver.py index 835d60ec3b4f..723c80717860 100644 --- a/nixos/lib/test-driver/test_driver/driver.py +++ b/nixos/lib/test-driver/test_driver/driver.py @@ -1,14 +1,14 @@ -from contextlib import contextmanager -from pathlib import Path -from typing import Any, Dict, Iterator, List, Union, Optional, Callable, ContextManager import os import re import tempfile +from contextlib import contextmanager +from pathlib import Path +from typing import Any, Callable, ContextManager, Dict, Iterator, List, Optional, Union from test_driver.logger import rootlog from test_driver.machine import Machine, NixStartScript, retry -from test_driver.vlan import VLan from test_driver.polling_condition import PollingCondition +from test_driver.vlan import VLan def get_tmp_dir() -> Path: diff --git a/nixos/lib/test-driver/test_driver/logger.py b/nixos/lib/test-driver/test_driver/logger.py index ea14c50dc517..116244b5e4ae 100644 --- a/nixos/lib/test-driver/test_driver/logger.py +++ b/nixos/lib/test-driver/test_driver/logger.py @@ -1,16 +1,17 @@ # mypy: disable-error-code="no-untyped-call" # drop the above line when mypy is upgraded to include # https://github.com/python/typeshed/commit/49b717ca52bf0781a538b04c0d76a5513f7119b8 -from colorama import Style, Fore -from contextlib import contextmanager -from typing import Any, Dict, Iterator -from queue import Queue, Empty -from xml.sax.saxutils import XMLGenerator import codecs import os import sys import time import unicodedata +from contextlib import contextmanager +from queue import Empty, Queue +from typing import Any, Dict, Iterator +from xml.sax.saxutils import XMLGenerator + +from colorama import Fore, Style class Logger: diff --git a/nixos/lib/test-driver/test_driver/machine.py b/nixos/lib/test-driver/test_driver/machine.py index 4accd2f9d195..5ee842450606 100644 --- a/nixos/lib/test-driver/test_driver/machine.py +++ b/nixos/lib/test-driver/test_driver/machine.py @@ -1,7 +1,3 @@ -from contextlib import _GeneratorContextManager, nullcontext -from pathlib import Path -from queue import Queue -from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple import base64 import io import os @@ -16,6 +12,10 @@ import sys import tempfile import threading import time +from contextlib import _GeneratorContextManager, nullcontext +from pathlib import Path +from queue import Queue +from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple from test_driver.logger import rootlog @@ -599,7 +599,7 @@ class Machine: return (-1, output.decode()) # Get the return code - self.shell.send("echo ${PIPESTATUS[0]}\n".encode()) + self.shell.send(b"echo ${PIPESTATUS[0]}\n") rc = int(self._next_newline_closed_block_from_shell().strip()) return (rc, output.decode(errors="replace")) @@ -1132,7 +1132,7 @@ class Machine: return assert self.shell - self.shell.send("poweroff\n".encode()) + self.shell.send(b"poweroff\n") self.wait_for_shutdown() def crash(self) -> None: diff --git a/nixos/lib/test-driver/test_driver/polling_condition.py b/nixos/lib/test-driver/test_driver/polling_condition.py index 02ca0a03ab3d..895ac6cb72f5 100644 --- a/nixos/lib/test-driver/test_driver/polling_condition.py +++ b/nixos/lib/test-driver/test_driver/polling_condition.py @@ -1,6 +1,6 @@ -from typing import Callable, Optional -from math import isfinite import time +from math import isfinite +from typing import Callable, Optional from .logger import rootlog diff --git a/nixos/lib/test-driver/test_driver/vlan.py b/nixos/lib/test-driver/test_driver/vlan.py index f2a7f250d1d2..ec9679108e58 100644 --- a/nixos/lib/test-driver/test_driver/vlan.py +++ b/nixos/lib/test-driver/test_driver/vlan.py @@ -1,8 +1,8 @@ -from pathlib import Path import io import os import pty import subprocess +from pathlib import Path from test_driver.logger import rootlog From 1810265b57aa29198522bec882514f413574189c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 10:23:21 +0200 Subject: [PATCH 6/7] nixos/test-driver: name exception according to pep8 see https://docs.astral.sh/ruff/rules/error-suffix-on-exception-name/ --- nixos/lib/test-driver/test_driver/polling_condition.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nixos/lib/test-driver/test_driver/polling_condition.py b/nixos/lib/test-driver/test_driver/polling_condition.py index 895ac6cb72f5..12cbad69e34e 100644 --- a/nixos/lib/test-driver/test_driver/polling_condition.py +++ b/nixos/lib/test-driver/test_driver/polling_condition.py @@ -5,7 +5,7 @@ from typing import Callable, Optional from .logger import rootlog -class PollingConditionFailed(Exception): +class PollingConditionError(Exception): pass @@ -60,7 +60,7 @@ class PollingCondition: def maybe_raise(self) -> None: if not self.check(): - raise PollingConditionFailed(self.status_message(False)) + raise PollingConditionError(self.status_message(False)) def status_message(self, status: bool) -> str: return f"Polling condition {'succeeded' if status else 'failed'}: {self.description}" From f1450e660f4adb3df96e0608ff6b850c3b235ddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 30 Sep 2023 10:27:28 +0200 Subject: [PATCH 7/7] nixos/test-driver: whitelist variable names that don't follow pep8 https://docs.astral.sh/ruff/rules/invalid-argument-name/ --- nixos/lib/test-driver/test_driver/machine.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nixos/lib/test-driver/test_driver/machine.py b/nixos/lib/test-driver/test_driver/machine.py index 5ee842450606..7ed001a1dfce 100644 --- a/nixos/lib/test-driver/test_driver/machine.py +++ b/nixos/lib/test-driver/test_driver/machine.py @@ -236,14 +236,14 @@ class LegacyStartCommand(StartCommand): def __init__( self, - netBackendArgs: Optional[str] = None, - netFrontendArgs: Optional[str] = None, + netBackendArgs: Optional[str] = None, # noqa: N803 + netFrontendArgs: Optional[str] = None, # noqa: N803 hda: Optional[Tuple[Path, str]] = None, cdrom: Optional[str] = None, usb: Optional[str] = None, bios: Optional[str] = None, - qemuBinary: Optional[str] = None, - qemuFlags: Optional[str] = None, + qemuBinary: Optional[str] = None, # noqa: N803 + qemuFlags: Optional[str] = None, # noqa: N803 ): if qemuBinary is not None: self._cmd = qemuBinary