From 40f6b16cfdd772e808cb02ac63279ac2ed7fbcd5 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 25 Jun 2024 15:25:57 +0200 Subject: [PATCH 01/16] Drop pint higher-bound (#2741) Closes #2740 Signed-off-by: Cristian Le --- .pre-commit-config.yaml | 4 ++-- pyproject.toml | 5 +++-- tmt/hardware.py | 4 +++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d28627af6e..a8f45c4f0e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -36,7 +36,7 @@ repos: - "docutils>=0.16" # 0.16 is the current one available for RHEL9 - "fmf>=1.3.0" - "jinja2>=2.11.3" # 3.1.2 / 3.1.2 - - "pint>=0.16.1,<0.20" # 0.16.1 / 0.19.x TODO: Pint 0.20 requires larger changes to tmt.hardware + - "pint>=0.16.1" # 0.16.1 - "pygments>=2.7.4" # 2.7.4 is the current one available for RHEL9 - "requests>=2.25.1" # 2.28.2 / 2.31.0 - "ruamel.yaml>=0.16.6" # 0.17.32 / 0.17.32 @@ -81,7 +81,7 @@ repos: - "docutils>=0.16" # 0.16 is the current one available for RHEL9 - "fmf>=1.3.0" - "jinja2>=2.11.3" # 3.1.2 / 3.1.2 - - "pint>=0.16.1,<0.20" # 0.16.1 / 0.19.x TODO: Pint 0.20 requires larger changes to tmt.hardware + - "pint>=0.16.1" # 0.16.1 / 0.19.x TODO: Pint 0.20 requires larger changes to tmt.hardware - "pygments>=2.7.4" # 2.7.4 is the current one available for RHEL9 - "requests>=2.25.1" # 2.28.2 / 2.31.0 - "ruamel.yaml>=0.16.6" # 0.17.32 / 0.17.32 diff --git a/pyproject.toml b/pyproject.toml index 9c6587aa1e..f815aafcef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ dependencies = [ # F39 / PyPI "docutils>=0.16", # 0.16 is the current one available for RHEL9 "fmf>=1.3.0", "jinja2>=2.11.3", # 3.1.2 / 3.1.2 - "pint>=0.16.1,<0.20", # 0.16.1 / 0.19.x TODO: Pint 0.20 requires larger changes to tmt.hardware + "pint>=0.16.1", # 0.16.1 "pygments>=2.7.4", # 2.7.4 is the current one available for RHEL9 "requests>=2.25.1", # 2.28.2 / 2.31.0 "ruamel.yaml>=0.16.6", # 0.17.32 / 0.17.32 @@ -249,7 +249,8 @@ ignore = [ "tmt/convert.py", "tmt/lint.py", "tmt/queue.py", - "tmt/utils.py" + "tmt/utils.py", + "tmt/hardware.py", # pyright does not pick up pint's _typing.py or something :/ ] pythonVersion = "3.9" diff --git a/tmt/hardware.py b/tmt/hardware.py index fa8bb0fa34..916fe73062 100644 --- a/tmt/hardware.py +++ b/tmt/hardware.py @@ -64,7 +64,9 @@ from typing_extensions import TypeAlias #: A type of values describing sizes of things like storage or RAM. - Size: TypeAlias = 'Quantity[int]' + # Note: type-hinting is a bit wonky with pyright + # https://github.com/hgrecco/pint/issues/1166 + Size: TypeAlias = Quantity #: Unit registry, used and shared by all code. UNITS = pint.UnitRegistry() From 3e8ad390b08144bc63de6df60fbba45171f921c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Tue, 25 Jun 2024 16:29:17 +0200 Subject: [PATCH 02/16] Fix our custom Fedora CoreOS test image to avoid dnf/dnf5 conflicts (#3014) --- containers/fedora/coreos/Containerfile | 12 +++++++++++- containers/fedora/coreos/ostree/Containerfile | 12 +++++++++++- docs/codespell.ignore | 2 ++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/containers/fedora/coreos/Containerfile b/containers/fedora/coreos/Containerfile index 4e256edb3e..d8c7187369 100644 --- a/containers/fedora/coreos/Containerfile +++ b/containers/fedora/coreos/Containerfile @@ -14,4 +14,14 @@ RUN rpm-ostree install dnf5 \ # Remove diffutils as its used in many package manager tests, and tests # are simpler if all environments lack the same package, we don't have # to parametrize them even more. - && dnf5 remove -y diffutils + # Do *NOT* use dnf5 to remove this package - it might create conflicts + # in /var/lib/dnf should the next command called be `debuginfo-install` + # or any other dnf4-ish command. + && rpm-ostree uninstall diffutils \ + # Removing diffutils, these need to be removed too. + containers-common-extra \ + passt \ + passt-selinux \ + podman \ + policycoreutils \ + toolbox diff --git a/containers/fedora/coreos/ostree/Containerfile b/containers/fedora/coreos/ostree/Containerfile index fd3609b3d2..1387e9f580 100644 --- a/containers/fedora/coreos/ostree/Containerfile +++ b/containers/fedora/coreos/ostree/Containerfile @@ -16,6 +16,16 @@ RUN rpm-ostree install dnf5 \ # Remove diffutils as its used in many package manager tests, and tests # are simpler if all environments lack the same package, we don't have # to parametrize them even more. - && dnf5 remove -y diffutils \ + # Do *NOT* use dnf5 to remove this package - it might create conflicts + # in /var/lib/dnf should the next command called be `debuginfo-install` + # or any other dnf4-ish command. + && rpm-ostree uninstall diffutils \ + # Removing diffutils, these need to be removed too. + containers-common-extra \ + passt \ + passt-selinux \ + podman \ + policycoreutils \ + toolbox \ # Simulate ostree-booted environment && touch /run/ostree-booted diff --git a/docs/codespell.ignore b/docs/codespell.ignore index d4a6693244..fab1ad29a6 100644 --- a/docs/codespell.ignore +++ b/docs/codespell.ignore @@ -1 +1,3 @@ # Override the parent implementation - it would try to call `Tree.storys()`... + passt \ + passt-selinux \ From 1ec3309db698db4df9a236e04957122e2ccd5149 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Tue, 25 Jun 2024 20:03:48 +0200 Subject: [PATCH 03/16] Add xfail for pip installation tests on py3.13 (#3043) --- tests/pip/install.fmf | 6 ++++++ tests/precommit/main.fmf | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/pip/install.fmf b/tests/pip/install.fmf index b7a0b735bd..f212f4a135 100644 --- a/tests/pip/install.fmf +++ b/tests/pip/install.fmf @@ -5,6 +5,12 @@ require: - python3 - python3-devel tier: null +adjust: + when: distro == fedora-rawhide + result: xfail + # 'mini' should start passing once https://github.com/hgrecco/pint/issues/1969 is resolved + # if/once that happens, the xfail should be moved to 'full' only + because: "Un-installable dependencies on Python 3.13" /mini: summary: Ensure the minimal pip install works diff --git a/tests/precommit/main.fmf b/tests/precommit/main.fmf index e8bae07b4b..b24a42be9f 100644 --- a/tests/precommit/main.fmf +++ b/tests/precommit/main.fmf @@ -4,3 +4,9 @@ require: - git-core - tmt tier: 4 +adjust: + when: distro == fedora-rawhide + result: xfail + # Remove the xfail adjust once it starts passing. + # Dependent on https://github.com/crate-py/rpds/issues/72, PyO3 0.22 + because: "Un-installable dependencies on Python 3.13" From 463c5c4b37d732976f8cfe48b7c1097ae20e6c72 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 10 Apr 2024 17:26:57 +0200 Subject: [PATCH 04/16] Add flake8-bandit ruff rules --- pyproject.toml | 16 ++++++++++++++++ tests/integration/test_nitrate.py | 4 ++-- tests/unit/test_export_to_nitrate.py | 4 ++-- tmt/base.py | 3 ++- tmt/cli.py | 11 ++++++----- tmt/steps/execute/internal.py | 2 +- tmt/steps/provision/__init__.py | 11 +++++------ tmt/steps/report/polarion.py | 7 ++++--- tmt/utils.py | 9 +++++++-- 9 files changed, 45 insertions(+), 22 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f815aafcef..657557e869 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -291,6 +291,7 @@ lint.select = [ "B", # flake8-bugbear "C4", # flake8-comprehensions "YTT", # flake8-2020 + "S", # flake8-bandit "PT", # flake8-pytest-style "RET", # flake8-return "SIM", # flake8-simplify @@ -321,6 +322,12 @@ lint.ignore = [ "PLE1205", # Too many arguments for `logging` format string "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar` "RUF013", # PEP 484 prohibits implicit `Optional` + # flake8-bandit (S) https://docs.astral.sh/ruff/rules/#flake8-bandit-s + "S101", # Use of `assert` detected + "S602", # `subprocess` call with `shell=True` identified + "S603", # `subprocess` call: check for execution of untrusted input + "S607", # Starting a process with a partial executable path + "S105", # Possible hardcoded password assigned to: "PASS" # pydocstyle # TODO: the permanent list (drop this comment once the temporary list @@ -346,6 +353,15 @@ lint.ignore = [ "D415", # First line should end with a period, question mark, or exclamation point ] +[tool.ruff.lint.per-file-ignores] +# Less strict security checks in tests +"tests/unit*" = [ + "S604", # Function call with shell=True parameter identified, security issue + "S605", # Starting a process with a shell: seems safe, but may be changed in the future + "S318", # Using xml to parse untrusted data is known to be vulnerable to XML attacks + "S108", # Probable insecure usage of temporary file or directory: "{}" + ] + [tool.ruff.lint.flake8-bugbear] extend-immutable-calls = ["tmt.utils.field"] diff --git a/tests/integration/test_nitrate.py b/tests/integration/test_nitrate.py index eb13e959a5..a29caa5511 100644 --- a/tests/integration/test_nitrate.py +++ b/tests/integration/test_nitrate.py @@ -24,8 +24,8 @@ class Base(RequreTestCase): def setUp(self): super().setUp() - self.tmpdir = Path(tempfile.mktemp(prefix=str(TEST_DIR))) - shutil.copytree(self.EXAMPLES, self.tmpdir) + self.tmpdir = Path(tempfile.mkdtemp(prefix=str(TEST_DIR))) + shutil.copytree(self.EXAMPLES, self.tmpdir, dirs_exist_ok=True) self.cwd = os.getcwd() self.runner_output = None diff --git a/tests/unit/test_export_to_nitrate.py b/tests/unit/test_export_to_nitrate.py index 7e336e9267..9547d4da80 100644 --- a/tests/unit/test_export_to_nitrate.py +++ b/tests/unit/test_export_to_nitrate.py @@ -11,8 +11,8 @@ class NitrateExportAutomated(TestCase): def setUp(self): - self.tmp_dir = Path(tempfile.mktemp(prefix=str(TEST_DIR))) - shutil.copytree(TEST_DIR, self.tmp_dir) + self.tmp_dir = Path(tempfile.mkdtemp(prefix=str(TEST_DIR))) + shutil.copytree(TEST_DIR, self.tmp_dir, dirs_exist_ok=True) self.cwd = os.getcwd() self.dir_name = 'manual_test' diff --git a/tmt/base.py b/tmt/base.py index 375c61d285..87c352e270 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -2824,8 +2824,9 @@ def _filters_conditions( # Links are in OR relation if links and all(not node.has_link(needle) for needle in links): continue - except BaseException: + except Exception as exc: # Handle broken link as not matching + self.debug(f'Invalid link ignored, exception was {exc}') continue # Exclude if any(node for expr in excludes if re.search(expr, node.name)): diff --git a/tmt/cli.py b/tmt/cli.py index 03963ae4f9..c4a6d8c5fa 100644 --- a/tmt/cli.py +++ b/tmt/cli.py @@ -2030,13 +2030,11 @@ def setup_completion(shell: str, install: bool) -> None: else: script = Path(config.path) / f'{COMPLETE_SCRIPT}.{shell}' - # SIM115: Use context handler for opening files. Would not reduce complexity here. - out = open(script, 'w') if install else sys.stdout # noqa: SIM115 - subprocess.run(f'{COMPLETE_VARIABLE}={shell}_source tmt', - shell=True, stdout=out) + command = f'{COMPLETE_VARIABLE}={shell}_source tmt' if install: - out.close() + with open(script, 'w') as f: + subprocess.run(command, stdout=f, shell=True, check=False) # If requested, modify .bashrc or .zshrc if shell != 'fish': config_path = Path(f'~/.{shell}rc').expanduser() @@ -2044,6 +2042,9 @@ def setup_completion(shell: str, install: bool) -> None: shell_config.write('\n# Generated by tmt\n') shell_config.write(f'source {script}') + else: + subprocess.run(command, stdout=sys.stdout, shell=True, check=False) + @completion.command(name='bash') @pass_context diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index e3c21914ef..a82f07fe25 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -32,7 +32,7 @@ TEST_PIDFILE_LOCK_FILENAME = f'{TEST_PIDFILE_FILENAME}.lock' #: The default directory for storing test pid file. -TEST_PIDFILE_ROOT = Path('/var/tmp') +TEST_PIDFILE_ROOT = Path('/var/tmp') # noqa: S108 insecure usage of temporary dir def effective_pidfile_root() -> Path: diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index c9473de7a7..e724f113c3 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -3,8 +3,8 @@ import datetime import enum import os -import random import re +import secrets import shlex import signal as _signal import string @@ -778,7 +778,7 @@ def _random_name(self, prefix: str = '', length: int = 16) -> str: # Append at least 5 random characters min_random_part = max(5, length - len(prefix)) name = prefix + ''.join( - random.choices(string.ascii_letters, k=min_random_part)) + secrets.choice(string.ascii_letters) for _ in range(min_random_part)) # Return tail (containing random characters) of name return name[-length:] @@ -1391,11 +1391,10 @@ def _ssh_guest(self) -> str: def _ssh_master_socket_path(self) -> Path: """ Return path to the SSH master socket """ - # Use '/run/user/uid' if it exists, '/tmp' otherwise. + # Use '/run/user/uid' if it exists, 'temp dir' otherwise. run_dir = Path(f"/run/user/{os.getuid()}") - socket_dir = run_dir / "tmt" if run_dir.is_dir() else Path("/tmp") - socket_dir.mkdir(exist_ok=True) - return Path(tempfile.mktemp(dir=socket_dir)) + socket_dir = run_dir if run_dir.is_dir() else Path(tempfile.mkdtemp()) + return socket_dir / "tmt" @property def _ssh_options(self) -> Command: diff --git a/tmt/steps/report/polarion.py b/tmt/steps/report/polarion.py index d2bc4e1dc5..1990af9924 100644 --- a/tmt/steps/report/polarion.py +++ b/tmt/steps/report/polarion.py @@ -219,8 +219,8 @@ def go(self) -> None: 'sample_image', 'logs', 'compose_id', 'fips'] junit_suite = make_junit_xml(self) - xml_tree = ElementTree.fromstring(junit_suite.to_xml_string([junit_suite])) - + # S314: Any potential xml parser vulnerability mitigation would require defusedxml package + xml_tree = ElementTree.fromstring(junit_suite.to_xml_string([junit_suite])) # noqa: S314 properties = { 'polarion-project-id': project_id, 'polarion-user-id': PolarionWorkItem._session.user_id, @@ -297,7 +297,8 @@ def go(self) -> None: response = post( polarion_import_url, auth=auth, - files={'file': ('xunit.xml', ElementTree.tostring(xml_tree))}) + files={'file': ('xunit.xml', ElementTree.tostring(xml_tree))}, timeout=10 + ) self.info( f'Response code is {response.status_code} with text: {response.text}') else: diff --git a/tmt/utils.py b/tmt/utils.py index 04b1f1b941..684f1384e6 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -174,7 +174,7 @@ def configure_constant(default: int, envvar: str) -> int: # Default workdir root and max -WORKDIR_ROOT = Path('/var/tmp/tmt') +WORKDIR_ROOT = Path('/var/tmp/tmt') # noqa: S108 insecure usage of temporary dir WORKDIR_MAX = 1000 # Maximum number of lines of stdout/stderr to show upon errors @@ -6980,7 +6980,12 @@ def default_template_environment() -> jinja2.Environment: Adds common filters, and enables block trimming and left strip. """ - environment = jinja2.Environment() + # S701: `autoescape=False` is dangerous and can lead to XSS. + # As there can be many different template file formats, used to render various formats, + # we need to explicitly set autoescape=False, as default might change in the future. + # Potential improvements are being tracked in /teemtee/tmt/issues/2873 + + environment = jinja2.Environment(autoescape=False) # noqa: S701 def regex_search( string: str, From 9f38bf51dcf07c691afb5c1410d4b6d63b2a8054 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 10 Apr 2024 18:04:27 +0200 Subject: [PATCH 05/16] Add flake8-builtins ruff rules --- pyproject.toml | 4 ++++ tmt/options.py | 4 ++-- tmt/steps/discover/fmf.py | 4 ++-- tmt/templates/__init__.py | 4 ++-- tmt/utils.py | 14 ++++++++------ 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 657557e869..09b2d7de16 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -289,6 +289,7 @@ lint.select = [ "D", # pydocstyle "UP", # pyupgrade "B", # flake8-bugbear + "A", # flake8-builtins "C4", # flake8-comprehensions "YTT", # flake8-2020 "S", # flake8-bandit @@ -375,6 +376,9 @@ extend-immutable-calls = ["tmt.utils.field"] convention = "pep257" property-decorators = ["tmt.utils.cached_property"] +[tool.ruff.lint.flake8-builtins] +builtins-ignorelist = ["help", "format", "input", "filter", "copyright", "max"] + [tool.ruff.lint.isort] known-first-party = ["tmt"] diff --git a/tmt/options.py b/tmt/options.py index 811c0baa74..af45b91e08 100644 --- a/tmt/options.py +++ b/tmt/options.py @@ -71,7 +71,7 @@ def option( is_flag: bool = False, multiple: bool = False, count: bool = False, - type: Optional[Union[click.Choice, Any]] = None, + type: Optional[Union[click.Choice, Any]] = None, # noqa: A002 `type` is shadowing a Python builtin help: Optional[str] = None, required: bool = False, default: Optional[Any] = None, @@ -108,7 +108,7 @@ def option( help = deprecated.rendered if choices is not None: - type = click.Choice(choices) + type = click.Choice(choices) # noqa: A001 `type` is shadowing a Python builtin # Add a metavar listing choices unless an explicit metavar has been provided if isinstance(type, click.Choice) and metavar is None: diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index a09c47561d..88fa1fbd9c 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -278,11 +278,11 @@ def is_in_standalone_mode(self) -> bool: return True return super().is_in_standalone_mode - def get_git_root(self, dir: Path) -> Path: + def get_git_root(self, directory: Path) -> Path: """ Find git root of the path """ output = self.run( Command("git", "rev-parse", "--show-toplevel"), - cwd=dir, + cwd=directory, ignore_dry=True) assert output.stdout is not None return Path(output.stdout.strip("\n")) diff --git a/tmt/templates/__init__.py b/tmt/templates/__init__.py index e45387c60d..579da44dd8 100644 --- a/tmt/templates/__init__.py +++ b/tmt/templates/__init__.py @@ -53,9 +53,9 @@ def _get_templates(root_dir: Path) -> TemplatesType: return templates -def _append_newline_if_missing(input: str) -> str: +def _append_newline_if_missing(input_string: str) -> str: """ Append newline to the input if it doesn't end with one. """ - return input if input.endswith('\n') else input + '\n' + return input_string if input_string.endswith('\n') else input_string + '\n' class TemplateManager: diff --git a/tmt/utils.py b/tmt/utils.py index 684f1384e6..03365a1dde 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -2605,7 +2605,7 @@ def quote(string: str) -> str: return f'"{string}"' -def ascii(text: Any) -> bytes: +def pure_ascii(text: Any) -> bytes: """ Transliterate special unicode characters into pure ascii """ if not isinstance(text, str): text = str(text) @@ -2632,7 +2632,7 @@ def filter_paths(directory: Path, searching: list[str], files_only: bool = False Returns list of matching paths. """ all_paths = list(directory.rglob('*')) # get all filepaths for given dir recursively - alldirs = [str(dir) for dir in all_paths if dir.is_dir()] + alldirs = [str(d) for d in all_paths if d.is_dir()] allfiles = [str(file) for file in all_paths if not file.is_dir()] found_paths: list[str] = [] @@ -5129,7 +5129,7 @@ def get( content = self._sections[section] except KeyError: raise StructuredFieldError( - f"Section [{ascii(section)!r}] not found") + f"Section [{pure_ascii(section)!r}] not found") # Return the whole section content if item is None: return content @@ -5138,7 +5138,7 @@ def get( return self._read_section(content)[item] except KeyError: raise StructuredFieldError( - f"Unable to read '{ascii(item)!r}' from section '{ascii(section)!r}'") + f"Unable to read '{pure_ascii(item)!r}' from section '{pure_ascii(section)!r}'") def set(self, section: str, content: Any, item: Optional[str] = None) -> None: @@ -5178,7 +5178,7 @@ def remove(self, section: str, item: Optional[str] = None) -> None: del self._order[self._order.index(section)] except KeyError: raise StructuredFieldError( - f"Section [{ascii(section)!r}] not found") + f"Section [{pure_ascii(section)!r}] not found") # Remove only selected item from the section else: try: @@ -5186,7 +5186,9 @@ def remove(self, section: str, item: Optional[str] = None) -> None: del (dictionary[item]) except KeyError: raise StructuredFieldError( - f"Unable to remove '{ascii(item)!r}' from section '{ascii(section)!r}'") + f"Unable to remove '{pure_ascii(item)!r}' " + f"from section '{pure_ascii(section)!r}'" + ) self._sections[section] = self._write_section(dictionary) From e4c0cbb6accf475487c4de8b7bcd57fbb72afcc8 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 10 Apr 2024 18:43:35 +0200 Subject: [PATCH 06/16] Add flake8-implicit-str-concat ruff rules --- pyproject.toml | 1 + tests/unit/test_report_junit.py | 23 ++++++++++------------- tmt/base.py | 29 +++++++++++++---------------- tmt/export/__init__.py | 17 +++++++---------- tmt/export/nitrate.py | 4 ++-- tmt/log.py | 18 +++++++++--------- tmt/steps/__init__.py | 13 +++++-------- 7 files changed, 47 insertions(+), 58 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 09b2d7de16..31b8e465b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -300,6 +300,7 @@ lint.select = [ "DTZ", # flake8-datetimez "T10", # flake8-debugger "EXE", # flake8-executable + "ISC", # flake8-implicit-str-concat "PIE", # flake8-pie "RSE", # flake8-raise "PGH", # pygrep-hooks diff --git a/tests/unit/test_report_junit.py b/tests/unit/test_report_junit.py index 104bf4e3da..ae8b792bd9 100644 --- a/tests/unit/test_report_junit.py +++ b/tests/unit/test_report_junit.py @@ -52,9 +52,8 @@ def _compare_xml_node(tree_path: list[str], expected: xml.dom.Node, actual: xml. # Make sure node names do match. assert expected.nodeName == actual.nodeName, \ - f"Element name mismatch at {tree_path_joined}: " \ - f"expected {expected.nodeName}, " \ - f"found {actual.nodeName}" + (f"Element name mismatch at {tree_path_joined}: " + f"expected {expected.nodeName}, found {actual.nodeName}") # If nodes have the same tag, move on to attributes. Make sure both nodes # have the same set of attributes, with same respective values. @@ -65,16 +64,15 @@ def _compare_xml_node(tree_path: list[str], expected: xml.dom.Node, actual: xml. actual_attributes = sorted((actual.attributes or {}).items()) assert len(expected_attributes) == len(actual_attributes), \ - f"Attribute count mismatch at {tree_path_joined}: " \ - f"expected {len(expected_attributes)}, " \ - f"found {len(actual_attributes)}" + (f"Attribute count mismatch at {tree_path_joined}: " + f"expected {len(expected_attributes)}, found {len(actual_attributes)}") for (expected_name, expected_value), (actual_name, actual_value) in zip( expected_attributes, actual_attributes): - assert expected_name == actual_name, f"Attribute mismatch at {tree_path_joined}: " \ - f"expected {expected_name}=\"{expected_value}\"" - assert expected_value == actual_value, f"Attribute mismatch at {tree_path_joined}: " \ - f"found {actual_name}=\"{actual_value}\"" + assert expected_name == actual_name, (f"Attribute mismatch at {tree_path_joined}: " + f"expected {expected_name}=\"{expected_value}\"") + assert expected_value == actual_value, (f"Attribute mismatch at {tree_path_joined}: " + f"found {actual_name}=\"{actual_value}\"") # Hooray, attributes match. Dig deeper, how about children? # To compare children, use this very function to compare each child with @@ -94,9 +92,8 @@ def _valid_children(node: xml.dom.Node) -> list[xml.dom.Node]: actual_children = _valid_children(actual) assert len(expected_children) == len(actual_children), \ - f"Children count mismatch at {tree_path_joined}: " \ - f"expected {len(expected_children)}, " \ - f"found {len(actual_children)}" + (f"Children count mismatch at {tree_path_joined}: " + f"expected {len(expected_children)}, found {len(actual_children)}") return all( _compare_xml_node( diff --git a/tmt/base.py b/tmt/base.py index 87c352e270..1086d3f72c 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -940,9 +940,8 @@ def detect_unallowed_properties_with_pattern( return for bad_property in match.group(1).replace("'", '').replace(' ', '').split(','): - yield LinterOutcome.WARN, \ - f'key "{bad_property}" not recognized by schema,' \ - f' and does not match "{match.group(2)}" pattern' + yield LinterOutcome.WARN, (f'key "{bad_property}" not recognized by schema, ' + f'and does not match "{match.group(2)}" pattern') # A key value is not recognized. This is often a case with keys whose values are # limited by an enum, like `how`. Unfortunately, validator will record every mismatch @@ -1513,9 +1512,8 @@ def lint_require_type_field(self) -> LinterReturn: if not tmt.utils.is_key_origin(self.node, 'require') \ and all(dependency in metadata.get('require', []) for dependency in missing_type): - yield LinterOutcome.FAIL, \ - 'some library/file requirement are missing type, but inherited from test parent,' \ - ' please, fix manually' + yield LinterOutcome.FAIL, ('some library/file requirement are missing type, ' + 'but inherited from test parent, please, fix manually') return for dependency in metadata.get('require', []): @@ -2213,20 +2211,19 @@ def _lint_step(step: str) -> LinterReturn: continue if guest_names and guest_roles: - yield LinterOutcome.FAIL, \ - f"{step} phase '{phase.get('name')}' needs guest or role '{where}', " \ - f"guests {names_formatted} " \ - f"and roles {roles_formatted} were found" + yield (LinterOutcome.FAIL, + f"{step} phase '{phase.get('name')}' needs guest or role '{where}'," + f" guests {names_formatted} and roles {roles_formatted} were found") elif guest_names: - yield LinterOutcome.FAIL, \ - f"{step} phase '{phase.get('name')}' needs guest or role '{where}', " \ - f"guests {names_formatted} and no roles were found" + yield (LinterOutcome.FAIL, + f"{step} phase '{phase.get('name')}' needs guest or role " + f"'{where}', guests {names_formatted} and no roles were found") else: - yield LinterOutcome.FAIL, \ - f"{step} phase '{phase.get('name')}' needs guest or role '{where}', " \ - f"roles {roles_formatted} and no guests were found" + yield (LinterOutcome.FAIL, + f"{step} phase '{phase.get('name')}' needs guest or role " + f"'{where}', roles {roles_formatted} and no guests were found") yield from _lint_step('prepare') yield from _lint_step('execute') diff --git a/tmt/export/__init__.py b/tmt/export/__init__.py index e0bdf126c2..3820668018 100644 --- a/tmt/export/__init__.py +++ b/tmt/export/__init__.py @@ -407,16 +407,13 @@ def count_html_headings(heading: str) -> None: count_html_headings(sections_headings['Setup'][0]) count_html_headings(sections_headings['Cleanup'][0]) - warn_outside_test_section = 'Heading "{}" from the section "{}" is '\ - 'used \noutside of Test sections.' - warn_headings_not_in_pairs = 'The number of headings from the section' \ - ' "Step" - {}\ndoesn\'t equal to the ' \ - 'number of headings from the section \n' \ - '"Expect" - {} in the test section "{}"' - warn_required_section_is_absent = '"{}" section doesn\'t exist in ' \ - 'the Markdown file' - warn_unexpected_headings = 'Headings "{}" aren\'t expected in the ' \ - 'section "{}"' + warn_outside_test_section = \ + 'Heading "{}" from the section "{}" is used \noutside of Test sections.' + warn_headings_not_in_pairs = \ + ('The number of headings from the section "Step" - {}\ndoesn\'t equal to the ' + 'number of headings from the section \n"Expect" - {} in the test section "{}"') + warn_required_section_is_absent = '"{}" section doesn\'t exist in the Markdown file' + warn_unexpected_headings = 'Headings "{}" aren\'t expected in the section "{}"' def required_section_exists( section: list[str], diff --git a/tmt/export/nitrate.py b/tmt/export/nitrate.py index dd62cc2724..862c6e6334 100644 --- a/tmt/export/nitrate.py +++ b/tmt/export/nitrate.py @@ -264,8 +264,8 @@ def return_markdown_file() -> Optional[Path]: files = '\n'.join(os.listdir()) reg_exp = r'.+\.md$' md_files = re.findall(reg_exp, files, re.M) - fail_message = "in the current working directory.\n" \ - "Manual steps couldn't be exported" + fail_message = ("in the current working directory.\n" + "Manual steps couldn't be exported") if len(md_files) == 1: return Path.cwd() / str(md_files[0]) if not md_files: diff --git a/tmt/log.py b/tmt/log.py index b5d7849956..143bb6ee3e 100644 --- a/tmt/log.py +++ b/tmt/log.py @@ -485,15 +485,15 @@ def __init__( self._decolorize_output = create_decolorizer(apply_colors_output) def __repr__(self) -> str: - return '' + return (f'') @property def labels_span(self) -> int: diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index 70ff7c91a8..0933a12e7d 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -1389,9 +1389,8 @@ def delegate( assert data is not None assert data.__class__ is plugin_data_class, \ - f'Data package is instance of {data.__class__.__name__}, ' \ - f'plugin {plugin_class.__name__} ' \ - f'expects {plugin_data_class.__name__}' + (f'Data package is instance of {data.__class__.__name__}, ' + f'plugin {plugin_class.__name__} expects {plugin_data_class.__name__}') plugin = plugin_class( logger=step._logger.descend(logger_name=None), @@ -1551,9 +1550,8 @@ def wake(self) -> None: """ assert self.data.__class__ is self._data_class, \ - f'Plugin {self.__class__.__name__} woken with incompatible ' \ - f'data {self.data}, ' \ - f'expects {self._data_class.__name__}' + (f'Plugin {self.__class__.__name__} woken with incompatible ' + f'data {self.data}, expects {self._data_class.__name__}') if self.step.status() == 'done': self.debug('step is done, not overwriting plugin data') @@ -2167,8 +2165,7 @@ def phase_name(self) -> str: @property def name(self) -> str: - return f'{self.phase_name} ' \ - f'on {fmf.utils.listed(self.guest_ids)}' + return f'{self.phase_name} on {fmf.utils.listed(self.guest_ids)}' def run_on_guest(self, guest: 'Guest', logger: tmt.log.Logger) -> None: self.phase.go(guest=guest, logger=logger) From 3f24907b34f70db58aa40896c0dfd586859c2a37 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 10 Apr 2024 19:14:30 +0200 Subject: [PATCH 07/16] Add flake8-logging and logging-format ruff rules --- pyproject.toml | 8 +++++--- tests/unit/provision/testcloud/test_hw.py | 7 ++++--- tests/unit/test_logging.py | 6 +++--- tmt/checks/watchdog.py | 4 ++-- tmt/convert.py | 9 ++++----- tmt/hardware.py | 2 +- tmt/log.py | 4 ++-- tmt/plugins/__init__.py | 2 +- tmt/steps/__init__.py | 4 ++-- tmt/steps/execute/__init__.py | 2 +- tmt/steps/execute/internal.py | 5 +++-- tmt/steps/provision/__init__.py | 2 +- tmt/steps/provision/mrack.py | 2 +- tmt/steps/provision/testcloud.py | 8 ++++---- tmt/utils.py | 8 ++++---- 15 files changed, 38 insertions(+), 35 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 31b8e465b6..88c350ea36 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -301,6 +301,8 @@ lint.select = [ "T10", # flake8-debugger "EXE", # flake8-executable "ISC", # flake8-implicit-str-concat + "LOG", # flake8-logging + "G", # flake8-logging-format "PIE", # flake8-pie "RSE", # flake8-raise "PGH", # pygrep-hooks @@ -316,9 +318,7 @@ lint.select = [ lint.ignore = [ "B904", # Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... "COM812", # Trailing comma missing - # tmt codebase uses `warn` by default - disabling the check, switching to - # `warning` can be done in an extra patch. - "G010", # `warn` is deprecated in favor of `warning` + "G004", # Logging statement uses f-string "PIE790", # Unnecessary `pass` statement "PLC1901", # `{}` can be simplified to `{}` as an empty string is falsey "PLE1205", # Too many arguments for `logging` format string @@ -355,6 +355,8 @@ lint.ignore = [ "D415", # First line should end with a period, question mark, or exclamation point ] +lint.logger-objects = ["tmt.log.Logger"] + [tool.ruff.lint.per-file-ignores] # Less strict security checks in tests "tests/unit*" = [ diff --git a/tests/unit/provision/testcloud/test_hw.py b/tests/unit/provision/testcloud/test_hw.py index 4b9ce97c5e..84bcb269c9 100644 --- a/tests/unit/provision/testcloud/test_hw.py +++ b/tests/unit/provision/testcloud/test_hw.py @@ -154,8 +154,9 @@ def test_tpm_unsupported_version( assert_log( caplog, - message=MATCH(r"warn: Cannot apply hardware requirement 'tpm\.version: == 0\.0\.0', TPM version not supported."), # noqa: E501 - levelno=logging.WARN) + message=MATCH(r"warn: Cannot apply hardware requirement 'tpm\.version: == 0\.0\.0', " + r"TPM version not supported."), + levelno=logging.WARNING) @pytest.mark.parametrize( @@ -181,4 +182,4 @@ def test_tpm_unsupported_operator( assert_log( caplog, message=MATCH(rf"warn: Cannot apply hardware requirement 'tpm\.version: {op} 2\.0', operator not supported."), # noqa: E501 - levelno=logging.WARN) + levelno=logging.WARNING) diff --git a/tests/unit/test_logging.py b/tests/unit/test_logging.py index 193a927b1b..a7559dbbef 100644 --- a/tests/unit/test_logging.py +++ b/tests/unit/test_logging.py @@ -40,7 +40,7 @@ def _exercise_logger( logger.debug('this is a debug message') logger.verbose('this is a verbose message') logger.info('this is just an info') - logger.warn('this is a warning') + logger.warning('this is a warning') logger.fail('this is a failure') captured = capsys.readouterr() @@ -76,7 +76,7 @@ def _exercise_logger( details_key='warn', details_value='this is a warning', details_logger_labels=labels, - levelno=logging.WARN) + levelno=logging.WARNING) assert_log( caplog, message=f'{prefix}fail: this is a failure', @@ -97,7 +97,7 @@ def test_creation(caplog: _pytest.logging.LogCaptureFixture, root_logger: Logger logger = Logger.create() assert logger._logger.name == 'tmt' - actual_logger = logging.Logger('3rd-party-app-logger') + actual_logger = logging.Logger('3rd-party-app-logger') # noqa: LOG001 logger = Logger.create(actual_logger) assert logger._logger is actual_logger diff --git a/tmt/checks/watchdog.py b/tmt/checks/watchdog.py index de3d7172b1..136718c9f9 100644 --- a/tmt/checks/watchdog.py +++ b/tmt/checks/watchdog.py @@ -420,12 +420,12 @@ def before_test( guest_context: GuestContext = invocation.check_data[check.how] if check.ping and not isinstance(invocation.guest, PINGABLE_GUEST_CLASSES): - watchdog_logger.warn('Ping against this guest is not supported, disabling.') + watchdog_logger.warning('Ping against this guest is not supported, disabling.') check.ping = False if check.ssh_ping and not isinstance(invocation.guest, SSH_PINGABLE_GUEST_CLASSES): - watchdog_logger.warn('SSH ping against this guest is not supported, disabling.') + watchdog_logger.warning('SSH ping against this guest is not supported, disabling.') check.ssh_ping = False diff --git a/tmt/convert.py b/tmt/convert.py index 3a983f6346..0417f2d30c 100644 --- a/tmt/convert.py +++ b/tmt/convert.py @@ -86,11 +86,10 @@ def read_manual( for case_id in case_ids: testcase = nitrate.TestCase(case_id) if testcase.status.name != 'CONFIRMED' and not disabled: - log.debug( - testcase.identifier + ' skipped (testcase is not CONFIRMED).') + log.debug(f'{testcase.identifier} skipped (testcase is not CONFIRMED).') continue if testcase.script is not None and not with_script: - log.debug(testcase.identifier + ' skipped (script is not empty).') + log.debug(f'{testcase.identifier} skipped (script is not empty).') continue # Filename sanitization @@ -612,8 +611,8 @@ def target_content_build() -> list[str]: if parent.get(key) == test[key]: test.pop(key) - log.debug('Common metadata:\n' + format_value(common_data)) - log.debug('Individual metadata:\n' + format_value(individual_data)) + log.debug(f'Common metadata:\n{format_value(common_data)}') + log.debug(f'Individual metadata:\n{format_value(individual_data)}') return common_data, individual_data diff --git a/tmt/hardware.py b/tmt/hardware.py index 916fe73062..cd93cd22e8 100644 --- a/tmt/hardware.py +++ b/tmt/hardware.py @@ -1552,7 +1552,7 @@ def report_support( or check(constraint): continue - logger.warn( + logger.warning( f"Hardware requirement '{constraint.printable_name}' is not supported.") def format_variants(self) -> Iterator[str]: diff --git a/tmt/log.py b/tmt/log.py index 143bb6ee3e..6afba7ed44 100644 --- a/tmt/log.py +++ b/tmt/log.py @@ -797,13 +797,13 @@ def debug( message_topic=topic) ) - def warn( + def warning( self, message: str, shift: int = 0 ) -> None: self._log( - logging.WARN, + logging.WARNING, LogRecordDetails( key='warn', value=message, diff --git a/tmt/plugins/__init__.py b/tmt/plugins/__init__.py index 278d85e547..7c987e1718 100644 --- a/tmt/plugins/__init__.py +++ b/tmt/plugins/__init__.py @@ -342,7 +342,7 @@ def register_plugin( # TODO: would be raising an exception better? Probably, but since # plugin discovery happens in import time, it's very hard to manage # it. For now, report a warning, but do not raise an exception yet. - logger.warn( + logger.warning( f"Registering plugin '{plugin.__module__}' collides" f" with an already registered id '{plugin_id}'" f" of plugin '{self._plugins[plugin_id]}'.") diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index 0933a12e7d..bed1846c04 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -1077,7 +1077,7 @@ def prune(self, logger: tmt.log.Logger) -> None: else: shutil.rmtree(member) except OSError as error: - logger.warn(f"Unable to remove '{member}': {error}") + logger.warning(f"Unable to remove '{member}': {error}") class Method: @@ -1616,7 +1616,7 @@ def prune(self, logger: tmt.log.Logger) -> None: try: shutil.rmtree(self.workdir) except OSError as error: - logger.warn(f"Unable to remove '{self.workdir}': {error}") + logger.warning(f"Unable to remove '{self.workdir}': {error}") class GuestlessPlugin(BasePlugin[StepDataT]): diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 6c1c60872e..208e33b62e 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -382,7 +382,7 @@ def handle_reboot(self) -> bool: raise except tmt.utils.ProvisionError: - self.logger.warn( + self.logger.warning( "Guest does not support soft reboot, trying hard reboot.") rebooted = self.guest.reboot(hard=True, timeout=timeout) diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index a82f07fe25..e665ff6b30 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -393,7 +393,8 @@ def _save_process( if self.data.interactive: if test.duration: - logger.warn('Ignoring requested duration, not supported in interactive mode.') + logger.warning( + 'Ignoring requested duration, not supported in interactive mode.') timeout = None @@ -423,7 +424,7 @@ def _save_process( logger.debug(f"Test duration '{test.duration}' exceeded.") elif tmt.utils.ProcessExitCodes.is_pidfile(invocation.return_code): - logger.warn('Test failed to manage its pidfile.') + logger.warning('Test failed to manage its pidfile.') with invocation.process_lock: invocation.process = None diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index e724f113c3..5cd210466b 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -1480,7 +1480,7 @@ def _cleanup_ssh_master_process( self._ssh_master_process.wait(timeout=3) except subprocess.TimeoutExpired: - logger.warn( + logger.warning( f'Terminating the SSH master process {self._ssh_master_process.pid}' ' timed out.') diff --git a/tmt/steps/provision/mrack.py b/tmt/steps/provision/mrack.py index e11843f8d0..414604096b 100644 --- a/tmt/steps/provision/mrack.py +++ b/tmt/steps/provision/mrack.py @@ -204,7 +204,7 @@ def _transform_unsupported( # sure user is aware it would have no effect, and since we have to return # something, return an empty `or` group - no harm done, composable with other # elements. - logger.warn(f"Hardware requirement '{constraint.printable_name}' will have no effect.") + logger.warning(f"Hardware requirement '{constraint.printable_name}' will have no effect.") return MrackHWOrGroup() diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index c0d896f253..ff80b01471 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -412,12 +412,12 @@ def _apply_hw_tpm( for constraint in tpm_constraints: if constraint.operator not in TPM_VERSION_ALLOWED_OPERATORS: - logger.warn( + logger.warning( f"Cannot apply hardware requirement '{constraint}', operator not supported.") return if constraint.value not in TPM_VERSION_SUPPORTED_VERSIONS[TPM_CONFIG_ALLOWS_VERSIONS]: - logger.warn( + logger.warning( f"Cannot apply hardware requirement '{constraint}'," " TPM version not supported.") return @@ -1101,13 +1101,13 @@ def go(self) -> None: if data.memory is not None and data.hardware.constraint.uses_constraint( 'memory', self._logger): - self._logger.warn( + self._logger.warning( "Hardware requirement 'memory' is specified in 'hardware' key," " it will be overruled by 'memory' key.") if data.disk is not None and data.hardware.constraint.uses_constraint( 'disk.size', self._logger): - self._logger.warn( + self._logger.warning( "Hardware requirement 'disk.size' is specified in 'hardware' key," " it will be overruled by 'disk' key.") diff --git a/tmt/utils.py b/tmt/utils.py index 03365a1dde..5cb7c69883 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -532,7 +532,7 @@ def from_sequence( environment = cls.from_yaml_file(filepath, logger) if not environment: - logger.warn(f"Empty environment file '{filepath}'.") + logger.warning(f"Empty environment file '{filepath}'.") result.update(environment) @@ -627,7 +627,7 @@ def from_file( environment = cls.from_dotenv(content) if not environment: - logger.warn(f"Empty environment file '{filename}'.") + logger.warning(f"Empty environment file '{filename}'.") return Environment() @@ -1949,7 +1949,7 @@ def debug( def warn(self, message: str, shift: int = 0) -> None: """ Show a yellow warning message on info level, send to stderr """ - self._logger.warn(message, shift=shift) + self._logger.warning(message, shift=shift) def fail(self, message: str, shift: int = 0) -> None: """ Show a red failure message on info level, send to stderr """ @@ -5947,7 +5947,7 @@ def _validate_fmf_node( validation_errors=errors) for _, error_message in errors: - logger.warn(error_message, shift=1) + logger.warning(error_message, shift=1) def __init__( self, From 14c272307f99650c7cab1077e9778da70032f80e Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 10 Apr 2024 19:25:23 +0200 Subject: [PATCH 08/16] Add flake8-pyi ruff rules --- pyproject.toml | 1 + tmt/steps/__init__.py | 2 +- tmt/utils.py | 8 ++++---- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 88c350ea36..bb2708a2b3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -304,6 +304,7 @@ lint.select = [ "LOG", # flake8-logging "G", # flake8-logging-format "PIE", # flake8-pie + "PYI", # flake8-pyi "RSE", # flake8-raise "PGH", # pygrep-hooks "PLC", # pylint-convention diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index bed1846c04..82d4c4c836 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -1016,7 +1016,7 @@ class (single class or tuple of classes). """ if classes is None: - _classes: tuple[Union[type[Phase], type[PhaseT]], ...] = (Phase,) + _classes: tuple[type[Union[Phase, PhaseT]], ...] = (Phase,) elif not isinstance(classes, tuple): _classes = (classes,) diff --git a/tmt/utils.py b/tmt/utils.py index 5cb7c69883..d9347cec90 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -3920,7 +3920,7 @@ def format_value( def format( key: str, - value: Union[None, int, float, bool, str, list[Any], dict[Any, Any]] = None, + value: Union[None, float, bool, str, list[Any], dict[Any, Any]] = None, indent: int = 24, window_size: int = OUTPUT_WIDTH, wrap: FormatWrap = 'auto', @@ -4493,7 +4493,7 @@ def __enter__(self) -> requests.Session: status_forcelist=self.status_forcelist, timeout=self.timeout) - def __exit__(self, *args: Any) -> None: + def __exit__(self, *args: object) -> None: pass @@ -5462,7 +5462,7 @@ def __init__( def __enter__(self: 'Self') -> 'Self': return self - def __exit__(self, *args: Any) -> None: + def __exit__(self, *args: object) -> None: if self.clear_on_exit: self.clear() @@ -7141,7 +7141,7 @@ def __enter__(self) -> 'Stopwatch': return self - def __exit__(self, *args: Any) -> None: + def __exit__(self, *args: object) -> None: self.end_time = datetime.datetime.now(datetime.timezone.utc) @property From 41ee28689c9adc291ad6dabc7d8fee17c57d6d6f Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 11 Apr 2024 14:39:39 +0200 Subject: [PATCH 09/16] Add ICN, TID, INT, Q ruff rules and clean-up Remove some of the unnecessary ignores. Sort ruff rules in pyproject.toml to follow Ruff documentation. Enable more Pylint rules. --- pyproject.toml | 29 +++++++++++--------- tests/integration/test_nitrate.py | 3 +-- tests/integration/test_polarion.py | 2 +- tests/unit/provision/mrack/test_hw.py | 3 +-- tests/unit/provision/testcloud/test_hw.py | 3 +-- tests/unit/test_base.py | 3 +-- tests/unit/test_cli.py | 3 +-- tests/unit/test_id.py | 3 +-- tests/unit/test_report_junit.py | 8 +++--- tmt/base.py | 33 +++++++++++------------ tmt/cli.py | 4 +-- tmt/convert.py | 17 ++++++------ tmt/log.py | 9 +++---- tmt/steps/__init__.py | 2 -- tmt/steps/discover/__init__.py | 1 - tmt/steps/prepare/install.py | 4 --- tmt/steps/report/html.py | 1 - tmt/steps/report/junit.py | 1 - tmt/steps/report/polarion.py | 1 - tmt/utils.py | 9 +++---- 20 files changed, 62 insertions(+), 77 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index bb2708a2b3..8a7bc3e2d4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -282,37 +282,40 @@ src = ["tmt", "tests"] target-version = "py39" lint.select = [ "F", # pyflakes - "E", # pycodestyle - "W", # pycodestyle + "E", # pycodestyle error + "W", # pycodestyle warning "I", # isort "N", # pep8-naming "D", # pydocstyle "UP", # pyupgrade - "B", # flake8-bugbear - "A", # flake8-builtins - "C4", # flake8-comprehensions "YTT", # flake8-2020 + "ASYNC", # flake8-async "S", # flake8-bandit - "PT", # flake8-pytest-style - "RET", # flake8-return - "SIM", # flake8-simplify + "B", # flake8-bugbear + "A", # flake8-builtins "COM", # flake8-commas + "C4", # flake8-comprehensions "DTZ", # flake8-datetimez "T10", # flake8-debugger "EXE", # flake8-executable "ISC", # flake8-implicit-str-concat + "ICN", # flake8-import-conventions "LOG", # flake8-logging "G", # flake8-logging-format "PIE", # flake8-pie "PYI", # flake8-pyi + "PT", # flake8-pytest-style + "Q003", # avoidable-escaped-quote + "Q004", # unnecessary-escaped-quote "RSE", # flake8-raise + "RET", # flake8-return + "SIM", # flake8-simplify + "TID", # flake8-tidy-imports + "INT", # flake8-gettext "PGH", # pygrep-hooks "PLC", # pylint-convention "PLE", # pylint-error - "PLR01", # pylint-refactor - "PLR02", - "PLR04", - "PLR1", + "PLR", # pylint-refactor "RUF", # ruff "D", # pydocstyle ] @@ -323,6 +326,8 @@ lint.ignore = [ "PIE790", # Unnecessary `pass` statement "PLC1901", # `{}` can be simplified to `{}` as an empty string is falsey "PLE1205", # Too many arguments for `logging` format string + "PLR09", # Too many branches/statements/arguments/returns + "PLR2004", # Magic value used in comparison "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar` "RUF013", # PEP 484 prohibits implicit `Optional` # flake8-bandit (S) https://docs.astral.sh/ruff/rules/#flake8-bandit-s diff --git a/tests/integration/test_nitrate.py b/tests/integration/test_nitrate.py index a29caa5511..ae6c3379e6 100644 --- a/tests/integration/test_nitrate.py +++ b/tests/integration/test_nitrate.py @@ -11,10 +11,9 @@ import tmt.base import tmt.cli import tmt.log +from tests import CliRunner from tmt.utils import ConvertError, Path -from .. import CliRunner - # Prepare path to examples TEST_DIR = Path(__file__).parent diff --git a/tests/integration/test_polarion.py b/tests/integration/test_polarion.py index 3c412f0bb6..e5cfa780c2 100644 --- a/tests/integration/test_polarion.py +++ b/tests/integration/test_polarion.py @@ -4,9 +4,9 @@ from fmf import Tree import tmt.cli +from tests import CliRunner from tmt.identifier import ID_KEY -from .. import CliRunner from .test_nitrate import TEST_DIR, Base diff --git a/tests/unit/provision/mrack/test_hw.py b/tests/unit/provision/mrack/test_hw.py index 27ae4c2375..d4966fceb0 100644 --- a/tests/unit/provision/mrack/test_hw.py +++ b/tests/unit/provision/mrack/test_hw.py @@ -3,6 +3,7 @@ import pytest import tmt.utils +from tests.unit.test_hardware import FULL_HARDWARE_REQUIREMENTS from tmt.hardware import ( Hardware, Operator, @@ -20,8 +21,6 @@ operator_to_beaker_op, ) -from ...test_hardware import FULL_HARDWARE_REQUIREMENTS - @pytest.mark.parametrize( ('operator', 'value', 'expected'), diff --git a/tests/unit/provision/testcloud/test_hw.py b/tests/unit/provision/testcloud/test_hw.py index 84bcb269c9..1f2234b8f7 100644 --- a/tests/unit/provision/testcloud/test_hw.py +++ b/tests/unit/provision/testcloud/test_hw.py @@ -6,6 +6,7 @@ import pytest from testcloud.domain_configuration import DomainConfiguration, TPMConfiguration +from tests.unit import MATCH, assert_log from tmt.hardware import TPM_VERSION_ALLOWED_OPERATORS, Hardware, Operator from tmt.log import Logger from tmt.steps.provision.testcloud import ( @@ -16,8 +17,6 @@ import_testcloud, ) -from ... import MATCH, assert_log - import_testcloud() # These must be imported *after* importing testcloud diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index 59a53d270e..80fbe84000 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -8,11 +8,10 @@ import tmt import tmt.cli +from tests import CliRunner from tmt.base import FmfId, Link, LinkNeedle, Links, expand_node_data from tmt.utils import Path, SpecificationError -from .. import CliRunner - runner = CliRunner() diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index fcd4f1b9ed..e9dcdb357e 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -9,8 +9,7 @@ import tmt.cli import tmt.log - -from .. import CliRunner +from tests import CliRunner # Prepare path to examples PATH = os.path.dirname(os.path.realpath(__file__)) diff --git a/tests/unit/test_id.py b/tests/unit/test_id.py index 5d4c5f7ff1..681b210350 100644 --- a/tests/unit/test_id.py +++ b/tests/unit/test_id.py @@ -8,11 +8,10 @@ import tmt import tmt.cli import tmt.log +from tests import CliRunner from tmt.identifier import ID_KEY from tmt.utils import Path -from .. import CliRunner - runner = CliRunner() test_path = Path(__file__).parent / "id" root_logger = tmt.log.Logger.create() diff --git a/tests/unit/test_report_junit.py b/tests/unit/test_report_junit.py index ae8b792bd9..ef092e74fe 100644 --- a/tests/unit/test_report_junit.py +++ b/tests/unit/test_report_junit.py @@ -69,10 +69,10 @@ def _compare_xml_node(tree_path: list[str], expected: xml.dom.Node, actual: xml. for (expected_name, expected_value), (actual_name, actual_value) in zip( expected_attributes, actual_attributes): - assert expected_name == actual_name, (f"Attribute mismatch at {tree_path_joined}: " - f"expected {expected_name}=\"{expected_value}\"") - assert expected_value == actual_value, (f"Attribute mismatch at {tree_path_joined}: " - f"found {actual_name}=\"{actual_value}\"") + assert expected_name == actual_name, (f'Attribute mismatch at {tree_path_joined}: ' + f'expected {expected_name}="{expected_value}"') + assert expected_value == actual_value, (f'Attribute mismatch at {tree_path_joined}: ' + f'found {actual_name}="{actual_value}"') # Hooray, attributes match. Dig deeper, how about children? # To compare children, use this very function to compare each child with diff --git a/tmt/base.py b/tmt/base.py index 1086d3f72c..8c10bfe0bd 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -2918,24 +2918,12 @@ def name_filter(nodes: Iterable[fmf.Tree]) -> list[fmf.Tree]: Test(node=test, logger=self._logger.descend()) for test in self.tree.prune( keys=keys, sources=cmd_line_names)] - else: + elif not unique and names: # First let's build the list of test objects based on keys & names. # If duplicate test names are allowed, match test name/regexp # one-by-one and preserve the order of tests within a plan. - if not unique and names: - tests = [] - for name in names: - selected_tests = [ - Test( - node=test, - tree=self, - logger=logger.descend( - logger_name=test.get('name', None) - ) # .apply_verbosity_options(**self._options), - ) for test in name_filter(self.tree.prune(keys=keys, names=[name]))] - tests.extend(sorted(selected_tests, key=lambda test: test.order)) - # Otherwise just perform a regular key/name filtering - else: + tests = [] + for name in names: selected_tests = [ Test( node=test, @@ -2943,8 +2931,19 @@ def name_filter(nodes: Iterable[fmf.Tree]) -> list[fmf.Tree]: logger=logger.descend( logger_name=test.get('name', None) ) # .apply_verbosity_options(**self._options), - ) for test in name_filter(self.tree.prune(keys=keys, names=names))] - tests = sorted(selected_tests, key=lambda test: test.order) + ) for test in name_filter(self.tree.prune(keys=keys, names=[name]))] + tests.extend(sorted(selected_tests, key=lambda test: test.order)) + # Otherwise just perform a regular key/name filtering + else: + selected_tests = [ + Test( + node=test, + tree=self, + logger=logger.descend( + logger_name=test.get('name', None) + ) # .apply_verbosity_options(**self._options), + ) for test in name_filter(self.tree.prune(keys=keys, names=names))] + tests = sorted(selected_tests, key=lambda test: test.order) # Apply filters & conditions return self._filters_conditions( diff --git a/tmt/cli.py b/tmt/cli.py index c4a6d8c5fa..61b71ec9c2 100644 --- a/tmt/cli.py +++ b/tmt/cli.py @@ -2034,7 +2034,7 @@ def setup_completion(shell: str, install: bool) -> None: if install: with open(script, 'w') as f: - subprocess.run(command, stdout=f, shell=True, check=False) + subprocess.run(command, stdout=f, check=False) # If requested, modify .bashrc or .zshrc if shell != 'fish': config_path = Path(f'~/.{shell}rc').expanduser() @@ -2043,7 +2043,7 @@ def setup_completion(shell: str, install: bool) -> None: shell_config.write(f'source {script}') else: - subprocess.run(command, stdout=sys.stdout, shell=True, check=False) + subprocess.run(command, stdout=sys.stdout, check=False) @completion.command(name='bash') diff --git a/tmt/convert.py b/tmt/convert.py index 0417f2d30c..c13b61f319 100644 --- a/tmt/convert.py +++ b/tmt/convert.py @@ -1006,16 +1006,15 @@ def read_nitrate_case( # Full 'Name Surname ' form if testcase.tester.name is not None: data['contact'] = f'{testcase.tester.name} <{testcase.tester.email}>' + elif makefile_data is None or 'contact' not in makefile_data: + # Otherwise use just the email address + data['contact'] = testcase.tester.email + # Use contact from Makefile if it's there and email matches + elif re.search(testcase.tester.email, makefile_data['contact']): + data['contact'] = makefile_data['contact'] else: - if makefile_data is None or 'contact' not in makefile_data: - # Otherwise use just the email address - data['contact'] = testcase.tester.email - # Use contact from Makefile if it's there and email matches - elif re.search(testcase.tester.email, makefile_data['contact']): - data['contact'] = makefile_data['contact'] - else: - # Otherwise use just the email address - data['contact'] = testcase.tester.email + # Otherwise use just the email address + data['contact'] = testcase.tester.email echo(style('contact: ', fg='green') + data['contact']) # Environment if testcase.arguments: diff --git a/tmt/log.py b/tmt/log.py index 6afba7ed44..cd94aa5a8e 100644 --- a/tmt/log.py +++ b/tmt/log.py @@ -305,12 +305,11 @@ def format(self, record: logging.LogRecord) -> str: pass # Otherwise render the message. - else: - if record.msg and record.args: - record.message = record.msg % record.args + elif record.msg and record.args: + record.message = record.msg % record.args - else: - record.message = record.msg + else: + record.message = record.msg # Original code from Formatter.format() - hard to inherit when overriding # Formatter.format()... diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index 82d4c4c836..f038dc53ea 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -279,8 +279,6 @@ def pre_normalization(cls, raw_data: _RawStepData, logger: tmt.log.Logger) -> No def post_normalization(self, raw_data: _RawStepData, logger: tmt.log.Logger) -> None: """ Called after normalization, useful for tweaking normalized data """ - pass - # ignore[override]: expected, we need to accept one extra parameter, `logger`. @classmethod def from_spec( # type: ignore[override] diff --git a/tmt/steps/discover/__init__.py b/tmt/steps/discover/__init__.py index c70e300b7f..1b09bb6d00 100644 --- a/tmt/steps/discover/__init__.py +++ b/tmt/steps/discover/__init__.py @@ -154,7 +154,6 @@ def log_import_plan_details(self) -> None: def post_dist_git(self, created_content: list[Path]) -> None: """ Discover tests after dist-git applied patches """ - pass class Discover(tmt.steps.Step): diff --git a/tmt/steps/prepare/install.py b/tmt/steps/prepare/install.py index d5e4eece07..3c02d75072 100644 --- a/tmt/steps/prepare/install.py +++ b/tmt/steps/prepare/install.py @@ -152,19 +152,15 @@ def prepare_install_local(self) -> None: def install_from_repository(self) -> None: """ Default base install method for packages from repositories """ - pass def install_local(self) -> None: """ Default base install method for local packages """ - pass def install_from_url(self) -> None: """ Default base install method for packages which are from URL """ - pass def install_debuginfo(self) -> None: """ Default base install method for debuginfo packages """ - pass def install(self) -> None: """ Perform the actual installation """ diff --git a/tmt/steps/report/html.py b/tmt/steps/report/html.py index dcb138c86d..7f44637940 100644 --- a/tmt/steps/report/html.py +++ b/tmt/steps/report/html.py @@ -57,7 +57,6 @@ class ReportHtml(tmt.steps.report.ReportPlugin[ReportHtmlData]): def prune(self, logger: tmt.log.Logger) -> None: """ Do not prune generated html report """ - pass def go(self) -> None: """ Process results """ diff --git a/tmt/steps/report/junit.py b/tmt/steps/report/junit.py index d302d79827..fa4b38387b 100644 --- a/tmt/steps/report/junit.py +++ b/tmt/steps/report/junit.py @@ -111,7 +111,6 @@ class ReportJUnit(tmt.steps.report.ReportPlugin[ReportJUnitData]): def prune(self, logger: tmt.log.Logger) -> None: """ Do not prune generated junit report """ - pass def go(self) -> None: """ Read executed tests and write junit """ diff --git a/tmt/steps/report/polarion.py b/tmt/steps/report/polarion.py index 1990af9924..d98adeaa57 100644 --- a/tmt/steps/report/polarion.py +++ b/tmt/steps/report/polarion.py @@ -189,7 +189,6 @@ class ReportPolarion(tmt.steps.report.ReportPlugin[ReportPolarionData]): def prune(self, logger: tmt.log.Logger) -> None: """ Do not prune generated xunit report """ - pass def go(self) -> None: """ Go through executed tests and report into Polarion """ diff --git a/tmt/utils.py b/tmt/utils.py index d9347cec90..1b5849d32c 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -3657,12 +3657,11 @@ def _format_str( if is_multiline: yield '' - else: - if not value.rstrip(): - yield '' + elif not value.rstrip(): + yield '' - else: - yield from value.rstrip().split('\n') + else: + yield from value.rstrip().split('\n') def _format_dict( From 926ea3b2bb6a65ac962b02a642fed7046c43efc3 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 19 Jun 2024 11:18:00 +0200 Subject: [PATCH 10/16] Removing shell=True from subprocess run --- pyproject.toml | 1 - tmt/cli.py | 29 ++++++++++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8a7bc3e2d4..64c4cd8dac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -332,7 +332,6 @@ lint.ignore = [ "RUF013", # PEP 484 prohibits implicit `Optional` # flake8-bandit (S) https://docs.astral.sh/ruff/rules/#flake8-bandit-s "S101", # Use of `assert` detected - "S602", # `subprocess` call with `shell=True` identified "S603", # `subprocess` call: check for execution of untrusted input "S607", # Starting a process with a partial executable path "S105", # Possible hardcoded password assigned to: "PASS" diff --git a/tmt/cli.py b/tmt/cli.py index 61b71ec9c2..885cb4eedf 100644 --- a/tmt/cli.py +++ b/tmt/cli.py @@ -5,8 +5,6 @@ import dataclasses import enum import re -import subprocess -import sys from collections.abc import Sequence from typing import TYPE_CHECKING, Any, Callable, Optional, TypeVar, Union @@ -29,7 +27,7 @@ import tmt.trying import tmt.utils from tmt.options import Deprecated, create_options_decorator, option -from tmt.utils import Path, cached_property +from tmt.utils import Command, Path, cached_property if TYPE_CHECKING: from typing_extensions import Concatenate, ParamSpec @@ -2019,7 +2017,7 @@ def completion(**kwargs: Any) -> None: COMPLETE_SCRIPT = 'tmt-complete' -def setup_completion(shell: str, install: bool) -> None: +def setup_completion(shell: str, install: bool, context: Context) -> None: """ Setup completion based on the shell """ config = tmt.utils.Config() # Fish gets installed into its special location where it is automatically @@ -2030,11 +2028,20 @@ def setup_completion(shell: str, install: bool) -> None: else: script = Path(config.path) / f'{COMPLETE_SCRIPT}.{shell}' - command = f'{COMPLETE_VARIABLE}={shell}_source tmt' + env_var = {COMPLETE_VARIABLE: f'{shell}_source'} + + logger = context.obj.logger + + completions = Command('tmt').run(env=tmt.utils.Environment.from_dict(env_var), + cwd=None, + logger=context.obj.logger + ).stdout + if not completions: + logger.warning("Unable to generate shell completion") + return if install: - with open(script, 'w') as f: - subprocess.run(command, stdout=f, check=False) + Path(script).write_text(completions) # If requested, modify .bashrc or .zshrc if shell != 'fish': config_path = Path(f'~/.{shell}rc').expanduser() @@ -2043,7 +2050,7 @@ def setup_completion(shell: str, install: bool) -> None: shell_config.write(f'source {script}') else: - subprocess.run(command, stdout=sys.stdout, check=False) + logger.info(completions) @completion.command(name='bash') @@ -2056,7 +2063,7 @@ def setup_completion(shell: str, install: bool) -> None: """) def completion_bash(context: Context, install: bool, **kwargs: Any) -> None: """ Setup shell completions for bash """ - setup_completion('bash', install) + setup_completion('bash', install, context) @completion.command(name='zsh') @@ -2069,7 +2076,7 @@ def completion_bash(context: Context, install: bool, **kwargs: Any) -> None: """) def completion_zsh(context: Context, install: bool, **kwargs: Any) -> None: """ Setup shell completions for zsh """ - setup_completion('zsh', install) + setup_completion('zsh', install, context) @completion.command(name='fish') @@ -2079,4 +2086,4 @@ def completion_zsh(context: Context, install: bool, **kwargs: Any) -> None: help="Persistently store the script to '~/.config/fish/completions/tmt.fish'.") def completion_fish(context: Context, install: bool, **kwargs: Any) -> None: """ Setup shell completions for fish """ - setup_completion('fish', install) + setup_completion('fish', install, context) From aea52a4298396dce1d83163e0b90c6e64e15bd94 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Fri, 21 Jun 2024 15:05:01 +0200 Subject: [PATCH 11/16] Add backwards compatibility for Logger.warn --- tmt/log.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tmt/log.py b/tmt/log.py index cd94aa5a8e..75b7d54840 100644 --- a/tmt/log.py +++ b/tmt/log.py @@ -31,6 +31,7 @@ import logging.handlers import os import sys +import warnings from typing import ( TYPE_CHECKING, Any, @@ -810,6 +811,14 @@ def warning( shift=shift) ) + def warn( + self, + message: str, + shift: int + ) -> None: + warnings.warn("Use `warning` instead", DeprecationWarning, stacklevel=1) + return self.warning(message, shift) + def fail( self, message: str, From 6010d6a5aa17b54f91d0f480889b6fd55d89cb0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kluso=C5=88?= Date: Wed, 26 Jun 2024 14:22:26 +0200 Subject: [PATCH 12/16] Use ansible playbook from worktree (#2989) During `tmt run` execution there are two copies of the playbook available: - One in the original repository with tmt plan - Second in the `worktree` under /var/tmp/tmt... `_ansible_playbook_path` uses the second one instead the first one now. Resolves: https://github.com/teemtee/tmt/issues/2935 --- tmt/steps/provision/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 5cd210466b..ccbca9f608 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -956,11 +956,9 @@ def _ansible_playbook_path(self, playbook: Path) -> Path: self.debug(f"Applying playbook '{playbook}' on guest '{self.primary_address}'.") # FIXME: cast() - https://github.com/teemtee/tmt/issues/1372 parent = cast(Provision, self.parent) - assert parent.plan.my_run is not None # narrow type - assert parent.plan.my_run.tree is not None # narrow type - assert parent.plan.my_run.tree.root is not None # narrow type + assert parent.plan.fmf_root is not None # narrow type # Playbook paths should be relative to the metadata tree root - playbook = parent.plan.my_run.tree.root / playbook.unrooted() + playbook = parent.plan.fmf_root / playbook.unrooted() self.debug(f"Playbook full path: '{playbook}'", level=2) return playbook From cd0b6e3aaf8f909ccd40b5c06ca18116bd6d5727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Wed, 26 Jun 2024 14:50:25 +0200 Subject: [PATCH 13/16] Add favicon to tmt docs (#3021) --- .gitignore | 1 + docs/Makefile | 12 ++++++++++-- docs/conf.py | 11 +++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 8c0a25c1ce..8a7bd72dce 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ docs/plugins/test-checks.rst docs/_build docs/spec docs/stories +docs/_static/tmt-small.png # Python diff --git a/docs/Makefile b/docs/Makefile index 58c91383e5..2f18033e38 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -4,8 +4,11 @@ .DEFAULT_GOAL := help .PHONY: help generate-plugins plugins/*.rst generate-stories generate-template-filters generate-autodocs clean +LOGO_SRC = https://raw.githubusercontent.com/teemtee/docs/main/logo/tmt-small.png +LOGO_DST = _static/tmt-small.png + clean: - rm -rf _build stories spec code/autodocs/*.rst code/template-filters.rst + rm -rf _build stories spec code/autodocs/*.rst code/template-filters.rst $(LOGO_DST) find plugins -name "*.rst" ! -name index.rst | xargs rm -f ## @@ -18,7 +21,12 @@ TEMPLATESDIR = templates PLUGINS_TEMPLATE := $(TEMPLATESDIR)/plugins.rst.j2 -generate: spec stories generate-lint-checks generate-template-filters generate-plugins generate-stories generate-autodocs ## Refresh all generated documentation sources +generate: $(LOGO_DST) spec stories generate-lint-checks generate-template-filters generate-plugins generate-stories generate-autodocs ## Refresh all generated documentation sources + +# We can ignore the error: later, during the build, if the logo is +# missing, Sphinx will complain. +$(LOGO_DST): + -curl -f $(LOGO_SRC) -o $(LOGO_DST) spec: mkdir -p spec diff --git a/docs/conf.py b/docs/conf.py index a106153062..39e0b65fc0 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -16,7 +16,7 @@ import subprocess import sys from pathlib import Path -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Any, Optional if TYPE_CHECKING: from sphinx.application import Sphinx @@ -222,7 +222,7 @@ def _load_theme( # The name of an image file (within the static path) to use as favicon of the # docs. This file should be a Windows icon file (.ico) being 16x16 or 32x32 # pixels large. -# html_favicon = None +html_favicon = '_static/tmt-small.png' # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, @@ -313,7 +313,7 @@ def _load_theme( ] -def generate_tmt_docs(app: Sphinx) -> None: +def generate_tmt_docs(app: Sphinx, config: Any) -> None: """ Run `make generate` to populate the auto-generated sources """ conf_dir = Path(app.confdir) @@ -321,4 +321,7 @@ def generate_tmt_docs(app: Sphinx) -> None: def setup(app: Sphinx) -> None: - app.connect("builder-inited", generate_tmt_docs) + # Generate sources after loading configuration. That should build + # everything, including the logo, before Sphinx starts checking + # whether all input files exist. + app.connect("config-inited", generate_tmt_docs) From 9717fc412d27256a10e7eef5319cc279ec3c4e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Wed, 26 Jun 2024 20:33:58 +0200 Subject: [PATCH 14/16] Add missing test case for parsing `device` HW requirement (#3012) --- tests/unit/provision/mrack/test_hw.py | 9 +++++++++ tests/unit/test_hardware.py | 12 ++++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/unit/provision/mrack/test_hw.py b/tests/unit/provision/mrack/test_hw.py index d4966fceb0..ce6e985397 100644 --- a/tests/unit/provision/mrack/test_hw.py +++ b/tests/unit/provision/mrack/test_hw.py @@ -144,6 +144,15 @@ def test_maximal_constraint(root_logger: Logger) -> None: {'or': []} ] }, + { + 'and': [ + {'or': []}, + {'or': []}, + {'or': []}, + {'or': []}, + {'or': []} + ] + }, { 'system': { 'memory': { diff --git a/tests/unit/test_hardware.py b/tests/unit/test_hardware.py index f1a254ecf3..98a544a237 100644 --- a/tests/unit/test_hardware.py +++ b/tests/unit/test_hardware.py @@ -169,6 +169,12 @@ def test_normalize_invalid_hardware( - avx - "= avx2" - "!= smep" + device: + device-name: '~ .*Thunderbolt.*' + device: 79 + vendor-name: '!= Intel' + vendor: "> 97" + driver: mc disk: - size: 40 GiB model-name: "~ WD 100G.*" @@ -235,6 +241,12 @@ def test_parse_maximal_constraint() -> None: - cpu.flag: contains avx - cpu.flag: contains avx2 - cpu.flag: not contains smep + - and: + - device.vendor: '> 97' + - device.device: == 79 + - device.vendor-name: '!= Intel' + - device.device-name: ~ .*Thunderbolt.* + - device.driver: == mc - and: - gpu.vendor: == 4318 - gpu.device: == 97 From 1bc2cb9b20e9907dca6b2ab205328226251efec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Fri, 28 Jun 2024 15:08:39 +0200 Subject: [PATCH 15/16] Remove prepare/install fields leaking into prepare step data (#2986) * Remove prepare/install fields leaking into prepare step data --- tmt/steps/__init__.py | 7 +++++++ tmt/steps/execute/upgrade.py | 5 +++-- tmt/steps/prepare/__init__.py | 15 +++++---------- tmt/steps/prepare/distgit.py | 7 ++++--- tmt/steps/prepare/install.py | 8 ++++++++ 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index f038dc53ea..1c2a607020 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -222,6 +222,9 @@ class _RawStepData(TypedDict, total=False): how: Optional[str] name: Optional[str] + summary: Optional[str] + order: Optional[int] + RawStepDataArgument = Union[_RawStepData, list[_RawStepData]] @@ -302,6 +305,10 @@ def from_spec( # type: ignore[override] return data +class RawWhereableStepData(TypedDict, total=False): + where: Union[str, list[str]] + + @dataclasses.dataclass class WhereableStepData: """ diff --git a/tmt/steps/execute/upgrade.py b/tmt/steps/execute/upgrade.py index ad011ea989..801ad11957 100644 --- a/tmt/steps/execute/upgrade.py +++ b/tmt/steps/execute/upgrade.py @@ -15,7 +15,8 @@ from tmt.steps.discover.fmf import DiscoverFmf, DiscoverFmfStepData, normalize_ref from tmt.steps.execute import ExecutePlugin from tmt.steps.execute.internal import ExecuteInternal, ExecuteInternalData -from tmt.steps.prepare import PreparePlugin, _RawPrepareStepData +from tmt.steps.prepare import PreparePlugin +from tmt.steps.prepare.install import _RawPrepareInstallStepData from tmt.utils import Environment, EnvVarValue, Path, field STATUS_VARIABLE = 'IN_PLACE_UPGRADE' @@ -266,7 +267,7 @@ def _install_dependencies( recommends: bool = False) -> None: """ Install packages required/recommended for upgrade """ phase_name = 'recommended' if recommends else 'required' - data: _RawPrepareStepData = { + data: _RawPrepareInstallStepData = { 'how': 'install', 'name': f'{phase_name}-packages-upgrade', 'summary': f'Install packages {phase_name} by the upgrade', diff --git a/tmt/steps/prepare/__init__.py b/tmt/steps/prepare/__init__.py index ebf7605dc2..919f00cdce 100644 --- a/tmt/steps/prepare/__init__.py +++ b/tmt/steps/prepare/__init__.py @@ -1,4 +1,3 @@ -import collections import copy import dataclasses from typing import ( @@ -48,14 +47,8 @@ class PrepareStepData(tmt.steps.WhereableStepData, tmt.steps.StepData): PrepareStepDataT = TypeVar('PrepareStepDataT', bound=PrepareStepData) -class _RawPrepareStepData(tmt.steps._RawStepData, total=False): - where: Optional[list[str]] - package: list[str] - missing: str - roles: collections.defaultdict[str, list[str]] - hosts: dict[str, str] - order: int - summary: str +class _RawPrepareStepData(tmt.steps._RawStepData, tmt.steps.RawWhereableStepData, total=False): + pass class PreparePlugin(tmt.steps.Plugin[PrepareStepDataT]): @@ -277,6 +270,8 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']: # # 1. make the list of requirements unique, # 2. group guests with same requirements. + from tmt.steps.prepare.install import _RawPrepareInstallStepData + pruned_requires: dict[frozenset[tmt.base.DependencySimple], DependencyCollection] = {} pruned_recommends: dict[frozenset[tmt.base.DependencySimple], DependencyCollection] = {} @@ -302,7 +297,7 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']: if not collection.dependencies: continue - data: _RawPrepareStepData = { + data: _RawPrepareInstallStepData = { 'how': 'install', 'name': 'requires', 'summary': 'Install required packages', diff --git a/tmt/steps/prepare/distgit.py b/tmt/steps/prepare/distgit.py index 358177f68c..1be8dbade4 100644 --- a/tmt/steps/prepare/distgit.py +++ b/tmt/steps/prepare/distgit.py @@ -8,7 +8,8 @@ import tmt.steps.prepare import tmt.utils from tmt.package_managers import Package -from tmt.steps.prepare import PreparePlugin, _RawPrepareStepData +from tmt.steps.prepare import PreparePlugin +from tmt.steps.prepare.install import _RawPrepareInstallStepData from tmt.steps.provision import Guest from tmt.utils import Command, Path, ShellScript, field, uniq @@ -53,7 +54,7 @@ def insert_to_prepare_step( prepare_step = discover_plugin.step.plan.prepare where = cast(tmt.steps.discover.DiscoverStepData, discover_plugin.data).where # Future install require - data_require: _RawPrepareStepData = { + data_require: _RawPrepareInstallStepData = { 'how': 'install', 'name': 'requires (dist-git)', 'summary': 'Install required packages of tests detected by dist-git', @@ -66,7 +67,7 @@ def insert_to_prepare_step( prepare_step._phases.append(future_requires) # Future install recommend - data_recommend: _RawPrepareStepData = { + data_recommend: _RawPrepareInstallStepData = { 'how': 'install', 'name': 'recommends (dist-git)', 'summary': 'Install recommended packages of tests detected by dist-git', diff --git a/tmt/steps/prepare/install.py b/tmt/steps/prepare/install.py index 3c02d75072..b59fecd437 100644 --- a/tmt/steps/prepare/install.py +++ b/tmt/steps/prepare/install.py @@ -525,6 +525,14 @@ def install_debuginfo(self) -> None: 'installing debuginfo packages.') +class _RawPrepareInstallStepData(tmt.steps.prepare._RawPrepareStepData, total=False): + package: Union[str, list[str]] + directory: Union[str, list[str]] + copr: Union[str, list[str]] + exclude: Union[str, list[str]] + missing: str + + @dataclasses.dataclass class PrepareInstallData(tmt.steps.prepare.PrepareStepData): package: list[tmt.base.DependencySimple] = field( From 76b5ff2cf67661c8d603e3f7f12eaf4ea3742f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Mon, 1 Jul 2024 09:14:06 +0200 Subject: [PATCH 16/16] Add a linter for Jinja2 templates (#3048) Maybe we would add also a formatter (same tool, different check and options), but starting small with linters. Few very minor issues reported, plus a genuine one. --- .pre-commit-config.yaml | 19 +++++++++++++++++++ docs/templates/lint-checks.rst.j2 | 2 +- docs/templates/plugins.rst.j2 | 4 ++-- docs/templates/story.rst.j2 | 2 +- docs/templates/template-filters.rst.j2 | 2 +- pyproject.toml | 4 ++++ tmt/steps/report/html/template.html.j2 | 7 ++++--- 7 files changed, 32 insertions(+), 8 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a8f45c4f0e..8389b21883 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -161,3 +161,22 @@ repos: - id: codespell additional_dependencies: - tomli # Required for python < 3.11 + + - repo: https://github.com/djlint/djLint + rev: v1.34.1 + hooks: + - id: djlint + files: "\\.j2" + types_or: ['jinja'] + + - repo: https://github.com/aristanetworks/j2lint.git + rev: v1.1.0 + hooks: + - id: j2lint + args: + # j2lint does not consume pyproject.toml + - "--ignore" + - jinja-statements-indentation + - jinja-variable-lower-case + - single-statement-per-line + - "--" diff --git a/docs/templates/lint-checks.rst.j2 b/docs/templates/lint-checks.rst.j2 index 52c5d614be..214ccb4f42 100644 --- a/docs/templates/lint-checks.rst.j2 +++ b/docs/templates/lint-checks.rst.j2 @@ -34,7 +34,7 @@ Below you can find the list of available checks. See the - {{ linter.help }} {% endfor %} -{% endmacro%} +{% endmacro %} {{ emit_table('Test checks', TEST_LINTERS) }} {{ emit_table('Plan checks', PLAN_LINTERS) }} diff --git a/docs/templates/plugins.rst.j2 b/docs/templates/plugins.rst.j2 index 7f25a6ebc1..db01eef1b0 100644 --- a/docs/templates/plugins.rst.j2 +++ b/docs/templates/plugins.rst.j2 @@ -10,7 +10,7 @@ .. _plugins/{{ STEP }}/{{ PLUGIN_ID | trim }}: {{ PLUGIN_ID }} -{{ '-' * (PLUGIN_ID | length)}} +{{ '-' * (PLUGIN_ID | length) }} {# Emit the warning only for plugins that have not been reviewed yet. #} {% set plugin_full_id = STEP + "/" + PLUGIN_ID %} @@ -65,7 +65,7 @@ Configuration Default: *could not render default value correctly* {% endif %} {% endif %} -{%endmacro %} +{% endmacro %} {% set ignored_fields = container_ignored_fields(PLUGIN_DATA_CLASS) %} {% set inherited_fields = container_inherited_fields(PLUGIN_DATA_CLASS) | sort %} diff --git a/docs/templates/story.rst.j2 b/docs/templates/story.rst.j2 index 4e01aeedd4..59f3568956 100644 --- a/docs/templates/story.rst.j2 +++ b/docs/templates/story.rst.j2 @@ -219,7 +219,7 @@ {# Links pointing to websites #} {% elif link.target | match('^https?://') %} -* {{ printable_relation(link) }} `{{ link.target}} <{{ link.target }}>`_ +* {{ printable_relation(link) }} `{{ link.target }} <{{ link.target }}>`_ {# Links pointing to anything else #} {% else %} diff --git a/docs/templates/template-filters.rst.j2 b/docs/templates/template-filters.rst.j2 index ee0e2756bb..f47e1aa53d 100644 --- a/docs/templates/template-filters.rst.j2 +++ b/docs/templates/template-filters.rst.j2 @@ -25,7 +25,7 @@ __ https://jinja.palletsprojects.com/en/3.1.x/templates/#filters {% set filter_callable = TEMPLATES[filter_name] %} {{ filter_name }} -{{ '-' * (filter_name | length)}} +{{ '-' * (filter_name | length) }} {% if filter_callable.__doc__ %} {{ filter_callable.__doc__ | dedent | trim }} diff --git a/pyproject.toml b/pyproject.toml index 64c4cd8dac..fd5ade6893 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -400,3 +400,7 @@ markers = [ ignore-words = "docs/codespell.dic" exclude-file = "docs/codespell.ignore" skip = "tests/execute/weird/data/weird.txt,tests/lint/plan/data/invalid_attr.fmf,tests/lint/plan/test.sh" + +[tool.djlint] +use_gitignore=true +ignore="H005,H030,H031" diff --git a/tmt/steps/report/html/template.html.j2 b/tmt/steps/report/html/template.html.j2 index 88c118466b..7153110c41 100644 --- a/tmt/steps/report/html/template.html.j2 +++ b/tmt/steps/report/html/template.html.j2 @@ -1,4 +1,5 @@ + Test results of {{ plan.name }}