diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5dabea7c51..9b47a1e272 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,11 +6,6 @@ repos: hooks: - id: autopep8 - - repo: https://github.com/PyCQA/isort - rev: "5.12.0" - hooks: - - id: isort - - repo: https://github.com/pre-commit/pre-commit-hooks rev: "v4.4.0" hooks: @@ -19,7 +14,7 @@ repos: - id: trailing-whitespace - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.2.0" + rev: "v1.3.0" hooks: - id: mypy # Do not install *types-click* - it's not recommended with Click 8 & newer, @@ -38,25 +33,25 @@ repos: args: [--config-file=pyproject.toml] - repo: https://github.com/python-jsonschema/check-jsonschema - rev: "0.22.0" + rev: "0.23.1" hooks: - id: check-metaschema name: "Check JSON schemas validity" files: ^tmt/schemas/.*\.yaml - repo: https://github.com/adrienverge/yamllint - rev: v1.30.0 + rev: v1.32.0 hooks: - id: yamllint files: ^tmt/schemas/.*\.yaml - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: v0.0.261 + rev: v0.0.274 hooks: - id: ruff args: [--fix] - repo: https://github.com/teemtee/tmt.git - rev: 1.23.0 + rev: 1.24.1 hooks: - id: tmt-lint diff --git a/bin/tmt b/bin/tmt index efe9c7c6ee..e960a6c27e 100755 --- a/bin/tmt +++ b/bin/tmt @@ -7,9 +7,9 @@ try: # Import utils first, before CLI gets a chance to spawn a logger. Without # tmt.utils, we would not be able to intercept the exception below. - import tmt.utils + import tmt.utils # noqa: I001 - import tmt.cli # isort: skip + import tmt.cli tmt.cli.main() diff --git a/docs/conf.py b/docs/conf.py index adb016198b..cc87ea9e9c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # # documentation build configuration file, created by # sphinx-quickstart on Mon Apr 27 17:44:03 2015. @@ -72,9 +71,9 @@ master_man = 'man.1' # General information about the project. -project = u'tmt' -copyright = u'Red Hat' -author = u'Petr Šplíchal' +project = 'tmt' +copyright = 'Red Hat' +author = 'Petr Šplíchal' # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the @@ -241,7 +240,7 @@ # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). man_pages = [ - (master_man, '', u'tmt Documentation', + (master_man, '', 'tmt Documentation', [author], 1) ] @@ -284,10 +283,10 @@ def __getattr__(cls, name: str) -> 'Mock': os.makedirs('stories', exist_ok=True) os.makedirs('spec', exist_ok=True) for area in areas: - with open('{}.rst'.format(area.lstrip('/')), 'w') as doc: + with open(f"{area.lstrip('/')}.rst", 'w') as doc: # Anchor and title doc.write(f'.. _{area}:\n\n') - doc.write('{}\n{}\n'.format(areas[area], '=' * len(areas[area]))) + doc.write(f"{areas[area]}\n{'=' * len(areas[area])}\n") # Included stories for story in tree.stories(names=[area], whole=True): if story.enabled: diff --git a/examples/plugins/provision.py b/examples/plugins/provision.py index c5ca9c60ea..6faa6e015f 100644 --- a/examples/plugins/provision.py +++ b/examples/plugins/provision.py @@ -34,8 +34,8 @@ def options(cls, how=None): help="Example how to pass value."), option( '-s', '--switch', is_flag=True, - help="Example how to enable something.") - ] + super().options(how) + help="Example how to enable something."), + *super().options(how)] def default(self, option, default=None): """ Return the default value for the given option """ @@ -73,7 +73,7 @@ def go(self): print("go() called") # Data dictionary is used to pass information among classes. - data = dict(what='Another default what. Object variable can be used.') + data = {'what': 'Another default what. Object variable can be used.'} for opt in ['what', 'switch']: val = self.get(opt) @@ -206,8 +206,7 @@ def execute(self, command, **kwargs): print("execute() called. This is an optional overload...") - output = ["Fedora", "whatever"] - return output + return ["Fedora", "whatever"] def delete(self): """ Remove the example instance """ diff --git a/pyproject.toml b/pyproject.toml index 90080e0e29..9d6879e74c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,11 +32,6 @@ module = [ ] ignore_missing_imports = true -# code is currently formatted with default isort config -#[tool.isort] -#line_length = 99 -#py_version = 37 - [tool.autopep8] max_line_length = 99 in-place = true @@ -56,8 +51,8 @@ select = [ "E", # pycodestyle "W", # pycodestyle # "C90", # mccabe -# "I", # isort -# "N", # pep8-naming + "I", # isort + "N", # pep8-naming # "UP", # pyupgrade "B", # flake8-bugbear "C4", # flake8-comprehensions @@ -65,51 +60,49 @@ select = [ # "ANN", # flake8-annotations # "S", # flake8-bandit # "ISC", # flake8-implicit-str-concat -# "PT", # flake8-pytest-style -# "RET", # flake8-return -# "SIM", # flake8-simplify + "PT", # flake8-pytest-style + "RET", # flake8-return + "SIM", # flake8-simplify # "ARG", # flake8-unused-arguments # "BLE", # flake8-blind-except # "FBT", # flake8-boolean-trap # "A", # flake8-builtins "COM", # flake8-commas -# "C4", # flake8-comprehensions "DTZ", # flake8-datetimez "T10", # flake8-debugger # "EM", # flake8-errmsg "EXE", # flake8-executable -# "ISC", # flake8-implicit-str-concat # "G", # flake8-logging-format -# "PIE", # flake8-pie + "PIE", # flake8-pie # "T20", # flake8-print -# "PT", # flake8-pytest-style # "Q", # flake8-quotes -# "RSE", # flake8-raise -# "RET", # flake8-return + "RSE", # flake8-raise # "SLF", # flake8-self -# "SIM", # flake8-simplify # "TCH", # flake8-type-checking "PGH", # pygrep-hooks -# "PLC", # pylint-convention -# "PLE", # pylint-error -# "PLR", # pylint-refactor + "PLC", # pylint-convention + "PLE", # pylint-error + "PLR01", # pylint-refactor + "PLR02", + "PLR04", + "PLR1", # "PLW", # pylint-warning # "TRY", # tryceratops "RUF", # ruff ] ignore = [ - "C408", # Unnecessary `dict` call (rewrite as a literal) - "C416", # Unnecessary `list` comprehension (rewrite using `list()`) - "C401", # Unnecessary generator (rewrite as a `set` comprehension) - "C405", # Unnecessary `list` literal (rewrite as a `set` literal) "B904", # Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... - "B00", # B00{1..9} - "B011", # Do not `assert False` (`python -O` removes these calls), raise `AssertionError()` - "B012", # return` inside `finally` blocks cause exceptions to be silenced - "B018", # Found useless expression. Either assign it to a variable or remove it. "COM812", # Trailing comma missing - "RUF005", # collection-literal-concatenation - Consider {expr} instead of concatenation + "PIE790", # Unnecessary `pass` statement + "PLC1901", # `{}` can be simplified to `{}` as an empty string is falsey + "PLE1205", # Too many arguments for `logging` format string + "RUF008", # Do not use mutable default values for dataclass attributes + "RUF009", # Do not perform function call `field` in dataclass defaults + "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar` + "RUF013", # PEP 484 prohibits implicit `Optional` ] +[tool.ruff.isort] +known-first-party = ["tmt"] [tool.ruff.per-file-ignores] "tmt/steps/report/polarion.py" = ["DTZ005"] diff --git a/setup.py b/setup.py index c6f4be58da..4c9e12615c 100755 --- a/setup.py +++ b/setup.py @@ -1,8 +1,6 @@ #!/usr/bin/env python -# coding: utf-8 import re -from io import open from typing import Dict, List from setuptools import setup @@ -109,7 +107,7 @@ readme = _file.read() github = 'https://github.com/teemtee/tmt' -download_url = '{0}/archive/main.zip'.format(github) +download_url = f'{github}/archive/main.zip' setup( url=github, diff --git a/tests/execute/reboot/get_value.py b/tests/execute/reboot/get_value.py index 3185ea2f3c..22a9df5b22 100755 --- a/tests/execute/reboot/get_value.py +++ b/tests/execute/reboot/get_value.py @@ -26,7 +26,7 @@ def main(key, yaml_file): def find_value(data, key): if not isinstance(data, dict): - raise ValueError() + raise ValueError try: return data[key] except KeyError: @@ -35,7 +35,7 @@ def find_value(data, key): return find_value(value, key) except ValueError: pass - return + return None if __name__ == "__main__": diff --git a/tests/integration/test_nitrate.py b/tests/integration/test_nitrate.py index e6e9566504..4ed86be38e 100644 --- a/tests/integration/test_nitrate.py +++ b/tests/integration/test_nitrate.py @@ -47,7 +47,7 @@ class NitrateExport(Base): def test_create(self): fmf_node = Tree(self.tmpdir).find("/new_testcase") - self.assertNotIn("extra-nitrate", fmf_node.data) + assert "extra-nitrate" not in fmf_node.data os.chdir(self.tmpdir / "new_testcase") runner = CliRunner() @@ -58,11 +58,11 @@ def test_create(self): catch_exceptions=False) # Reload the node data to see if it appears there fmf_node = Tree(self.tmpdir).find("/new_testcase") - self.assertIn("extra-nitrate", fmf_node.data) + assert "extra-nitrate" in fmf_node.data def test_create_dryrun(self): fmf_node_before = Tree(self.tmpdir).find("/new_testcase") - self.assertNotIn("extra-nitrate", fmf_node_before.data) + assert "extra-nitrate" not in fmf_node_before.data os.chdir(self.tmpdir / "new_testcase") runner = CliRunner() @@ -72,15 +72,13 @@ def test_create_dryrun(self): "--create", "--dry", "--general", "--append-summary", "."], catch_exceptions=False) fmf_node = Tree(self.tmpdir).find("/new_testcase") - self.assertNotIn("extra-nitrate", fmf_node.data) - self.assertEqual(fmf_node_before.data, fmf_node.data) - self.assertIn( - "summary: tmt /new_testcase - This i", - self.runner_output.output) + assert "extra-nitrate" not in fmf_node.data + assert fmf_node_before.data == fmf_node.data + assert "summary: tmt /new_testcase - This i" in self.runner_output.output def test_existing(self): fmf_node = Tree(self.tmpdir).find("/existing_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_testcase") runner = CliRunner() @@ -93,7 +91,7 @@ def test_existing(self): def test_existing_dryrun(self): fmf_node = Tree(self.tmpdir).find("/existing_dryrun_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_dryrun_testcase") runner = CliRunner() @@ -102,13 +100,11 @@ def test_existing_dryrun(self): ["test", "export", "--how", "nitrate", "--ignore-git-validation", "--debug", "--dry", "--general", "--bugzilla", "--append-summary", "."], catch_exceptions=False) - self.assertIn( - "summary: tmt /existing_dryrun_testcase - ABCDEF", - self.runner_output.output) + assert "summary: tmt /existing_dryrun_testcase - ABCDEF" in self.runner_output.output def test_existing_release_dryrun(self): fmf_node = Tree(self.tmpdir).find("/existing_dryrun_release_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_dryrun_release_testcase") runner = CliRunner() @@ -126,22 +122,15 @@ def test_existing_release_dryrun(self): "--append-summary", "."], catch_exceptions=False) - self.assertIn( - "summary: tmt /existing_dryrun_release_testcase - ABCDEF", - self.runner_output.output) - self.assertIn( - "Linked to general plan 'TP#28164 - tmt / General'", - self.runner_output.output) - self.assertIn( - "Link to plan 'TP#31698", - self.runner_output.output) - self.assertIn( - "Link to run 'TR#425023", - self.runner_output.output) + assert "summary: tmt /existing_dryrun_release_testcase - ABCDEF" in \ + self.runner_output.output + assert "Linked to general plan 'TP#28164 - tmt / General'" in self.runner_output.output + assert "Link to plan 'TP#31698" in self.runner_output.output + assert "Link to run 'TR#425023" in self.runner_output.output def test_coverage_bugzilla(self): fmf_node = Tree(self.tmpdir).find("/existing_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_testcase") runner = CliRunner() @@ -178,7 +167,7 @@ def test_export_blocked_by_validation(self): tmt.cli.main, ["test", "export", "--nitrate", "--debug", "--dry", "--append-summary", "."], catch_exceptions=False) - self.assertIn('Uncommitted changes', str(error.exception)) + assert "Uncommitted changes" in str(error.exception) def test_export_forced_validation(self): os.chdir(self.tmpdir / "validation") @@ -194,9 +183,7 @@ def test_export_forced_validation(self): "--append-summary", "."], catch_exceptions=False) - self.assertIn( - "Exporting regardless 'Uncommitted changes", - self.runner_output.output) + assert "Exporting regardless 'Uncommitted changes" in self.runner_output.output class NitrateImport(Base): @@ -209,16 +196,10 @@ def test_import_manual_confirmed(self): ['-vvvvdddd', '--root', self.tmpdir / "import_case", "test", "import", "--no-general", "--nitrate", "--manual", "--case=609704"], catch_exceptions=False) - self.assertEqual(self.runner_output.exit_code, 0) - self.assertIn( - "Importing the 'Imported_Test_Case'", - self.runner_output.output) - self.assertIn( - "test case found 'TC#0609704'", - self.runner_output.output) - self.assertIn( - "Metadata successfully stored into", - self.runner_output.output) + assert self.runner_output.exit_code == 0 + assert "Importing the 'Imported_Test_Case'" in self.runner_output.output + assert "test case found 'TC#0609704'" in self.runner_output.output + assert "Metadata successfully stored into" in self.runner_output.output filename = next( filter( lambda x: "Metadata successfully stored into" in x @@ -227,13 +208,13 @@ def test_import_manual_confirmed(self): # /home/jscotka/git/tmt/Manual/Imported_Test_Case/main.fmf # TODO: not possible to specify, where to store data, # it always creates Manual subdir, I do not want it. - self.assertIn("/Manual/Imported_Test_Case/main.fmf", filename) - self.assertTrue(Path(filename).exists()) + assert "/Manual/Imported_Test_Case/main.fmf" in filename + assert Path(filename).exists() with open(Path(filename)) as file: yaml = YAML(typ='safe') out = yaml.load(file) - self.assertIn("Tier1", out["tag"]) - self.assertIn("tmt_test_component", out["component"]) + assert "Tier1" in out["tag"] + assert "tmt_test_component" in out["component"] def test_import_manual_proposed(self): runner = CliRunner() @@ -241,14 +222,14 @@ def test_import_manual_proposed(self): tmt.cli.main, ['--root', self.tmpdir / "import_case", "test", "import", "--no-general", "--nitrate", "--manual", "--case=609705"], catch_exceptions=False) - self.assertEqual(self.runner_output.exit_code, 0) + assert self.runner_output.exit_code == 0 # TODO: This is strange, expect at least some output in # case there is proper case, just case is not CONFIRMED # I can imagine also e.g. at least raise error but not pass, # with no output - self.assertEqual(self.runner_output.output.strip(), "") + assert self.runner_output.output.strip() == "" fmf_node = Tree(self.tmpdir).find("/import_case") - self.assertEqual(fmf_node, None) + assert fmf_node is None class NitrateImportAutomated(Base): @@ -310,35 +291,35 @@ class NitrateImportAutomated(Base): def test_basic(self): os.chdir(self.tmpdir / "import_case_automated") files = os.listdir() - self.assertIn("Makefile", files) - self.assertNotIn("main.fmf", files) - self.assertNotIn("test.md", files) + assert "Makefile" in files + assert "main.fmf" not in files + assert "test.md" not in files runner = CliRunner() self.runner_output = runner.invoke( tmt.cli.main, [ "test", "import", "--nitrate"], catch_exceptions=False) - self.assertEqual(self.runner_output.exit_code, 0) + assert self.runner_output.exit_code == 0 files = os.listdir() - self.assertIn("Makefile", files) - self.assertIn("test.md", files) + assert "Makefile" in files + assert "test.md" in files with open("test.md") as file: - self.assertIn(self.test_md_content, file.read()) - self.assertIn("main.fmf", files) + assert self.test_md_content in file.read() + assert "main.fmf" in files with open("main.fmf") as file: yaml = YAML(typ='safe') generated = yaml.load(file) referenced = yaml.load(self.main_fmf_content) - self.assertEqual(generated, referenced) + assert generated == referenced def test_old_relevancy(self): os.chdir(self.tmpdir / "import_old_relevancy") files = os.listdir() - self.assertEquals(files, ['Makefile']) + assert files == ["Makefile"] runner = CliRunner() self.runner_output = runner.invoke( tmt.cli.main, [ "test", "import", "--nitrate", "--no-general"], catch_exceptions=False) - self.assertEqual(self.runner_output.exit_code, 0) + assert self.runner_output.exit_code == 0 tree_f36_intel = tmt.Tree( logger=tmt.log.Logger.create(), @@ -348,11 +329,11 @@ def test_old_relevancy(self): 'arch': ['x86_64']}) found_tests = tree_f36_intel.tests(names=['/import_old_relevancy']) - self.assertEquals(len(found_tests), 1) + assert len(found_tests) == 1 test = found_tests[0] - self.assertTrue(test.enabled) - self.assertEquals(test.environment, {'ARCH': 'not arch'}) - self.assertEquals(test.node.get('extra-nitrate'), 'TC#0545993') + assert test.enabled + assert test.environment == {"ARCH": "not arch"} + assert test.node.get("extra-nitrate") == "TC#0545993" tree_f35_intel = tmt.Tree( logger=tmt.log.Logger.create(), @@ -362,9 +343,9 @@ def test_old_relevancy(self): 'arch': ['x86_64']}) found_tests = tree_f35_intel.tests(names=['/import_old_relevancy']) - self.assertEquals(len(found_tests), 1) + assert len(found_tests) == 1 test = found_tests[0] - self.assertFalse(test.enabled) + assert not test.enabled # second rule is ignored if the order is correctly transferred - self.assertEquals(test.environment, {}) - self.assertEquals(test.node.get('extra-nitrate'), 'TC#0545993') + assert test.environment == {} + assert test.node.get("extra-nitrate") == "TC#0545993" diff --git a/tests/integration/test_polarion.py b/tests/integration/test_polarion.py index 7b66fc5d9a..d24641c642 100644 --- a/tests/integration/test_polarion.py +++ b/tests/integration/test_polarion.py @@ -16,7 +16,7 @@ class PolarionExport(Base): def test_create(self): fmf_node = Tree(self.tmpdir).find("/new_testcase") - self.assertNotIn(ID_KEY, fmf_node.data) + assert ID_KEY not in fmf_node.data os.chdir(self.tmpdir / "new_testcase") runner = CliRunner() @@ -25,11 +25,11 @@ def test_create(self): "RHIVOS", "--create", "."]) # Reload the node data to see if it appears there fmf_node = Tree(self.tmpdir).find("/new_testcase") - self.assertIn(ID_KEY, fmf_node.data) + assert ID_KEY in fmf_node.data def test_create_dryrun(self): fmf_node_before = Tree(self.tmpdir).find("/new_testcase") - self.assertNotIn(ID_KEY, fmf_node_before.data) + assert ID_KEY not in fmf_node_before.data os.chdir(self.tmpdir / "new_testcase") runner = CliRunner() @@ -38,15 +38,13 @@ def test_create_dryrun(self): ["test", "export", "--how", "polarion", "--create", "--dry", "."], catch_exceptions=False) fmf_node = Tree(self.tmpdir).find("/new_testcase") - self.assertNotIn(ID_KEY, fmf_node.data) - self.assertEqual(fmf_node_before.data, fmf_node.data) - self.assertIn( - "title: tmt /new_testcase - This i", - self.runner_output.output) + assert ID_KEY not in fmf_node.data + assert fmf_node_before.data == fmf_node.data + assert "title: tmt /new_testcase - This i" in self.runner_output.output def test_existing(self): fmf_node = Tree(self.tmpdir).find("/existing_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_testcase") runner = CliRunner() @@ -55,11 +53,11 @@ def test_existing(self): "RHIVOS", "--create", "."]) fmf_node = Tree(self.tmpdir).find("/existing_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" def test_existing_dryrun(self): fmf_node = Tree(self.tmpdir).find("/existing_dryrun_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_dryrun_testcase") runner = CliRunner() @@ -68,13 +66,11 @@ def test_existing_dryrun(self): ["test", "export", "--how", "polarion", "--debug", "--dry", "--bugzilla", "."], catch_exceptions=False) - self.assertIn( - "title: tmt /existing_dryrun_testcase - ABCDEF", - self.runner_output.output) + assert "title: tmt /existing_dryrun_testcase - ABCDEF" in self.runner_output.output def test_coverage_bugzilla(self): fmf_node = Tree(self.tmpdir).find("/existing_testcase") - self.assertEqual(fmf_node.data["extra-nitrate"], "TC#0609686") + assert fmf_node.data["extra-nitrate"] == "TC#0609686" os.chdir(self.tmpdir / "existing_testcase") runner = CliRunner() diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py index 54a9eb907a..853bbb1f4a 100644 --- a/tests/unit/__init__.py +++ b/tests/unit/__init__.py @@ -4,6 +4,7 @@ from typing import Any, Callable, Iterable, List, Tuple import _pytest.logging +import pytest class PatternMatching: @@ -117,10 +118,8 @@ def field_getter(record, name): return getattr(record, name) # Given a logging record, apply all field/operator/value triplets, and make sure all match the # actual record properties. def _cmp(record: logging.LogRecord) -> bool: - return all([ - op(expected_value, field_getter(record, field_name)) - for field_getter, field_name, op, expected_value in operators - ]) + return all(op(expected_value, field_getter(record, field_name)) + for field_getter, field_name, op, expected_value in operators) # Final step: apply our "make sure field/operator/value triplets match given record" to each # and every record, and reduce per-record results into a single answer. By default, `any` is @@ -141,11 +140,11 @@ def _report(message: str) -> None: f'expected=>>>{expected_value}<<<', f'comparison={op(expected_value, field_getter(record, field_name))}') - assert False, f""" + pytest.fail(f""" {message}: {chr(10).join(formatted_fields)} -""" +""") # Cannot find log record with these properties diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index accf50a363..aed95ea712 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -22,7 +22,7 @@ def source_dir(tmpdir_factory): test_path = source_location / f'tests/bz{num}' test_path.mkdir() (test_path / 'runtests.sh').touch() - yield source_location + return source_location @pytest.fixture() diff --git a/tests/unit/test_adjust.py b/tests/unit/test_adjust.py index dfc43ebf05..e75e26da97 100644 --- a/tests/unit/test_adjust.py +++ b/tests/unit/test_adjust.py @@ -4,13 +4,13 @@ from tmt.utils import ConvertError -@pytest.fixture +@pytest.fixture() def mini(): """ Minimal example """ return relevancy_to_adjust("distro = fedora: False") -@pytest.fixture +@pytest.fixture() def full(): """ Full example """ return relevancy_to_adjust(""" @@ -37,7 +37,7 @@ def check(condition, expected): def test_empty(): """ Empty relevancy """ - assert relevancy_to_adjust('') == list() + assert relevancy_to_adjust('') == [] def test_comments(full): diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index d3393ff772..6a198d6961 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -1,5 +1,3 @@ -# coding: utf-8 - import os import shutil import tempfile @@ -33,17 +31,17 @@ def test_invalid_yaml_syntax(): def test_test_defaults(root_logger): """ Test default test attributes """ - test = tmt.Test.from_dict(logger=root_logger, mapping=dict(test='./test.sh'), name='/smoke') + test = tmt.Test.from_dict(logger=root_logger, mapping={'test': './test.sh'}, name='/smoke') assert test.name == '/smoke' - assert test.component == list() + assert test.component == [] assert str(test.test) == './test.sh' assert test.path == Path('/') - assert test.require == list() - assert test.environment == dict() + assert test.require == [] + assert test.environment == {} assert test.duration == '5m' assert test.enabled is True assert test.result == 'respect' - assert test.tag == list() + assert test.tag == [] def test_test_invalid(root_logger): @@ -100,20 +98,20 @@ def test_link(): assert Links(data=['one', 'two']).get() == [ Link(relation='relates', target='one'), Link(relation='relates', target='two')] # Multiple string mixed relation - assert Links(data=['implicit', dict(duplicates='explicit')]).get() == [ + assert Links(data=['implicit', {'duplicates': 'explicit'}]).get() == [ Link(relation='relates', target='implicit'), Link(relation='duplicates', target='explicit')] # Multiple strings (explicit relation) - assert Links(data=[dict(parent='mom'), dict(child='son')]).get() == [ + assert Links(data=[{'parent': 'mom'}, {'child': 'son'}]).get() == [ Link(relation='parent', target='mom'), Link(relation='child', target='son')] # Single dictionary (default relation) - assert Links(data=dict(name='foo')).get() == [ + assert Links(data={'name': 'foo'}).get() == [ Link(relation='relates', target=FmfId(name='foo'))] # Single dictionary (explicit relation) - assert Links(data=dict(verifies='foo')).get() == [Link(relation='verifies', target='foo')] + assert Links(data={'verifies': 'foo'}).get() == [Link(relation='verifies', target='foo')] # Multiple dictionaries - family = [dict(parent='mom', note='foo'), dict(child='son')] + family = [{'parent': 'mom', 'note': 'foo'}, {'child': 'son'}] assert Links(data=family).get() == [ Link(relation='parent', target='mom', note='foo'), Link(relation='child', target='son') ] @@ -145,12 +143,12 @@ def test_link(): " 'int' found."): Links(data=123) with pytest.raises(SpecificationError, match='Multiple relations'): - Links(data=dict(verifies='one', blocks='another')) + Links(data={'verifies': 'one', 'blocks': 'another'}) with pytest.raises(SpecificationError, match='Invalid link relation'): - Links(data=dict(depends='other')) + Links(data={'depends': 'other'}) # Searching for links - links = Links(data=[dict(parent='mom', note='foo'), dict(child='son', note='bar')]) + links = Links(data=[{'parent': 'mom', 'note': 'foo'}, {'child': 'son', 'note': 'bar'}]) assert links.has_link() assert links.has_link(needle=LinkNeedle()) assert links.has_link(needle=LinkNeedle(relation=r'.*', target=r'.*')) diff --git a/tests/unit/test_beakerlib.py b/tests/unit/test_beakerlib.py index b5e862bc72..723a09c973 100644 --- a/tests/unit/test_beakerlib.py +++ b/tests/unit/test_beakerlib.py @@ -8,7 +8,7 @@ from tmt.utils import Path -@pytest.mark.web +@pytest.mark.web() def test_basic(root_logger): """ Fetch a beakerlib library with/without providing a parent """ parent = tmt.utils.Common(logger=root_logger, workdir=True) @@ -30,9 +30,9 @@ def test_basic(root_logger): shutil.rmtree(library.parent.workdir) -@pytest.mark.web +@pytest.mark.web() @pytest.mark.parametrize( - 'url, name, default_branch', [ + ('url', 'name', 'default_branch'), [ ('https://github.com/beakerlib/httpd', '/http', 'master'), ('https://github.com/beakerlib/example', '/file', 'main') ]) @@ -53,7 +53,7 @@ def test_require_from_fmf(url, name, default_branch, root_logger): shutil.rmtree(library.parent.workdir) -@pytest.mark.web +@pytest.mark.web() def test_invalid_url_conflict(root_logger): """ Saner check if url mismatched for translated library """ parent = tmt.utils.Common(logger=root_logger, workdir=True) @@ -74,7 +74,7 @@ def test_invalid_url_conflict(root_logger): shutil.rmtree(parent.workdir) -@pytest.mark.web +@pytest.mark.web() def test_dependencies(root_logger): """ Check requires for possible libraries """ parent = tmt.utils.Common(logger=root_logger, workdir=True) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index ceb05f3eaf..897f69a660 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -1,5 +1,3 @@ -# coding: utf-8 - import dataclasses import os import shutil @@ -231,8 +229,8 @@ class DecideColorizationTestcase: @pytest.mark.parametrize( - ('testcase',), - [(testcase,) for testcase in _DECIDE_COLORIZATION_TESTCASES], + 'testcase', + list(_DECIDE_COLORIZATION_TESTCASES), ids=[testcase.name for testcase in _DECIDE_COLORIZATION_TESTCASES] ) def test_decide_colorization( @@ -243,8 +241,8 @@ def test_decide_colorization( monkeypatch.delenv('TMT_NO_COLOR', raising=False) monkeypatch.delenv('TMT_FORCE_COLOR', raising=False) - no_color = True if testcase.set_no_color_option else False - force_color = True if testcase.set_force_color_option else False + no_color = bool(testcase.set_no_color_option) + force_color = bool(testcase.set_force_color_option) if testcase.set_no_color_envvar: monkeypatch.setenv('NO_COLOR', '') diff --git a/tests/unit/test_convert.py b/tests/unit/test_convert.py index 46952fef54..d39d36bdf6 100644 --- a/tests/unit/test_convert.py +++ b/tests/unit/test_convert.py @@ -1,4 +1,3 @@ -# coding: utf-8 from textwrap import dedent import pytest diff --git a/tests/unit/test_dataclasses.py b/tests/unit/test_dataclasses.py index ee0b6ff792..c19814b066 100644 --- a/tests/unit/test_dataclasses.py +++ b/tests/unit/test_dataclasses.py @@ -6,9 +6,13 @@ import tmt.log import tmt.utils -from tmt.utils import (SerializableContainer, dataclass_field_by_name, - dataclass_field_metadata, dataclass_normalize_field, - field) +from tmt.utils import ( + SerializableContainer, + dataclass_field_by_name, + dataclass_field_metadata, + dataclass_normalize_field, + field, + ) def test_sanity(): diff --git a/tests/unit/test_export_to_nitrate.py b/tests/unit/test_export_to_nitrate.py index 661bc31830..86aa80fdad 100644 --- a/tests/unit/test_export_to_nitrate.py +++ b/tests/unit/test_export_to_nitrate.py @@ -20,7 +20,7 @@ def test_export_to_nitrate_step(self): os.chdir(self.tmp_dir / self.dir_name) files = os.listdir() file_name = 'test.md' - self.assertIn(file_name, files) + assert file_name in files step = convert_manual_to_nitrate(file_name)[0] html_generated = """Test\ @@ -38,13 +38,13 @@ def test_export_to_nitrate_step(self): Test two

Step 6.

description for step 2-1

Step 7.

description for step 2-2

""" - self.assertEqual(step, html_generated) + assert step == html_generated def test_export_to_nitrate_expect(self): os.chdir(self.tmp_dir / self.dir_name) files = os.listdir() file_name = 'test.md' - self.assertIn(file_name, files) + assert file_name in files expect = convert_manual_to_nitrate(file_name)[1] html_generated = """Test\ @@ -71,31 +71,31 @@ def test_export_to_nitrate_expect(self): Test two

Step 6.

description for result 2-1

Step 7.

description for Expected Result 2-2

""" - self.assertEqual(expect, html_generated) + assert expect == html_generated def test_export_to_nitrate_empty_file(self): os.chdir(self.tmp_dir / self.dir_name) files = os.listdir() file_name = 'test_empty.md' - self.assertIn(file_name, files) + assert file_name in files html = convert_manual_to_nitrate(file_name) html_generated = ('', '', '', '') - self.assertEqual(html, html_generated) + assert html == html_generated def test_export_to_nitrate_setup_doesnt_exist(self): os.chdir(self.tmp_dir / self.dir_name) files = os.listdir() file_name = 'test.md' - self.assertIn(file_name, files) + assert file_name in files cleanup = convert_manual_to_nitrate(file_name)[2] html_generated = '' - self.assertEqual(cleanup, html_generated) + assert cleanup == html_generated def test_export_to_nitrate_cleanup_latest_heading(self): os.chdir(self.tmp_dir / self.dir_name) files = os.listdir() file_name = 'test.md' - self.assertIn(file_name, files) + assert file_name in files cleanup = convert_manual_to_nitrate(file_name)[3] html_generated = """

Optionally remove temporary directory created \ @@ -103,7 +103,7 @@ def test_export_to_nitrate_cleanup_latest_heading(self): 2 line of cleanup 3 line of cleanup

""" - self.assertEqual(cleanup, html_generated) + assert cleanup == html_generated def tearDown(self): shutil.rmtree(self.tmp_dir) diff --git a/tests/unit/test_file_require.py b/tests/unit/test_file_require.py index c9e11e5ef9..75e5bfba7c 100644 --- a/tests/unit/test_file_require.py +++ b/tests/unit/test_file_require.py @@ -15,7 +15,8 @@ def test_basic(root_logger, source_dir, target_dir): identifier=tmt.base.DependencyFile(type='file', pattern=['lib.*']), source_location=source_dir, target_location=target_dir) - assert target_dir.exists() and target_dir.is_dir() + assert target_dir.exists() + assert target_dir.is_dir() target_content = list(target_dir.iterdir()) assert target_dir / 'library' in target_content assert target_dir / 'lib_folder' in target_content diff --git a/tests/unit/test_id.py b/tests/unit/test_id.py index 2ff98edaea..a19822aeb3 100644 --- a/tests/unit/test_id.py +++ b/tests/unit/test_id.py @@ -23,26 +23,25 @@ def setUp(self) -> None: def test_defined(self): node = self.base_tree.find("/yes") - self.assertEqual( - locate_key(node, ID_KEY), node) + assert locate_key(node, ID_KEY) == node def test_defined_partially(self): node = self.base_tree.find("/partial") test = tmt.Test(logger=root_logger, node=node) - self.assertEqual(locate_key(node, ID_KEY), test.node) + assert locate_key(node, ID_KEY) == test.node def test_not_defined(self): node = self.base_tree.find("/deep/structure/no") - self.assertEqual(locate_key(node, ID_KEY).name, "/deep") + assert locate_key(node, ID_KEY).name == "/deep" def test_deeper(self): node = self.base_tree.find("/deep/structure/yes") - self.assertEqual(node, locate_key(node, ID_KEY)) + assert node == locate_key(node, ID_KEY) def test_deeper_not_defined(self): node = self.base_tree.find("/deep/structure/no") - self.assertNotEqual(node, locate_key(node, ID_KEY)) - self.assertEqual(locate_key(node, ID_KEY).name, "/deep") + assert node != locate_key(node, ID_KEY) + assert locate_key(node, ID_KEY).name == "/deep" class IdLocationEmpty(TestCase): @@ -51,11 +50,11 @@ def setUp(self) -> None: def test_defined_root(self): node = self.base_tree.find("/") - self.assertEqual(locate_key(node, ID_KEY), None) + assert locate_key(node, ID_KEY) is None def test_defined(self): node = self.base_tree.find("/some/structure") - self.assertEqual(locate_key(node, ID_KEY), None) + assert locate_key(node, ID_KEY) is None class IdEmpty(TestCase): @@ -74,19 +73,19 @@ def tearDown(self): def test_base(self): node = self.base_tree.find("/some/structure") test = tmt.Test(logger=root_logger, node=node) - self.assertEqual(test.id, None) + assert test.id is None def test_manually_add_id(self): node = self.base_tree.find("/some/structure") test = tmt.Test(logger=root_logger, node=node) - self.assertEqual(test.id, None) + assert test.id is None identifier = tmt.identifier.add_uuid_if_not_defined(node, dry=False) - self.assertGreater(len(identifier), 10) + assert len(identifier) > 10 self.base_tree = fmf.Tree(self.path) node = self.base_tree.find("/some/structure") test = tmt.Test(logger=root_logger, node=node) - self.assertEqual(test.id, identifier) + assert test.id == identifier class TestGeneratorDefined(TestCase): @@ -104,25 +103,25 @@ def tearDown(self): def test_test_dry(self): result = runner.invoke( tmt.cli.main, ["test", "id", "--dry", "^/no"]) - self.assertIn("added to test '/no", result.output) + assert "added to test '/no" in result.output result = runner.invoke( tmt.cli.main, ["test", "id", "--dry", "^/no"]) - self.assertIn("added to test '/no", result.output) + assert "added to test '/no" in result.output def test_test_real(self): # Empty before node = fmf.Tree(self.path).find("/no") - self.assertEqual(node.get(ID_KEY), None) + assert node.get(ID_KEY) is None # Generate only when called for the first time result = runner.invoke(tmt.cli.main, ["test", "id", "^/no"]) - self.assertIn("added to test '/no", result.output) + assert "added to test '/no" in result.output result = runner.invoke(tmt.cli.main, ["test", "id", "^/no"]) - self.assertNotIn("added to test '/no", result.output) + assert "added to test '/no" not in result.output # Defined after node = fmf.Tree(self.path).find("/no") - self.assertGreater(len(node.data[ID_KEY]), 10) + assert len(node.data[ID_KEY]) > 10 class TestGeneratorEmpty(TestCase): @@ -140,26 +139,18 @@ def tearDown(self): def test_test_dry(self): result = runner.invoke( tmt.cli.main, ["test", "id", "--dry"]) - self.assertIn( - "added to test '/some/structure'", - result.output) + assert "added to test '/some/structure'" in result.output result = runner.invoke( tmt.cli.main, ["test", "id", "--dry"]) - self.assertIn( - "added to test '/some/structure'", - result.output) + assert "added to test '/some/structure'" in result.output def test_test_real(self): result = runner.invoke(tmt.cli.main, ["test", "id"]) - self.assertIn( - "added to test '/some/structure'", - result.output) + assert "added to test '/some/structure'" in result.output result = runner.invoke(tmt.cli.main, ["test", "id"]) - self.assertNotIn( - "added to test '/some/structure'", - result.output) + assert "added to test '/some/structure'" not in result.output base_tree = fmf.Tree(self.path) node = base_tree.find("/some/structure") - self.assertGreater(len(node.data[ID_KEY]), 10) + assert len(node.data[ID_KEY]) > 10 diff --git a/tests/unit/test_logging.py b/tests/unit/test_logging.py index f6525e5b57..95363d2214 100644 --- a/tests/unit/test_logging.py +++ b/tests/unit/test_logging.py @@ -5,8 +5,16 @@ import click import pytest -from tmt.log import (DebugLevelFilter, Logger, QuietnessFilter, Topic, - TopicFilter, VerbosityLevelFilter, indent, render_labels) +from tmt.log import ( + DebugLevelFilter, + Logger, + QuietnessFilter, + Topic, + TopicFilter, + VerbosityLevelFilter, + indent, + render_labels, + ) from . import assert_log @@ -19,11 +27,7 @@ def _exercise_logger( reset: bool = True) -> None: labels = labels or [] - if labels: - prefix = render_labels(labels) + indent_by + ' ' - - else: - prefix = indent_by + prefix = render_labels(labels) + indent_by + ' ' if labels else indent_by if reset: caplog.clear() diff --git a/tests/unit/test_report_junit.py b/tests/unit/test_report_junit.py index 9e22dfeba7..643dc2ebaf 100644 --- a/tests/unit/test_report_junit.py +++ b/tests/unit/test_report_junit.py @@ -10,7 +10,7 @@ from tmt.utils import Path -@pytest.fixture +@pytest.fixture() def report_fix(tmpdir, root_logger): # need to provide genuine workdir paths - mock would break os.path.* calls step_mock = MagicMock(workdir=str(tmpdir)) @@ -78,10 +78,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 and expected_value == actual_value, \ - f"Attribute mismatch at {tree_path_joined}: " \ - f"expected {expected_name}=\"{expected_value}\", " \ - 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 @@ -107,8 +107,7 @@ def _valid_children(node: xml.dom.Node) -> List[xml.dom.Node]: return all( _compare_xml_node( - tree_path + [ - expected_child.nodeName], + [*tree_path, expected_child.nodeName], expected_child, actual_child) for expected_child, actual_child in zip( @@ -117,10 +116,9 @@ def _valid_children(node: xml.dom.Node) -> List[xml.dom.Node]: def assert_xml(actual_filepath, expected): - with open(actual_filepath) as f: - with xml.dom.minidom.parse(f) as actual_dom: - with xml.dom.minidom.parseString(expected) as expected_dom: - assert _compare_xml_node([expected_dom.nodeName], expected_dom, actual_dom) + with open(actual_filepath) as f, xml.dom.minidom.parse(f) as actual_dom, \ + xml.dom.minidom.parseString(expected) as expected_dom: + assert _compare_xml_node([expected_dom.nodeName], expected_dom, actual_dom) @pytest.mark.skipif(pytest.__version__.startswith('3'), diff --git a/tests/unit/test_schemas.py b/tests/unit/test_schemas.py index bfff9236d9..2065b16cd2 100644 --- a/tests/unit/test_schemas.py +++ b/tests/unit/test_schemas.py @@ -70,7 +70,7 @@ def validate_node(tree, node, schema, label, name): errors = tmt.utils.validate_fmf_node(node, schema, LOGGER) if errors: - print(f"""A node in tree loaded from {str(_tree_path(tree))} failed validation + print(f"""A node in tree loaded from {_tree_path(tree)!s} failed validation """) for error, message in errors: @@ -81,7 +81,7 @@ def validate_node(tree, node, schema, label, name): {textwrap.indent(str(error), ' ')} """) - assert False, f'{label} {name} fails validation' + pytest.fail(f"{label} {name} fails validation") def _tree_path(tree): @@ -114,10 +114,9 @@ def test_plans_schema(tree, plan): # Exercise the HW requirement schema with some interesting examples # @pytest.mark.parametrize( - ('hw',), + 'hw', [ - ( - """ + """ --- arch: x86_64 @@ -157,9 +156,7 @@ def test_plans_schema(tree, plan): is-virtualized: false hypervisor: xen """, - ), - ( - """ + """ --- and: @@ -172,8 +169,7 @@ def test_plans_schema(tree, plan): is-supported: true - virtualization: is-supported: false - """, - ) + """ ], ids=[ 'all-requirements', @@ -207,10 +203,9 @@ def test_hw_schema_examples(hw: str, request) -> None: # Exercise the KS requirement schema with some real examples # @pytest.mark.parametrize( - ('ks',), + 'ks', [ - ( - """ + """ --- pre-install: | @@ -231,9 +226,7 @@ def test_hw_schema_examples(hw: str, request) -> None: "no-autopart harness=restraint" kernel-options: "ksdevice=eth1" kernel-options-post: "quiet" - """, - ), - ], + """], ids=[ 'all-properties', ] diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index c56587caf4..ca464489a7 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,5 +1,3 @@ -# coding: utf-8 - import datetime import queue import re @@ -19,16 +17,29 @@ import tmt.plugins import tmt.steps.discover import tmt.utils -from tmt.utils import (Command, Common, GeneralError, Path, ShellScript, - StructuredField, StructuredFieldError, - WaitingIncomplete, WaitingTimedOutError, _CommonBase, - duration_to_seconds, filter_paths, listify, - public_git_url, validate_git_status, wait) +from tmt.utils import ( + Command, + Common, + GeneralError, + Path, + ShellScript, + StructuredField, + StructuredFieldError, + WaitingIncompleteError, + WaitingTimedOutError, + _CommonBase, + duration_to_seconds, + filter_paths, + listify, + public_git_url, + validate_git_status, + wait, + ) run = Common(logger=tmt.log.Logger.create(verbose=0, debug=0, quiet=False)).run -@pytest.fixture +@pytest.fixture() def local_git_repo(tmpdir: py.path.local) -> Path: origin = Path(str(tmpdir)) / 'origin' origin.mkdir() @@ -54,7 +65,7 @@ def local_git_repo(tmpdir: py.path.local) -> Path: return origin -@pytest.fixture +@pytest.fixture() def origin_and_local_git_repo(local_git_repo: Path) -> Tuple[Path, Path]: top_dir = local_git_repo.parent fork_dir = top_dir / 'fork' @@ -112,8 +123,8 @@ def test_listify(): assert listify('abc') == ['abc'] assert listify('a b c') == ['a b c'] assert listify('a b c', split=True) == ['a', 'b', 'c'] - assert listify(dict(a=1, b=2)) == dict(a=[1], b=[2]) - assert listify(dict(a=1, b=2), keys=['a']) == dict(a=[1], b=2) + assert listify({'a': 1, 'b': 2}) == {'a': [1], 'b': [2]} + assert listify({'a': 1, 'b': 2}, keys=['a']) == {'a': [1], 'b': 2} def test_config(): @@ -150,7 +161,7 @@ def create_last_run(config, counter): t.join() all_good = True - for t in threads: + for _ in threads: value = results.get() if isinstance(value, Exception): # Print exception for logging @@ -216,7 +227,7 @@ def create_workdir(): all_good = True unique_workdirs = set() - for t in threads: + for _ in threads: value = results.get() if isinstance(value, Path): unique_workdirs.add(value) @@ -250,7 +261,7 @@ def test_duration_to_seconds(): duration_to_seconds('1sm') -class test_structured_field(unittest.TestCase): +class TestStructuredField(unittest.TestCase): """ Self Test """ def setUp(self): @@ -277,8 +288,8 @@ def test_everything(self): inited0 = StructuredField(text0, version=0) loaded0 = StructuredField() loaded0.load(text0, version=0) - self.assertEqual(inited0.save(), text0) - self.assertEqual(loaded0.save(), text0) + assert inited0.save() == text0 + assert loaded0.save() == text0 # Version 1 text1 = "\n".join([ self.header, @@ -287,72 +298,72 @@ def test_everything(self): inited1 = StructuredField(text1) loaded1 = StructuredField() loaded1.load(text1) - self.assertEqual(inited1.save(), text1) - self.assertEqual(loaded1.save(), text1) + assert inited1.save() == text1 + assert loaded1.save() == text1 # Common checks for field in [inited0, loaded0, inited1, loaded1]: - self.assertEqual(field.header(), self.header) - self.assertEqual(field.footer(), self.footer) - self.assertEqual(field.sections(), ["one", "two", "three"]) - self.assertEqual(field.get("one"), "1\n") - self.assertEqual(field.get("two"), "2\n") - self.assertEqual(field.get("three"), "3\n") + assert field.header() == self.header + assert field.footer() == self.footer + assert field.sections() == ['one', 'two', 'three'] + assert field.get('one') == '1\n' + assert field.get('two') == '2\n' + assert field.get('three') == '3\n' def test_no_header(self): """ No header """ # Version 0 text0 = "\n".join([self.sections, self.zeroend, self.footer]) field0 = StructuredField(text0, version=0) - self.assertEqual(field0.save(), text0) + assert field0.save() == text0 # Version 1 text1 = "\n".join( [self.start, self.sections, self.end, self.footer]) field1 = StructuredField(text1) - self.assertEqual(field1.save(), text1) + assert field1.save() == text1 # Common checks for field in [field0, field1]: - self.assertEqual(field.header(), "") - self.assertEqual(field.footer(), self.footer) - self.assertEqual(field.get("one"), "1\n") - self.assertEqual(field.get("two"), "2\n") - self.assertEqual(field.get("three"), "3\n") + assert field.header() == '' + assert field.footer() == self.footer + assert field.get('one') == '1\n' + assert field.get('two') == '2\n' + assert field.get('three') == '3\n' def test_no_footer(self): """ No footer """ # Version 0 text0 = "\n".join([self.header, self.sections, self.zeroend]) field0 = StructuredField(text0, version=0) - self.assertEqual(field0.save(), text0) + assert field0.save() == text0 # Version 1 text1 = "\n".join( [self.header, self.start, self.sections, self.end]) field1 = StructuredField(text1) - self.assertEqual(field1.save(), text1) + assert field1.save() == text1 # Common checks for field in [field0, field1]: - self.assertEqual(field.header(), self.header) - self.assertEqual(field.footer(), "") - self.assertEqual(field.get("one"), "1\n") - self.assertEqual(field.get("two"), "2\n") - self.assertEqual(field.get("three"), "3\n") + assert field.header() == self.header + assert field.footer() == '' + assert field.get('one') == '1\n' + assert field.get('two') == '2\n' + assert field.get('three') == '3\n' def test_just_sections(self): """ Just sections """ # Version 0 text0 = "\n".join([self.sections, self.zeroend]) field0 = StructuredField(text0, version=0) - self.assertEqual(field0.save(), text0) + assert field0.save() == text0 # Version 1 text1 = "\n".join([self.start, self.sections, self.end]) field1 = StructuredField(text1) - self.assertEqual(field1.save(), text1) + assert field1.save() == text1 # Common checks for field in [field0, field1]: - self.assertEqual(field.header(), "") - self.assertEqual(field.footer(), "") - self.assertEqual(field.get("one"), "1\n") - self.assertEqual(field.get("two"), "2\n") - self.assertEqual(field.get("three"), "3\n") + assert field.header() == '' + assert field.footer() == '' + assert field.get('one') == '1\n' + assert field.get('two') == '2\n' + assert field.get('three') == '3\n' def test_plain_text(self): """ Plain text """ @@ -360,11 +371,11 @@ def test_plain_text(self): field0 = StructuredField(text, version=0) field1 = StructuredField(text) for field in [field0, field1]: - self.assertEqual(field.header(), text) - self.assertEqual(field.footer(), "") - self.assertEqual(field.save(), text) - self.assertEqual(list(field), []) - self.assertEqual(bool(field), False) + assert field.header() == text + assert field.footer() == '' + assert field.save() == text + assert list(field) == [] + assert bool(field) is False def test_missing_end_tag(self): """ Missing end tag """ @@ -382,15 +393,13 @@ def test_set_content(self): field1 = StructuredField() for field in [field0, field1]: field.set("one", "1") - self.assertEqual(field.get("one"), "1\n") + assert field.get('one') == '1\n' field.set("two", "2") - self.assertEqual(field.get("two"), "2\n") + assert field.get('two') == '2\n' field.set("three", "3") - self.assertEqual(field.get("three"), "3\n") - self.assertEqual(field0.save(), "\n".join( - [self.sections, self.zeroend])) - self.assertEqual(field1.save(), "\n".join( - [self.start, self.sections, self.end])) + assert field.get('three') == '3\n' + assert field0.save() == '\n'.join([self.sections, self.zeroend]) + assert field1.save() == '\n'.join([self.start, self.sections, self.end]) def test_remove_section(self): """ Remove section """ @@ -401,19 +410,17 @@ def test_remove_section(self): for field in [field0, field1]: field.remove("one") field.remove("two") - self.assertEqual( - field0.save(), "\n".join([self.three, self.zeroend])) - self.assertEqual( - field1.save(), "\n".join([self.start, self.three, self.end])) + assert field0.save() == '\n'.join([self.three, self.zeroend]) + assert field1.save() == '\n'.join([self.start, self.three, self.end]) def test_section_tag_escaping(self): """ Section tag escaping """ field = StructuredField() field.set("section", "\n[content]\n") reloaded = StructuredField(field.save()) - self.assertTrue("section" in reloaded) - self.assertTrue("content" not in reloaded) - self.assertEqual(reloaded.get("section"), "\n[content]\n") + assert 'section' in reloaded + assert 'content' not in reloaded + assert reloaded.get('section') == '\n[content]\n' def test_nesting(self): """ Nesting """ @@ -430,37 +437,37 @@ def test_nesting(self): parent = StructuredField(parent.save()) child = StructuredField(parent.get("child")) grandchild = StructuredField(child.get("child")) - self.assertEqual(parent.get("name"), "Parent Name\n") - self.assertEqual(child.get("name"), "Child Name\n") - self.assertEqual(grandchild.get("name"), "Grand Child\n") + assert parent.get('name') == 'Parent Name\n' + assert child.get('name') == 'Child Name\n' + assert grandchild.get('name') == 'Grand Child\n' def test_section_tags_in_header(self): """ Section tags in header """ field = StructuredField("\n".join( ["[something]", self.start, self.one, self.end])) - self.assertTrue("something" not in field) - self.assertTrue("one" in field) - self.assertEqual(field.get("one"), "1\n") + assert 'something' not in field + assert 'one' in field + assert field.get('one') == '1\n' def test_empty_section(self): """ Empty section """ field = StructuredField() field.set("section", "") reloaded = StructuredField(field.save()) - self.assertEqual(reloaded.get("section"), "") + assert reloaded.get('section') == '' def test_section_item_get(self): """ Get section item """ text = "\n".join([self.start, "[section]\nx = 3\n", self.end]) field = StructuredField(text) - self.assertEqual(field.get("section", "x"), "3") + assert field.get('section', 'x') == '3' def test_section_item_set(self): """ Set section item """ text = "\n".join([self.start, "[section]\nx = 3\n", self.end]) field = StructuredField() field.set("section", "3", "x") - self.assertEqual(field.save(), text) + assert field.save() == text def test_section_item_remove(self): """ Remove section item """ @@ -468,29 +475,28 @@ def test_section_item_remove(self): [self.start, "[section]\nx = 3\ny = 7\n", self.end]) field = StructuredField(text) field.remove("section", "x") - self.assertEqual(field.save(), "\n".join( - [self.start, "[section]\ny = 7\n", self.end])) + assert field.save() == '\n'.join([self.start, '[section]\ny = 7\n', self.end]) def test_unicode_header(self): """ Unicode text in header """ - text = u"Už abychom měli unicode jako defaultní kódování!" + text = "Už abychom měli unicode jako defaultní kódování!" field = StructuredField(text) field.set("section", "content") - self.assertTrue(text in field.save()) + assert text in field.save() def test_unicode_section_content(self): """ Unicode in section content """ - chars = u"ěščřžýáíéů" + chars = "ěščřžýáíéů" text = "\n".join([self.start, "[section]", chars, self.end]) field = StructuredField(text) - self.assertEqual(field.get("section").strip(), chars) + assert field.get('section').strip() == chars def test_unicode_section_name(self): """ Unicode in section name """ - chars = u"ěščřžýáíéů" - text = "\n".join([self.start, u"[{0}]\nx".format(chars), self.end]) + chars = "ěščřžýáíéů" + text = "\n".join([self.start, f"[{chars}]\nx", self.end]) field = StructuredField(text) - self.assertEqual(field.get(chars).strip(), "x") + assert field.get(chars).strip() == 'x' def test_header_footer_modify(self): """ Modify header & footer """ @@ -499,8 +505,8 @@ def test_header_footer_modify(self): original.header("header-content\n") original.footer("footer-content\n") copy = StructuredField(original.save()) - self.assertEqual(copy.header(), "header-content\n") - self.assertEqual(copy.footer(), "footer-content\n") + assert copy.header() == 'header-content\n' + assert copy.footer() == 'footer-content\n' def test_trailing_whitespace(self): """ Trailing whitespace """ @@ -508,9 +514,9 @@ def test_trailing_whitespace(self): original.set("name", "value") # Test with both space and tab appended after the section tag for char in [" ", "\t"]: - spaced = re.sub(r"\]\n", "]{0}\n".format(char), original.save()) + spaced = re.sub(r"\]\n", f"]{char}\n", original.save()) copy = StructuredField(spaced) - self.assertEqual(original.get("name"), copy.get("name")) + assert original.get('name') == copy.get('name') def test_carriage_returns(self): """ Carriage returns """ @@ -518,7 +524,7 @@ def test_carriage_returns(self): text2 = re.sub(r"\n", "\r\n", text1) field1 = StructuredField(text1) field2 = StructuredField(text2) - self.assertEqual(field1.save(), field2.save()) + assert field1.save() == field2.save() def test_multiple_values(self): """ Multiple values """ @@ -526,17 +532,16 @@ def test_multiple_values(self): section = "[section]\nkey=val1 # comment\nkey = val2\n key = val3 " text = "\n".join([self.start, section, self.end]) field = StructuredField(text, multi=True) - self.assertEqual( - field.get("section", "key"), ["val1", "val2", "val3"]) + assert field.get('section', 'key') == ['val1', 'val2', 'val3'] # Writing multiple values values = ['1', '2', '3'] field = StructuredField(multi=True) field.set("section", values, "key") - self.assertEqual(field.get("section", "key"), values) - self.assertTrue("key = 1\nkey = 2\nkey = 3" in field.save()) + assert field.get('section', 'key') == values + assert 'key = 1\nkey = 2\nkey = 3' in field.save() # Remove multiple values field.remove("section", "key") - self.assertTrue("key = 1\nkey = 2\nkey = 3" not in field.save()) + assert 'key = 1\nkey = 2\nkey = 3' not in field.save() self.assertRaises( StructuredFieldError, field.get, "section", "key") @@ -619,7 +624,7 @@ def test_run_big(root_logger): def test_get_distgit_handler(): - for wrong_remotes in [[], ["blah"]]: + for _wrong_remotes in [[], ["blah"]]: with pytest.raises(tmt.utils.GeneralError): tmt.utils.get_distgit_handler([]) # Fedora detection @@ -645,7 +650,7 @@ def test_get_distgit_handler_explicit(): assert instance.__class__.__name__ == 'RedHatGitlab' -def test_FedoraDistGit(tmpdir): +def test_fedora_dist_git(tmpdir): # Fake values, production hash is too long path = Path(str(tmpdir)) path.joinpath('sources').write_text('SHA512 (fn-1.tar.gz) = 09af\n') @@ -655,7 +660,8 @@ def test_FedoraDistGit(tmpdir): "fn-1.tar.gz")] == fedora_sources_obj.url_and_name(cwd=path) -class Test_validate_git_status: +class TestValidateGitStatus: + @classmethod @pytest.mark.parametrize("use_path", [False, True], ids=["without path", "with path"]) def test_all_good( @@ -668,10 +674,7 @@ def test_all_good( # In local repo: # Init tmt and add test - if use_path: - fmf_root = mine / 'fmf_root' - else: - fmf_root = mine + fmf_root = mine / 'fmf_root' if use_path else mine tmt.Tree.init(logger=root_logger, path=fmf_root, template=None, force=None) fmf_root.joinpath('main.fmf').write_text('test: echo') run(ShellScript(f'git add {fmf_root} {fmf_root / "main.fmf"}').to_shell_command(), @@ -684,6 +687,7 @@ def test_all_good( validation = validate_git_status(test) assert validation == (True, '') + @classmethod def test_no_remote(cls, local_git_repo: Path, root_logger): tmpdir = local_git_repo tmt.Tree.init(logger=root_logger, path=tmpdir, template=None, force=None) @@ -698,6 +702,7 @@ def test_no_remote(cls, local_git_repo: Path, root_logger): assert not val assert "Failed to get remote branch" in msg + @classmethod def test_untracked_fmf_root(cls, local_git_repo: Path, root_logger): # local repo is enough since this can't get passed 'is pushed' check tmt.Tree.init(logger=root_logger, path=local_git_repo, template=None, force=None) @@ -713,6 +718,7 @@ def test_untracked_fmf_root(cls, local_git_repo: Path, root_logger): validate = validate_git_status(test) assert validate == (False, 'Uncommitted changes in .fmf/version') + @classmethod def test_untracked_sources(cls, local_git_repo: Path, root_logger): tmt.Tree.init(logger=root_logger, path=local_git_repo, template=None, force=None) local_git_repo.joinpath('main.fmf').write_text('test: echo') @@ -727,6 +733,7 @@ def test_untracked_sources(cls, local_git_repo: Path, root_logger): validate = validate_git_status(test) assert validate == (False, 'Uncommitted changes in main.fmf') + @classmethod @pytest.mark.parametrize("use_path", [False, True], ids=["without path", "with path"]) def test_local_changes( @@ -736,10 +743,7 @@ def test_local_changes( root_logger): origin, mine = origin_and_local_git_repo - if use_path: - fmf_root = origin / 'fmf_root' - else: - fmf_root = origin + fmf_root = origin / 'fmf_root' if use_path else origin tmt.Tree.init(logger=root_logger, path=fmf_root, template=None, force=None) fmf_root.joinpath('main.fmf').write_text('test: echo') run(ShellScript('git add -A').to_shell_command(), cwd=origin) @@ -764,6 +768,7 @@ def test_local_changes( assert validation_result == ( False, "Uncommitted changes in " + ('fmf_root/' if use_path else '') + "main.fmf") + @classmethod def test_not_pushed(cls, origin_and_local_git_repo: Tuple[Path, Path], root_logger): # No need for original repo (it is required just to have remote in # local clone) @@ -834,7 +839,7 @@ def check(): ticks.pop() - raise WaitingIncomplete() + raise WaitingIncompleteError # We want to reach end of our list, give enough time budget. r = wait(Common(logger=root_logger), check, datetime.timedelta(seconds=3600), tick=0.01) @@ -851,7 +856,7 @@ def test_wait_timeout(root_logger): check = unittest.mock.MagicMock( __name__='mock_check', - side_effect=WaitingIncomplete) + side_effect=WaitingIncompleteError) # We want to reach end of time budget before reaching end of the list. with pytest.raises(WaitingTimedOutError): @@ -990,12 +995,12 @@ def test_flatten(lists: List[List[Any]], unique: bool, expected: List[Any]) -> N @pytest.mark.parametrize( ('duration', 'expected'), - ( + [ (timedelta(seconds=8), '00:00:08'), (timedelta(minutes=6, seconds=8), '00:06:08'), (timedelta(hours=4, minutes=6, seconds=8), '04:06:08'), (timedelta(days=15, hours=4, minutes=6, seconds=8), '364:06:08'), - ) + ] ) def test_format_duration(duration, expected): from tmt.steps.execute import ExecutePlugin diff --git a/tmt/base.py b/tmt/base.py index 9f6cb385f5..a84a16e1a6 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -9,9 +9,24 @@ import shutil import sys import time -from typing import (TYPE_CHECKING, Any, Callable, ClassVar, Dict, Generator, - Iterable, Iterator, List, Optional, Sequence, Tuple, Type, - TypeVar, Union, cast) +from typing import ( + TYPE_CHECKING, + Any, + Callable, + ClassVar, + Dict, + Generator, + Iterable, + Iterator, + List, + Optional, + Sequence, + Tuple, + Type, + TypeVar, + Union, + cast, + ) import fmf import fmf.base @@ -37,9 +52,17 @@ import tmt.utils from tmt.lint import LinterOutcome, LinterReturn from tmt.result import Result, ResultOutcome -from tmt.utils import (Command, EnvironmentType, FmfContextType, Path, - ShellScript, WorkdirArgumentType, field, - normalize_shell_script, verdict) +from tmt.utils import ( + Command, + EnvironmentType, + FmfContextType, + Path, + ShellScript, + WorkdirArgumentType, + field, + normalize_shell_script, + verdict, + ) if sys.version_info >= (3, 8): from typing import Literal, TypedDict @@ -324,7 +347,7 @@ class DependencyFmfId(FmfId): several extra keys. """ - VALID_KEYS: ClassVar[List[str]] = FmfId.VALID_KEYS + ['destination', 'nick', 'type'] + VALID_KEYS: ClassVar[List[str]] = [*FmfId.VALID_KEYS, 'destination', 'nick', 'type'] destination: Optional[Path] = None nick: Optional[str] = None @@ -502,7 +525,7 @@ def normalize_require( if raw_require is None: return [] - if isinstance(raw_require, str) or isinstance(raw_require, dict): + if isinstance(raw_require, (str, dict)): return [dependency_factory(raw_require)] if isinstance(raw_require, list): @@ -690,10 +713,7 @@ def web_link(self) -> Optional[str]: # Detect relative path of the last source from the metadata tree root relative_path = Path(self.node.sources[-1]).relative_to(self.node.root) - if str(relative_path) == '.': - relative_path = Path('/') - else: - relative_path = Path('/') / relative_path + relative_path = Path('/') if str(relative_path) == '.' else Path('/') / relative_path # Add fmf path if the tree is nested deeper in the git repo if fmf_id.path: @@ -704,7 +724,7 @@ def web_link(self) -> Optional[str]: @classmethod def _save_cli_context(cls, context: 'tmt.cli.Context') -> None: """ Save provided command line context for future use """ - super(Core, cls)._save_cli_context(context) + super()._save_cli_context(context) # Handle '.' as an alias for the current working directory names = cls._opt('names') @@ -725,9 +745,8 @@ def _save_cli_context(cls, context: 'tmt.cli.Context') -> None: def name_and_summary(self) -> str: """ Node name and optional summary """ if self.summary: - return '{0} ({1})'.format(self.name, self.summary) - else: - return self.name + return f'{self.name} ({self.summary})' + return self.name def ls(self, summary: bool = False) -> None: """ List node """ @@ -740,7 +759,7 @@ def _export(self, *, keys: Optional[List[str]] = None) -> tmt.export._RawExporte keys = self._keys() # Always include node name, add requested keys, ignore adjust - data: Dict[str, Any] = dict(name=self.name) + data: Dict[str, Any] = {'name': self.name} for key in keys: # TODO: provide more mature solution for https://github.com/teemtee/tmt/issues/1688 # Until that, do not export fields that start with an underscore, to avoid leaking @@ -775,7 +794,7 @@ def _export(self, *, keys: Optional[List[str]] = None) -> tmt.export._RawExporte def _lint_keys(self, additional_keys: List[str]) -> List[str]: """ Return list of invalid keys used, empty when all good """ known_keys = additional_keys + self._keys() - return [key for key in self.node.get().keys() if key not in known_keys] + return [key for key in self.node.get() if key not in known_keys] def lint_validate(self) -> LinterReturn: """ C000: fmf node should pass the schema validation """ @@ -1022,10 +1041,7 @@ def __init__( try: directory = Path(node.sources[-1]).parent relative_path = directory.relative_to(Path(node.root)) - if relative_path == Path('.'): - default_path = Path('/') - else: - default_path = Path('/') / relative_path + default_path = Path('/') if relative_path == Path('.') else Path('/') / relative_path except (AttributeError, IndexError): default_path = Path('/') @@ -1058,7 +1074,8 @@ def _export(self, *, keys: Optional[List[str]] = None) -> tmt.export._RawExporte dependency.to_minimal_spec() for dependency in cast(List[Dependency], value) ] - elif key == 'test' and isinstance(value, ShellScript): + # Combining `if` branches using `or` here would result in long, complex line. + elif key == 'test' and isinstance(value, ShellScript): # noqa: SIM114 data[key] = str(value) elif key == 'path' and isinstance(value, Path): @@ -1145,7 +1162,7 @@ def show(self) -> None: [dependency.to_minimal_spec() for dependency in cast(List[Dependency], value)] )) continue - if value not in [None, list(), dict()]: + if value not in [None, [], {}]: echo(tmt.utils.format(key, value)) if self.opt('verbose'): self._show_additional_keys() @@ -1156,7 +1173,7 @@ def show(self) -> None: if key in self._keys(): continue value = self.node.get(key) - if value not in [None, list(), dict()]: + if value not in [None, [], {}]: echo(tmt.utils.format(key, value, key_color='blue')) # FIXME - Make additional attributes configurable @@ -1339,7 +1356,7 @@ class Plan( _original_plan: Optional['Plan'] = None _remote_plan_fmf_id: Optional[FmfId] = None - _extra_L2_keys = [ + _extra_l2_keys = [ 'context', 'environment', 'environment-file', @@ -1461,7 +1478,7 @@ def _expand_node_data(self, data: T, fmf_context: Dict[str, str]) -> T: # and we need to help with an explicit cast(). return cast(T, ''.join(expanded_ctx)) - elif isinstance(data, dict): + if isinstance(data, dict): for key, value in data.items(): data[key] = self._expand_node_data(value, fmf_context) elif isinstance(data, list): @@ -1499,8 +1516,7 @@ def environment(self) -> EnvironmentType: combined["TMT_TREE"] = str(self.worktree) return combined - else: - return self._environment + return self._environment def _get_environment_vars(self, node: fmf.Tree) -> EnvironmentType: """ Get variables from 'environment' and 'environment-file' keys """ @@ -1517,7 +1533,7 @@ def _get_environment_vars(self, node: fmf.Tree) -> EnvironmentType: # Environment variables from key, make sure that values are string environment = { key: str(value) for key, value - in node.get('environment', dict()).items()} + in node.get('environment', {}).items()} # Combine both sources into one ('environment' key takes precendence) combined.update(environment) @@ -1653,7 +1669,7 @@ def create( content = tmt.templates.PLAN[template] except KeyError: raise tmt.utils.GeneralError( - "Invalid template '{}'.".format(template)) + f"Invalid template '{template}'.") # Override template with data provided on command line content = Plan.edit_template(content) @@ -1761,7 +1777,7 @@ def lint_unknown_keys(self) -> LinterReturn: """ P001: all keys are known """ invalid_keys = self._lint_keys( - list(self.step_names(enabled=True, disabled=True)) + self._extra_L2_keys) + list(self.step_names(enabled=True, disabled=True)) + self._extra_l2_keys) if invalid_keys: for key in invalid_keys: @@ -1920,7 +1936,7 @@ def go(self) -> None: f'These steps require running on their own, their combination ' f'with the given options is not compatible: ' f'{fmf.utils.listed(standalone)}.') - elif standalone: + if standalone: assert self._cli_context_object is not None # narrow type self._cli_context_object.steps = standalone self.debug( @@ -1948,7 +1964,7 @@ def go(self) -> None: def _export(self, *, keys: Optional[List[str]] = None) -> tmt.export._RawExportedInstance: data = super()._export(keys=keys) - for key in self._extra_L2_keys: + for key in self._extra_l2_keys: value = self.node.data.get(key) if value: data[key] = value @@ -2184,7 +2200,7 @@ def create( content = tmt.templates.STORY[template] except KeyError: raise tmt.utils.GeneralError( - "Invalid template '{}'.".format(template)) + f"Invalid template '{template}'.") tmt.utils.create_file( path=story_path, content=content, @@ -2268,7 +2284,7 @@ class Tree(tmt.utils.Common): def __init__(self, *, - path: Path = Path.cwd(), + path: Optional[Path] = None, tree: Optional[fmf.Tree] = None, fmf_context: Optional[tmt.utils.FmfContextType] = None, logger: tmt.log.Logger) -> None: @@ -2277,7 +2293,7 @@ def __init__(self, # TODO: not calling parent __init__ on purpose? self.inject_logger(logger) - self._path = path + self._path = path or Path.cwd() self._tree = tree self._custom_fmf_context = fmf_context or {} @@ -2285,7 +2301,7 @@ def __init__(self, def grow( cls, *, - path: Path = Path.cwd(), + path: Optional[Path] = None, tree: Optional[fmf.Tree] = None, fmf_context: Optional[tmt.utils.FmfContextType] = None, logger: Optional[tmt.log.Logger] = None) -> 'Tree': @@ -2309,7 +2325,7 @@ def grow( tmt.plugins.explore(logger) return Tree( - path=path, + path=path or Path.cwd(), tree=tree, fmf_context=fmf_context, logger=logger or tmt.log.Logger.create()) @@ -2374,7 +2390,7 @@ def sanitize_cli_names(self, names: List[str]) -> List[str]: """ Sanitize CLI names in case name includes control character """ for name in names: if not name.isprintable(): - raise tmt.utils.GeneralError(f"Invalid name {repr(name)} as it's not printable.") + raise tmt.utils.GeneralError(f"Invalid name {name!r} as it's not printable.") return names @property @@ -2552,8 +2568,7 @@ def plans( filters, conditions, links, excludes) if Plan._opt('shallow'): return plans - else: - return [plan.import_plan() or plan for plan in plans] + return [plan.import_plan() or plan for plan in plans] def stories( self, @@ -2709,7 +2724,7 @@ def __init__(self, self._workdir_path: WorkdirArgumentType = id_ or True self._tree = tree self._plans: Optional[List[Plan]] = None - self._environment_from_workdir: EnvironmentType = dict() + self._environment_from_workdir: EnvironmentType = {} self._environment_from_options: Optional[EnvironmentType] = None self.remove = self.opt('remove') @@ -2755,7 +2770,7 @@ def environment(self) -> EnvironmentType: # Gather environment variables from options only once if self._environment_from_options is None: assert self.tree is not None # narrow type - self._environment_from_options = dict() + self._environment_from_options = {} # Variables gathered from 'environment-file' options self._environment_from_options.update( tmt.utils.environment_files_to_dict( @@ -2917,20 +2932,20 @@ def finish(self) -> None: def follow(self) -> None: """ Periodically check for new lines in the log. """ assert self.workdir is not None # narrow type - logfile = open(self.workdir / tmt.log.LOG_FILENAME, 'r') - # Move to the end of the file - logfile.seek(0, os.SEEK_END) - # Rewind some lines back to show more context - location = logfile.tell() - read_lines = 0 - while location >= 0: - logfile.seek(location) - location -= 1 - current_char = logfile.read(1) - if current_char == '\n': - read_lines += 1 - if read_lines > FOLLOW_LINES: - break + with open(self.workdir / tmt.log.LOG_FILENAME) as logfile: + # Move to the end of the file + logfile.seek(0, os.SEEK_END) + # Rewind some lines back to show more context + location = logfile.tell() + read_lines = 0 + while location >= 0: + logfile.seek(location) + location -= 1 + current_char = logfile.read(1) + if current_char == '\n': + read_lines += 1 + if read_lines > FOLLOW_LINES: + break while True: line = logfile.readline() @@ -3001,7 +3016,7 @@ def go(self) -> None: # Show summary, store run data if not self.plans: raise tmt.utils.GeneralError("No plans found.") - self.verbose('Found {0}.'.format(listed(self.plans, 'plan'))) + self.verbose(f"Found {listed(self.plans, 'plan')}.") self.save() # Iterate over plans @@ -3032,8 +3047,7 @@ def get_overall_plan_status(plan: Plan) -> str: if i + 1 == len(steps): # Last enabled step, consider the whole plan done return 'done' - else: - return step_names[i] + return step_names[i] return 'todo' def plan_matches_filters(self, plan: Plan) -> bool: @@ -3052,8 +3066,7 @@ def colorize_column(content: str) -> str: """ Add color to a status column """ if 'done' in content: return style(content, fg='green') - else: - return style(content, fg='yellow') + return style(content, fg='yellow') @classmethod def pad_with_spaces(cls, string: str) -> str: @@ -3081,7 +3094,7 @@ def print_run_status(self, run: Run) -> None: plan_status = self.get_overall_plan_status(plan) if plan_status == 'done': continue - elif plan_status == 'todo': + if plan_status == 'todo': # If plan has no steps done, consider the whole run not done earliest_step_index = -1 break @@ -3217,30 +3230,30 @@ def _stop_running_guests(self, run: Run) -> bool: # Clean guests if provision is done but finish is not done successful = True for plan in run.plans: - if plan.provision.status() == 'done': - if plan.finish.status() != 'done': - # Wake up provision to load the active guests - plan.provision.wake() - if not self._matches_how(plan): - continue - if self.opt('dry'): - self.verbose( - f"Would stop guests in run '{run.workdir}'" - f" plan '{plan.name}'.", shift=1) - else: - self.verbose(f"Stopping guests in run '{run.workdir}' " - f"plan '{plan.name}'.", shift=1) - # Set --quiet to avoid finish logging to terminal - quiet = self._cli_options['quiet'] - self._cli_options['quiet'] = True - try: - plan.finish.go() - except tmt.utils.GeneralError as error: - self.warn(f"Could not stop guest in run " - f"'{run.workdir}': {error}.", shift=1) - successful = False - finally: - self._cli_options['quiet'] = quiet + if plan.provision.status() == 'done' and plan.finish.status() != 'done': + # Wake up provision to load the active guests + plan.provision.wake() + if not self._matches_how(plan): + continue + if self.opt('dry'): + self.verbose( + f"Would stop guests in run '{run.workdir}'" + f" plan '{plan.name}'.", shift=1) + else: + self.verbose(f"Stopping guests in run '{run.workdir}' " + f"plan '{plan.name}'.", shift=1) + # Set --quiet to avoid finish logging to terminal + + quiet = self._cli_options['quiet'] + self._cli_options['quiet'] = True + try: + plan.finish.go() + except tmt.utils.GeneralError as error: + self.warn(f"Could not stop guest in run " + f"'{run.workdir}': {error}.", shift=1) + successful = False + finally: + self._cli_options['quiet'] = quiet return successful def guests(self) -> bool: @@ -3292,7 +3305,7 @@ def runs(self) -> bool: last_run._workdir_load(last_run._workdir_path) assert last_run.workdir is not None # narrow type return self._clean_workdir(last_run.workdir) - all_workdirs = [path for path in tmt.utils.generate_runs(root_path, id_)] + all_workdirs = list(tmt.utils.generate_runs(root_path, id_)) keep = self.opt('keep') if keep is not None: # Sort by modify time of the workdirs and keep the newest workdirs @@ -3406,7 +3419,7 @@ def from_spec(cls, spec: _RawLink) -> 'Link': # Count how many relations are stored in spec. relations = [cast(_RawLinkRelationName, key) - for key in spec if key not in (FmfId.VALID_KEYS + ['note'])] + for key in spec if key not in ([*FmfId.VALID_KEYS, 'note'])] # If there are no relations, spec must be an fmf id, representing # a target. @@ -3609,4 +3622,5 @@ def resolve_dynamic_ref( Plan(logger=logger, node=reference_tree, run=plan.my_run, skip_validation=True) ref = reference_tree.get("ref") - return ref + # RET504: Unnecessary variable assignment before `return` statement. Keeping for readability. + return ref # noqa: RET504 diff --git a/tmt/cli.py b/tmt/cli.py index fc4229fd11..560db1b727 100644 --- a/tmt/cli.py +++ b/tmt/cli.py @@ -6,8 +6,7 @@ import dataclasses import subprocess import sys -from typing import (TYPE_CHECKING, Any, DefaultDict, List, Optional, Set, - Tuple, Type, Union) +from typing import TYPE_CHECKING, Any, DefaultDict, List, Optional, Set, Tuple, Type, Union import click import fmf @@ -119,10 +118,9 @@ def get_command( # type: ignore[override] if command.startswith(cmd_name)] if not matches: return None - elif len(matches) == 1: + if len(matches) == 1: return click.Group.get_command(self, context, matches[0]) - context.fail('Did you mean {}?'.format( - listed(sorted(matches), join='or'))) + context.fail(f"Did you mean {listed(sorted(matches), join='or')}?") return None @@ -549,8 +547,8 @@ def tests_lint( @click.argument('name') @option( '-t', '--template', metavar='TEMPLATE', - help='Test template ({}).'.format(_test_templates), - prompt='Template ({})'.format(_test_templates)) + help=f'Test template ({_test_templates}).', + prompt=f'Template ({_test_templates})') @verbosity_options @force_dry_options def tests_create( @@ -659,9 +657,8 @@ def tests_import( if not (case or plan): raise tmt.utils.GeneralError( "Option --case or --plan is mandatory when using --manual.") - else: - tmt.convert.read_manual(plan, case, disabled, with_script) - return + tmt.convert.read_manual(plan, case, disabled, with_script) + return if not paths: paths = ['.'] @@ -671,7 +668,7 @@ def tests_import( path = path.resolve() if not path.is_dir(): raise tmt.utils.GeneralError( - "Path '{0}' is not a directory.".format(path)) + f"Path '{path}' is not a directory.") # Gather old metadata and store them as fmf common, individual = tmt.convert.read( path, makefile, restraint, nitrate, polarion, polarion_case_id, link_polarion, @@ -932,8 +929,8 @@ def plans_lint( @click.argument('name') @option( '-t', '--template', metavar='TEMPLATE', - help='Plan template ({}).'.format(_plan_templates), - prompt='Template ({})'.format(_plan_templates)) + help=f'Plan template ({_plan_templates}).', + prompt=f'Template ({_plan_templates})') @option( '--discover', metavar='YAML', multiple=True, help='Discover phase content in yaml format.') @@ -1122,8 +1119,8 @@ def stories_show( @click.argument('name') @option( '-t', '--template', metavar='TEMPLATE', - prompt='Template ({})'.format(_story_templates), - help='Story template ({}).'.format(_story_templates)) + prompt=f'Template ({_story_templates})', + help=f'Story template ({_story_templates}).') @verbosity_options @force_dry_options def stories_create( @@ -1206,12 +1203,12 @@ def headfoot(text: str) -> None: if not total: return if code: - headfoot('{}%'.format(round(100 * code_coverage / total))) + headfoot(f'{round(100 * code_coverage / total)}%') if test: - headfoot('{}%'.format(round(100 * test_coverage / total))) + headfoot(f'{round(100 * test_coverage / total)}%') if docs: - headfoot('{}%'.format(round(100 * docs_coverage / total))) - headfoot('from {}'.format(listed(total, 'story'))) + headfoot(f'{round(100 * docs_coverage / total)}%') + headfoot(f"from {listed(total, 'story')}") echo() @@ -1355,9 +1352,8 @@ def stories_id( @click.argument('path', default='.') @option( '-t', '--template', default='empty', metavar='TEMPLATE', - type=click.Choice(['empty'] + tmt.templates.INIT_TEMPLATES), - help='Template ({}).'.format( - listed(tmt.templates.INIT_TEMPLATES, join='or'))) + type=click.Choice(['empty', *tmt.templates.INIT_TEMPLATES]), + help=f"Template ({listed(tmt.templates.INIT_TEMPLATES, join='or')}).") @verbosity_options @force_dry_options def init( @@ -1724,7 +1720,8 @@ def setup_completion(shell: str, install: bool) -> None: else: script = Path(config.path) / f'{COMPLETE_SCRIPT}.{shell}' - out = open(script, 'w') if install else sys.stdout + # 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) diff --git a/tmt/convert.py b/tmt/convert.py index b2e72163b1..e869c0cf90 100644 --- a/tmt/convert.py +++ b/tmt/convert.py @@ -1,5 +1,3 @@ -# coding: utf-8 - """ Convert metadata into the new format """ import copy @@ -8,7 +6,7 @@ import re import shlex import subprocess -from io import open +from contextlib import suppress from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union from uuid import UUID, uuid4 @@ -103,7 +101,7 @@ def read_manual( directory.mkdir(exist_ok=True) os.chdir(directory) - echo("Importing the '{0}' test case.".format(directory)) + echo(f"Importing the '{directory}' test case.") # Test case data md_content = read_manual_data(testcase) @@ -164,14 +162,14 @@ def write_markdown(path: Path, content: Dict[str, str]) -> None: md_file.write(to_print) echo(style( f"Test case successfully stored into '{path}'.", fg='magenta')) - except IOError: + except OSError: raise ConvertError(f"Unable to write '{path}'.") def add_link(target: str, data: NitrateDataType, system: int = SYSTEM_BUGZILLA, type_: str = 'relates') -> None: """ Add relevant link into data under the 'link' key """ - new_link = dict() + new_link = {} if system == SYSTEM_BUGZILLA: new_link[type_] = f"{BUGZILLA_URL}{target}" elif system == SYSTEM_JIRA: @@ -201,7 +199,7 @@ def read_datafile( Returns task name and a dictionary of the collected values. """ - data: NitrateDataType = dict() + data: NitrateDataType = {} makefile_regex_test = r'^run:.*\n\t(.*)$' if filename == 'Makefile': regex_task = r'Name:[ \t]*(.*)$' @@ -276,7 +274,7 @@ def read_datafile( makefile = makefile_file.read() search_result = \ re.search(makefile_regex_test, makefile, re.M) - except IOError: + except OSError: raise ConvertError("Makefile is missing.") # Retrieve the path to the test file from the Makefile if search_result is not None: @@ -291,8 +289,8 @@ def read_datafile( else: data["framework"] = "shell" echo(style("framework: ", fg="green") + data["framework"]) - except IOError: - raise ConvertError("Unable to open '{0}'.".format(test_path)) + except OSError: + raise ConvertError(f"Unable to open '{test_path}'.") # Contact search_result = re.search(regex_contact, testinfo, re.M) @@ -399,7 +397,7 @@ def read( data for individual testcases (if multiple nitrate testcases found). """ - echo("Checking the '{0}' directory.".format(path)) + echo(f"Checking the '{path}' directory.") # Make sure there is a metadata tree initialized try: @@ -422,7 +420,7 @@ def read( if not makefile and not restraint and not polarion: raise ConvertError( "Please specify either a Makefile or a Restraint file or a Polarion case ID.") - elif makefile and restraint: + if makefile and restraint: if 'metadata' in filenames: filename = 'metadata' restraint_file = True @@ -436,17 +434,15 @@ def read( elif makefile: if 'Makefile' not in filenames: raise ConvertError("Unable to find Makefile") - else: - filename = 'Makefile' - makefile_file = True - echo(style('Makefile ', fg='blue'), nl=False) + filename = 'Makefile' + makefile_file = True + echo(style('Makefile ', fg='blue'), nl=False) elif restraint: if 'metadata' not in filenames: raise ConvertError("Unable to find restraint metadata file") - else: - filename = 'metadata' - restraint_file = True - echo(style('Restraint ', fg='blue'), nl=False) + filename = 'metadata' + restraint_file = True + echo(style('Restraint ', fg='blue'), nl=False) if filename is None and not polarion: raise GeneralError('Filename is not defined and there is no import from Polarion') @@ -457,10 +453,9 @@ def read( try: with open(datafile_path, encoding='utf-8') as datafile_file: datafile = datafile_file.read() - except IOError: - raise ConvertError("Unable to open '{0}'.".format( - datafile_path)) - echo("found in '{0}'.".format(datafile_path)) + except OSError: + raise ConvertError(f"Unable to open '{datafile_path}'.") + echo(f"found in '{datafile_path}'.") # If testinfo.desc exists read it to preserve content and remove it testinfo_path = path / 'testinfo.desc' @@ -469,9 +464,9 @@ def read( with open(testinfo_path, encoding='utf-8') as testinfo_file: old_testinfo = testinfo_file.read() testinfo_path.unlink() - except IOError: + except OSError: raise ConvertError( - "Unable to open '{0}'.".format(testinfo_path)) + f"Unable to open '{testinfo_path}'.") else: old_testinfo = None @@ -492,8 +487,7 @@ def read( stdout=subprocess.DEVNULL) except FileNotFoundError: raise ConvertError( - "Install tmt-test-convert to " - "convert metadata from {0}.".format(filename)) + f"Install tmt-test-convert to convert metadata from {filename}.") except subprocess.CalledProcessError: raise ConvertError( "Failed to convert metadata using 'make testinfo.desc'.") @@ -502,9 +496,8 @@ def read( try: with open(testinfo_path, encoding='utf-8') as testinfo_file: testinfo = testinfo_file.read() - except IOError: - raise ConvertError("Unable to open '{0}'.".format( - testinfo_path)) + except OSError: + raise ConvertError(f"Unable to open '{testinfo_path}'.") # restraint if restraint_file: @@ -569,9 +562,9 @@ def target_content_build() -> List[str]: try: with open(testinfo_path, 'w', encoding='utf-8') as testinfo_file: testinfo_file.write(old_testinfo) - except IOError: + except OSError: raise ConvertError( - "Unable to write '{0}'.".format(testinfo_path)) + f"Unable to write '{testinfo_path}'.") # Remove created testinfo.desc otherwise else: testinfo_path.unlink() @@ -586,13 +579,13 @@ def target_content_build() -> List[str]: try: with open(purpose_path, encoding='utf-8') as purpose_file: content = purpose_file.read() - echo("found in '{0}'.".format(purpose_path)) + echo(f"found in '{purpose_path}'.") for header in ['PURPOSE', 'Description', 'Author']: - content = re.sub('^{0}.*\n'.format(header), '', content) + content = re.sub(f'^{header}.*\n', '', content) data['description'] = content.lstrip('\n') echo(style('description:', fg='green')) echo(data['description'].rstrip('\n')) - except IOError: + except OSError: echo("not found.") # Nitrate (extract contact, environment and relevancy) @@ -612,7 +605,7 @@ def target_content_build() -> List[str]: parent_name = str(Path('/') / parent_path.relative_to(tree.root)) parent = tree.find(parent_name) if parent: - for test in [common_data] + individual_data: + for test in [common_data, *individual_data]: for key in list(test): if parent.get(key) == test[key]: test.pop(key) @@ -633,9 +626,8 @@ def filter_common_data( if len(individual_data) > 1: for testcase in individual_data[1:]: for key, value in testcase.items(): - if key in common_candidates: - if value != common_candidates[key]: - common_candidates.pop(key) + if key in common_candidates and value != common_candidates[key]: + common_candidates.pop(key) if key in histogram: histogram[key] += 1 @@ -694,15 +686,15 @@ def read_nitrate( gssapi.raw.misc.GSSError) as error: raise ConvertError(str(error)) if not testcases: - echo("No {0}testcase found for '{1}'.".format( + echo("No {}testcase found for '{}'.".format( '' if disabled else 'non-disabled ', beaker_task)) return common_data, [] - elif len(testcases) > 1: - echo("Multiple test cases found for '{0}'.".format(beaker_task)) + if len(testcases) > 1: + echo(f"Multiple test cases found for '{beaker_task}'.") # Process individual test cases - individual_data = list() - md_content = dict() + individual_data = [] + md_content = {} for testcase in testcases: # Testcase data must be fetched due to # https://github.com/psss/python-nitrate/issues/24 @@ -726,9 +718,9 @@ def read_nitrate( "successfully removed.", fg='magenta')) except FileNotFoundError: pass - except IOError: + except OSError: raise ConvertError( - "Unable to remove '{0}'.".format(md_path)) + f"Unable to remove '{md_path}'.") # Merge environment from Makefile and Nitrate if 'environment' in common_data: @@ -863,7 +855,7 @@ def read_polarion_case( echo(style('description: ', fg='green') + current_data['description']) # Update status - status = True if polarion_case.status == 'approved' else False + status = polarion_case.status == 'approved' if not current_data.get('enabled') or current_data['enabled'] != status: current_data['enabled'] = status echo(style('enabled: ', fg='green') + str(current_data['enabled'])) @@ -911,11 +903,11 @@ def read_polarion_case( if link.role == 'verifies': add_link( f'{server_url}#/project/{link.project_id}/' - f'workitem?id={str(link.work_item_id)}', + f'workitem?id={link.work_item_id!s}', current_data, system=SYSTEM_OTHER, type_=str(link.role)) add_link( f'{server_url}#/project/{polarion_case.project_id}/workitem?id=' - f'{str(polarion_case.work_item_id)}', + f'{polarion_case.work_item_id!s}', current_data, system=SYSTEM_OTHER, type_='implements') if not file_name: file_name = str(polarion_case.work_item_id) @@ -960,7 +952,7 @@ def read_nitrate_case( import tmt.export.nitrate data: NitrateDataType = {'tag': []} - echo("test case found '{0}'.".format(testcase.identifier)) + echo(f"test case found '{testcase.identifier}'.") # Test identifier data['extra-nitrate'] = testcase.identifier # Beaker task name (taken from summary) @@ -971,8 +963,7 @@ def read_nitrate_case( if testcase.tester: # Full 'Name Surname ' form if testcase.tester.name is not None: - data['contact'] = '{} <{}>'.format( - testcase.tester.name, testcase.tester.email) + data['contact'] = f'{testcase.tester.name} <{testcase.tester.email}>' else: if makefile_data is None or 'contact' not in makefile_data: # Otherwise use just the email address @@ -1101,15 +1092,15 @@ def adjust_runtest(path: Path) -> None: else: runtest.write(line) runtest.truncate() - except IOError: - raise ConvertError("Unable to read/write '{0}'.".format(path)) + except OSError: + raise ConvertError(f"Unable to read/write '{path}'.") # Make sure the script has correct execute permissions try: path.chmod(0o755) - except IOError: + except OSError: raise tmt.convert.ConvertError( - "Could not make '{0}' executable.".format(path)) + f"Could not make '{path}' executable.") def write(path: Path, data: NitrateDataType, quiet: bool = False) -> None: @@ -1119,21 +1110,20 @@ def write(path: Path, data: NitrateDataType, quiet: bool = False) -> None: 'adjust', 'extra-nitrate', 'extra-summary', 'extra-task', 'extra-hardware', 'extra-pepa'] - sorted_data = dict() + sorted_data = {} for key in tmt.base.Test._keys() + extra_keys: - try: + with suppress(KeyError): sorted_data[key] = data[key] - except KeyError: - pass + # Store metadata into a fmf file try: with open(path, 'w', encoding='utf-8') as fmf_file: fmf_file.write(tmt.utils.dict_to_yaml(sorted_data)) - except IOError: - raise ConvertError("Unable to write '{0}'".format(path)) + except OSError: + raise ConvertError(f"Unable to write '{path}'") if not quiet: echo(style( - "Metadata successfully stored into '{0}'.".format(path), fg='magenta')) + f"Metadata successfully stored into '{path}'.", fg='magenta')) def relevancy_to_adjust( @@ -1144,8 +1134,8 @@ def relevancy_to_adjust( Expects a string or list of strings with relevancy rules. Returns a list of dictionaries with adjust rules. """ - rules = list() - rule = dict() + rules = [] + rule = {} if isinstance(relevancy, list): relevancy = '\n'.join(str(line) for line in relevancy) @@ -1177,7 +1167,7 @@ def relevancy_to_adjust( f"Invalid test case relevancy decision '{decision}'.") # Adjust condition syntax - expressions = list() + expressions = [] for expression in re.split(r'\s*&&?\s*', condition): search_result = re.search(RELEVANCY_EXPRESSION, expression) if search_result is None: @@ -1220,6 +1210,6 @@ def relevancy_to_adjust( rule['when'] = ' and '.join(expressions) rule['continue'] = False rules.append(rule) - rule = dict() + rule = {} return rules diff --git a/tmt/export/__init__.py b/tmt/export/__init__.py index c2666a6dc2..729dc74415 100644 --- a/tmt/export/__init__.py +++ b/tmt/export/__init__.py @@ -1,5 +1,3 @@ -# coding: utf-8 - """ Metadata export functionality core. @@ -12,8 +10,21 @@ import traceback import types import xmlrpc.client -from typing import (TYPE_CHECKING, Any, Callable, ClassVar, Dict, Generic, - List, Optional, Tuple, Type, TypeVar, Union, cast) +from typing import ( + TYPE_CHECKING, + Any, + Callable, + ClassVar, + Dict, + Generic, + List, + Optional, + Tuple, + Type, + TypeVar, + Union, + cast, + ) if sys.version_info >= (3, 8): from typing import Protocol @@ -139,7 +150,7 @@ def _export(self, *, keys: Optional[List[str]] = None) -> _RawExportedInstance: position. """ - raise NotImplementedError() + raise NotImplementedError def export(self, *, format: str, keys: Optional[List[str]] = None, **kwargs: Any) -> str: """ Export this instance in a given format """ @@ -180,7 +191,7 @@ class ExportPlugin: @classmethod def export_fmfid_collection(cls, fmf_ids: List['tmt.base.FmfId'], **kwargs: Any) -> str: """ Export collection of fmf ids """ - raise NotImplementedError() + raise NotImplementedError @classmethod def export_test_collection(cls, @@ -188,7 +199,7 @@ def export_test_collection(cls, keys: Optional[List[str]] = None, **kwargs: Any) -> str: """ Export collection of tests """ - raise NotImplementedError() + raise NotImplementedError @classmethod def export_plan_collection(cls, @@ -196,7 +207,7 @@ def export_plan_collection(cls, keys: Optional[List[str]] = None, **kwargs: Any) -> str: """ Export collection of plans """ - raise NotImplementedError() + raise NotImplementedError @classmethod def export_story_collection(cls, @@ -204,7 +215,7 @@ def export_story_collection(cls, keys: Optional[List[str]] = None, **kwargs: Any) -> str: """ Export collection of stories """ - raise NotImplementedError() + raise NotImplementedError # It's tempting to make this the default implementation of `ExporterPlugin` class, # but that would mean the `ExportPlugin` would suddenly not raise `NotImplementedError` @@ -415,8 +426,7 @@ def required_section_exists( warnings_list.append( warn_required_section_is_absent.format(section_name)) return 0 - else: - return len(res) + return len(res) # Required sections don't exist if not required_section_exists(html_headings_from_file, diff --git a/tmt/export/nitrate.py b/tmt/export/nitrate.py index 31c128730e..8acc6fb613 100644 --- a/tmt/export/nitrate.py +++ b/tmt/export/nitrate.py @@ -2,9 +2,9 @@ import os import re import types +from contextlib import suppress from functools import lru_cache -from typing import (TYPE_CHECKING, Any, Dict, Generator, List, Optional, Tuple, - Union, cast) +from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional, Tuple, Union, cast import fmf.context from click import echo, style @@ -113,13 +113,13 @@ def convert_manual_to_nitrate(test_md: Path) -> SectionsReturnType: html = tmt.utils.markdown_to_html(test_md) html_splitlines = html.splitlines() - for key in sections_headings.keys(): + for key in sections_headings: result: HeadingsType = [] i = 0 while html_splitlines: try: if re.search("^" + key + "$", html_splitlines[i]): - html_content = str() + html_content = '' if key.startswith('

Test'): html_content = html_splitlines[i].\ replace('

', '').\ @@ -143,7 +143,7 @@ def convert_manual_to_nitrate(test_md: Path) -> SectionsReturnType: break def concatenate_headings_content(headings: Tuple[str, ...]) -> HeadingsType: - content = list() + content = [] for v in headings: content += sections_headings[v] return content @@ -249,12 +249,12 @@ def return_markdown_file() -> Optional[Path]: if len(md_files) == 1: return Path.cwd() / str(md_files[0]) if not md_files: - echo((style(f'Markdown file doesn\'t exist {fail_message}', - fg='yellow'))) + echo(style(f'Markdown file doesn\'t exist {fail_message}', + fg='yellow')) return None - echo((style(f'{len(md_files)} Markdown files found {fail_message}', - fg='yellow'))) + echo(style(f'{len(md_files)} Markdown files found {fail_message}', + fg='yellow')) return None @@ -269,7 +269,7 @@ def get_category(path: Path) -> str: if category_search: category = category_search.group(1) # Default to 'Sanity' if Makefile or Type not found - except (IOError, AttributeError): + except (OSError, AttributeError): pass return category @@ -389,7 +389,7 @@ def export_to_nitrate(test: 'tmt.Test') -> None: try: nitrate_id = test.node.get('extra-nitrate')[3:] nitrate_case: NitrateTestCase = nitrate.TestCase(int(nitrate_id)) - nitrate_case.summary # Make sure we connect to the server now + nitrate_case.summary # noqa: B018 - Make sure we connect to the server now echo(style(f"Test case '{nitrate_case.identifier}' found.", fg='blue')) except TypeError: # Create a new nitrate test case @@ -398,11 +398,8 @@ def export_to_nitrate(test: 'tmt.Test') -> None: # Check for existing Nitrate tests with the same fmf id if not duplicate: testcases = _nitrate_find_fmf_testcases(test) - try: - # Select the first found testcase if any + with suppress(StopIteration): nitrate_case = next(testcases) - except StopIteration: - pass if not nitrate_case: # Summary for TCMS case extra_summary = prepare_extra_summary(test, append_summary) @@ -486,7 +483,7 @@ def export_to_nitrate(test: 'tmt.Test') -> None: # Remove unexpected general plans if general and nitrate_case: # Remove also all general plans linked to testcase - for nitrate_plan in [plan for plan in nitrate_case.testplans]: + for nitrate_plan in list(nitrate_case.testplans): if (nitrate_plan.type.name == "General" and nitrate_plan not in expected_general_plans): echo(style( @@ -572,10 +569,8 @@ def export_to_nitrate(test: 'tmt.Test') -> None: } for section, attribute in section_to_attr.items(): if attribute is None: - try: + with suppress(tmt.utils.StructuredFieldError): struct_field.remove(section) - except tmt.utils.StructuredFieldError: - pass else: struct_field.set(section, attribute) echo(style(section + ': ', fg='green') + attribute.strip()) @@ -640,13 +635,12 @@ def export_to_nitrate(test: 'tmt.Test') -> None: # Update nitrate test case if not dry_mode: nitrate_case.update() - echo(style("Test case '{0}' successfully exported to nitrate.".format( + echo(style("Test case '{}' successfully exported to nitrate.".format( nitrate_case.identifier), fg='magenta')) # Optionally link Bugzilla to Nitrate case - if link_bugzilla and verifies_bug_ids: - if not dry_mode: - tmt.export.bz_set_coverage(verifies_bug_ids, nitrate_case.id, NITRATE_TRACKER_ID) + if link_bugzilla and verifies_bug_ids and not dry_mode: + tmt.export.bz_set_coverage(verifies_bug_ids, nitrate_case.id, NITRATE_TRACKER_ID) @tmt.base.Test.provides_export('nitrate') diff --git a/tmt/export/polarion.py b/tmt/export/polarion.py index 751e618f37..1f6387998d 100644 --- a/tmt/export/polarion.py +++ b/tmt/export/polarion.py @@ -20,7 +20,7 @@ POLARION_TRACKER_ID = 117 # ID of polarion in RH's bugzilla RE_POLARION_URL = r'.*/polarion/#/project/.*/workitem\?id=(.*)' -LEGACY_POLARION_PROJECTS = set(['RedHatEnterpriseLinux7']) +LEGACY_POLARION_PROJECTS = {'RedHatEnterpriseLinux7'} # TODO: why this exists? log = fmf.utils.Logging('tmt').logger @@ -128,7 +128,7 @@ def get_polarion_case( polarion_case = PolarionTestCase( project_id=project_id, work_item_id=case_id) echo(style( - f"Test case '{str(polarion_case.work_item_id)}' found.", + f"Test case '{polarion_case.work_item_id!s}' found.", fg='blue')) return polarion_case except PolarionException: @@ -301,7 +301,7 @@ def export_to_polarion(test: tmt.base.Test) -> None: tmt.convert.add_link( f'{server_url}{"" if server_url.endswith("/") else "/"}' f'#/project/{polarion_case.project_id}/workitem?id=' - f'{str(polarion_case.work_item_id)}', + f'{polarion_case.work_item_id!s}', data, system=tmt.convert.SYSTEM_OTHER, type_='implements') # List of bugs test verifies @@ -352,7 +352,7 @@ def export_to_polarion(test: tmt.base.Test) -> None: # Optionally link Bugzilla to Polarion case if link_bugzilla and bug_ids and not dry_mode: case_id = (f"{polarion_case.project_id}/workitem?id=" - f"{str(polarion_case.work_item_id)}") + f"{polarion_case.work_item_id!s}") tmt.export.bz_set_coverage(bug_ids, case_id, POLARION_TRACKER_ID) diff --git a/tmt/identifier.py b/tmt/identifier.py index 3fd7871c9b..7c6cedf8b3 100644 --- a/tmt/identifier.py +++ b/tmt/identifier.py @@ -29,8 +29,7 @@ def locate_key(node: fmf.Tree, key: str) -> Optional[fmf.Tree]: # Return node only if the key is defined if node.get(key) is None: return None - else: - return node + return node def key_defined_in_leaf(node: fmf.Tree, key: str) -> bool: diff --git a/tmt/libraries/__init__.py b/tmt/libraries/__init__.py index 0b21dc7f33..878c2d2f41 100644 --- a/tmt/libraries/__init__.py +++ b/tmt/libraries/__init__.py @@ -8,8 +8,7 @@ import tmt.base import tmt.log import tmt.utils -from tmt.base import (Dependency, DependencyFile, DependencyFmfId, - DependencySimple) +from tmt.base import Dependency, DependencyFile, DependencyFmfId, DependencySimple from tmt.utils import Path # A beakerlib identifier type, can be a string or a fmf id (with extra beakerlib keys) diff --git a/tmt/libraries/beakerlib.py b/tmt/libraries/beakerlib.py index c7c27c0647..643fc998a3 100644 --- a/tmt/libraries/beakerlib.py +++ b/tmt/libraries/beakerlib.py @@ -162,7 +162,7 @@ def _library_cache(self) -> Dict[str, 'BeakerLib']: # Initialize library cache (indexed by the repository name) # FIXME: cast() - https://github.com/teemtee/tmt/issues/1372 if not hasattr(self.parent, '_library_cache'): - cast(CommonWithLibraryCache, self.parent)._library_cache = dict() + cast(CommonWithLibraryCache, self.parent)._library_cache = {} return cast(CommonWithLibraryCache, self.parent)._library_cache diff --git a/tmt/lint.py b/tmt/lint.py index 7d0931daec..3db4f0834b 100644 --- a/tmt/lint.py +++ b/tmt/lint.py @@ -68,8 +68,19 @@ def lint_manual_valid_markdown(self) -> LinterReturn: import enum import re import textwrap -from typing import (TYPE_CHECKING, Any, Callable, ClassVar, Generator, Generic, - Iterable, List, Optional, Tuple, TypeVar) +from typing import ( + TYPE_CHECKING, + Any, + Callable, + ClassVar, + Generator, + Generic, + Iterable, + List, + Optional, + Tuple, + TypeVar, + ) from click import style @@ -380,11 +391,7 @@ def format_rulings(rulings: Iterable[LinterRuling]) -> Generator[str, None, None _OUTCOME_TO_MARK[eventual_outcome], fg=_OUTCOME_TO_COLOR[eventual_outcome]) - if actual_outcome == eventual_outcome: - eventual = ' ' * 8 - - else: - eventual = f' -> {eventual_status}' + eventual = ' ' * 8 if actual_outcome == eventual_outcome else f' -> {eventual_status}' else: eventual = '' diff --git a/tmt/log.py b/tmt/log.py index 0067b717d4..4c45354606 100644 --- a/tmt/log.py +++ b/tmt/log.py @@ -181,10 +181,7 @@ def indent( key = click.style(key, fg=color) # Prepare prefix if labels provided - if labels: - prefix = render_labels(labels).ljust(labels_padding) + ' ' - else: - prefix = '' + prefix = render_labels(labels).ljust(labels_padding) + ' ' if labels else '' # Handle key only if value is None: @@ -290,7 +287,9 @@ def format(self, record: logging.LogRecord) -> str: # Original code from Formatter.format() - hard to inherit when overriding # Formatter.format()... s = self._decolorize(self.formatMessage(record)) - if record.exc_info: + # SIM102: Use a single `if` statement instead of nested `if` statements. Keeping for + # readability. + if record.exc_info: # noqa: SIM102 # Cache the traceback text to avoid converting it multiple times # (it's constant anyway) if not record.exc_text: @@ -331,7 +330,7 @@ def filter(self, record: logging.LogRecord) -> bool: if message_verbosity_level is None: return True - return True if details['logger_verbosity_level'] >= message_verbosity_level else False + return details['logger_verbosity_level'] >= message_verbosity_level class DebugLevelFilter(logging.Filter): @@ -349,7 +348,7 @@ def filter(self, record: logging.LogRecord) -> bool: if message_debug_level is None: return True - return True if details['logger_debug_level'] >= message_debug_level else False + return details['logger_debug_level'] >= message_debug_level class QuietnessFilter(logging.Filter): @@ -563,10 +562,7 @@ def apply_verbosity_options(self, **kwargs: Any) -> 'Logger': """ verbosity_level = cast(Optional[int], kwargs.get('verbose', None)) - if verbosity_level is None: - pass - - elif verbosity_level == 0: + if verbosity_level is None or verbosity_level == 0: pass else: @@ -580,10 +576,7 @@ def apply_verbosity_options(self, **kwargs: Any) -> 'Logger': else: debug_level_from_option = cast(Optional[int], kwargs.get('debug', None)) - if debug_level_from_option is None: - pass - - elif debug_level_from_option == 0: + if debug_level_from_option is None or debug_level_from_option == 0: pass else: @@ -667,7 +660,7 @@ def _log( labels=self.labels, labels_padding=self.labels_padding) - self._logger._log(level, message, tuple(), extra={'details': details}) + self._logger._log(level, message, (), extra={'details': details}) def print( self, diff --git a/tmt/options.py b/tmt/options.py index 3873a107cd..bf1782d85f 100644 --- a/tmt/options.py +++ b/tmt/options.py @@ -1,11 +1,9 @@ -# coding: utf-8 - """ Common options and the MethodCommand class """ +import contextlib import re import textwrap -from typing import (TYPE_CHECKING, Any, Callable, Dict, List, NamedTuple, - Optional, Type, Union) +from typing import TYPE_CHECKING, Any, Callable, Dict, List, NamedTuple, Optional, Type, Union import click @@ -140,8 +138,8 @@ def option( FORCE_DRY_OPTIONS: List[ClickOptionDecoratorType] = [ option( '-f', '--force', is_flag=True, - help='Overwrite existing files and step data.') - ] + DRY_OPTIONS + help='Overwrite existing files and step data.'), + *DRY_OPTIONS] # Fix action @@ -448,10 +446,8 @@ def _find_how(args: List[str]) -> Optional[str]: return None - try: + with contextlib.suppress(IndexError): how = _find_how(args[:]) - except IndexError: - pass # Find method with the first matching prefix if how is not None: diff --git a/tmt/plugins/__init__.py b/tmt/plugins/__init__.py index 74ec2cb961..15dfdd3135 100644 --- a/tmt/plugins/__init__.py +++ b/tmt/plugins/__init__.py @@ -1,13 +1,10 @@ -# coding: utf-8 - """ Handle Plugins """ import importlib import os import pkgutil import sys -from typing import (Any, Dict, Generator, Generic, List, Optional, Tuple, - TypeVar) +from typing import Any, Dict, Generator, Generic, List, Optional, Tuple, TypeVar if sys.version_info < (3, 9): from importlib_metadata import entry_points diff --git a/tmt/queue.py b/tmt/queue.py index 971e9dcd64..6ad763c85f 100644 --- a/tmt/queue.py +++ b/tmt/queue.py @@ -1,7 +1,6 @@ import dataclasses from concurrent.futures import Future, ThreadPoolExecutor, as_completed -from typing import (TYPE_CHECKING, Dict, Generator, Generic, List, Optional, - TypeVar) +from typing import TYPE_CHECKING, Dict, Generator, Generic, List, Optional, TypeVar import fmf.utils diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index a99b57ba82..e2c6adb2cf 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -7,9 +7,22 @@ import shutil import sys import textwrap -from typing import (TYPE_CHECKING, Any, Callable, DefaultDict, Dict, Generator, - List, Optional, Tuple, Type, TypeVar, Union, cast, - overload) +from typing import ( + TYPE_CHECKING, + Any, + Callable, + DefaultDict, + Dict, + Generator, + List, + Optional, + Tuple, + Type, + TypeVar, + Union, + cast, + overload, + ) if sys.version_info >= (3, 8): from typing import TypedDict @@ -358,7 +371,7 @@ def status(self, status: Optional[str] = None) -> Optional[str]: if status not in ['todo', 'done']: raise tmt.utils.GeneralError(f"Invalid status '{status}'.") # Show status only if changed - elif self._status != status: + if self._status != status: self._status = status self.debug('status', status, color='yellow', level=2) # Return status @@ -595,7 +608,7 @@ def __init__( ) -> None: """ Store method data """ - doc = (doc or getattr(class_, '__doc__') or '').strip() + doc = (doc or class_.__doc__ or '').strip() if not doc: if class_: @@ -1142,7 +1155,7 @@ def phases(cls, step: Step) -> List[int]: @classmethod def _parse_phases(cls, step: Step) -> Dict[str, List[int]]: """ Parse options and store phase order """ - phases = dict() + phases = {} options: List[str] = cls._opt('step', default=[]) # Use the end of the last enabled step if no --step given @@ -1182,7 +1195,7 @@ def _parse_phases(cls, step: Step) -> Dict[str, List[int]]: # Convert 'start' and 'end' aliases try: phase = cast(Dict[str, int], - dict(start=PHASE_START, end=PHASE_END))[phase] + {'start': PHASE_START, 'end': PHASE_END})[phase] except KeyError: raise tmt.utils.GeneralError(f"Invalid phase '{phase}'.") # Store the phase for given step @@ -1193,7 +1206,7 @@ def _parse_phases(cls, step: Step) -> Dict[str, List[int]]: return phases def go(self) -> None: - raise NotImplementedError() + raise NotImplementedError class Reboot(Action): @@ -1239,7 +1252,8 @@ def go(self) -> None: """ Reboot the guest(s) """ self.info('reboot', 'Rebooting guest', color='yellow') assert isinstance(self.parent, Step) - assert hasattr(self.parent, 'plan') and self.parent.plan is not None + assert hasattr(self.parent, 'plan') + assert self.parent.plan is not None for guest in self.parent.plan.provision.guests(): guest.reboot(hard=self.opt('hard')) self.info('reboot', 'Reboot finished', color='yellow') diff --git a/tmt/steps/discover/__init__.py b/tmt/steps/discover/__init__.py index 99f5cda4bd..b543de7220 100644 --- a/tmt/steps/discover/__init__.py +++ b/tmt/steps/discover/__init__.py @@ -1,6 +1,5 @@ import dataclasses -from typing import (TYPE_CHECKING, Any, Dict, Generator, List, Optional, Type, - cast) +from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional, Type, cast import click from fmf.utils import listed @@ -9,8 +8,8 @@ if TYPE_CHECKING: import tmt.cli - import tmt.steps import tmt.options + import tmt.steps import tmt.base import tmt.steps @@ -327,10 +326,7 @@ def _iter_phase_tests() -> Generator['tmt.Test', None, None]: yield from self._tests[phase_name] - if phase_name is None: - iterator = _iter_all_tests - else: - iterator = _iter_phase_tests + iterator = _iter_all_tests if phase_name is None else _iter_phase_tests if enabled is None: return list(iterator()) diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index c88ed042ac..ed127f165e 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -225,7 +225,7 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor 'supported. Defaults to the top fmf root if it is ' 'present, otherwise top directory (shortcut "/").' ), - ] + super().options(how) + *super().options(how)] @property def is_in_standalone_mode(self) -> bool: @@ -236,7 +236,7 @@ def is_in_standalone_mode(self) -> bool: def go(self) -> None: """ Discover available tests """ - super(DiscoverFmf, self).go() + super().go() # Check url and path, prepare test directory url = self.get('url') @@ -510,7 +510,7 @@ def assert_git_url(plan_name: Optional[str] = None) -> None: ), cwd=self.testdir)[0] if output: directories = [os.path.dirname(name) for name in output.split('\n')] - modified = set(f"^/{re.escape(name)}" for name in directories if name) + modified = {f"^/{re.escape(name)}" for name in directories if name} self.debug(f"Limit to modified test dirs: {modified}", level=3) names.extend(modified) diff --git a/tmt/steps/discover/shell.py b/tmt/steps/discover/shell.py index 35d1468570..b0137cf378 100644 --- a/tmt/steps/discover/shell.py +++ b/tmt/steps/discover/shell.py @@ -280,8 +280,8 @@ def fetch_remote_repository( def go(self) -> None: """ Discover available tests """ - super(DiscoverShell, self).go() - tests = fmf.Tree(dict(summary='tests')) + super().go() + tests = fmf.Tree({'summary': 'tests'}) assert self.workdir is not None testdir = self.workdir / "tests" diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 2b7024834a..a8c35549f3 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -177,7 +177,7 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor option( '-x', '--exit-first', is_flag=True, help='Stop execution after the first test failure.'), - ] + super().options(how) + *super().options(how)] def go( self, @@ -253,7 +253,7 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: for script in self.scripts: source = SCRIPTS_SRC_DIR / script.path.name - for dest in [script.path] + script.aliases: + for dest in [script.path, *script.aliases]: guest.push( source=source, destination=dest, @@ -573,7 +573,7 @@ def wake(self) -> None: # There should be just a single definition if len(self.data) > 1: raise tmt.utils.SpecificationError( - "Multiple execute steps defined in '{}'.".format(self.plan)) + f"Multiple execute steps defined in '{self.plan}'.") # Choose the right plugin and wake it up # FIXME: cast() - see https://github.com/teemtee/tmt/issues/1599 diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index a0568d3240..9c0b5697f0 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -3,6 +3,7 @@ import json import os import sys +from contextlib import suppress from typing import Any, Dict, List, Optional, cast import click @@ -17,8 +18,12 @@ from tmt.base import Test from tmt.options import option from tmt.result import Result, ResultOutcome -from tmt.steps.execute import (SCRIPTS, TEST_OUTPUT_FILENAME, - TMT_FILE_SUBMIT_SCRIPT, TMT_REBOOT_SCRIPT) +from tmt.steps.execute import ( + SCRIPTS, + TEST_OUTPUT_FILENAME, + TMT_FILE_SUBMIT_SCRIPT, + TMT_REBOOT_SCRIPT, + ) from tmt.steps.provision import Guest from tmt.utils import EnvironmentType, Path, ShellScript, field @@ -79,8 +84,8 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor # Disable interactive progress bar option( '--no-progress-bar', is_flag=True, - help='Disable interactive progress bar showing the current test.') - ] + super().options(how) + help='Disable interactive progress bar showing the current test.'), + *super().options(how)] # TODO: consider switching to utils.updatable_message() - might need more # work, since use of _show_progress is split over several methods. @@ -283,11 +288,10 @@ def check(self, test: Test, guest: Guest) -> List[Result]: return self.check_custom_results(test, guest) if test.framework == 'beakerlib': return self.check_beakerlib(test, guest) - else: - try: - return self.check_result_file(test, guest) - except tmt.utils.FileError: - return self.check_shell(test, guest) + try: + return self.check_result_file(test, guest) + except tmt.utils.FileError: + return self.check_shell(test, guest) def _will_reboot(self, test: Test, guest: Guest) -> bool: """ True if reboot is requested """ @@ -314,14 +318,13 @@ def _handle_reboot(self, test: Test, guest: Guest) -> bool: f"with reboot count {test._reboot_count}.") reboot_request_path = self._reboot_request_path(test, guest) test_data = self.data_path(test, guest, full=True) / tmt.steps.execute.TEST_DATA - with open(reboot_request_path, 'r') as reboot_file: + with open(reboot_request_path) as reboot_file: reboot_data = json.loads(reboot_file.read()) reboot_command = None if reboot_data.get('command'): - try: + with suppress(TypeError): reboot_command = ShellScript(reboot_data.get('command')) - except TypeError: - pass + try: timeout = int(reboot_data.get('timeout')) except ValueError: diff --git a/tmt/steps/execute/upgrade.py b/tmt/steps/execute/upgrade.py index 846252f719..2a371b39d7 100644 --- a/tmt/steps/execute/upgrade.py +++ b/tmt/steps/execute/upgrade.py @@ -132,8 +132,8 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor tmt.steps.discover.fmf.REF_OPTION, tmt.steps.discover.fmf.TEST_OPTION, tmt.steps.discover.fmf.FILTER_OPTION, - tmt.steps.discover.fmf.EXCLUDE_OPTION - ] + super().options(how) + tmt.steps.discover.fmf.EXCLUDE_OPTION, + *super().options(how)] @property # type:ignore[override] def discover(self) -> Union[ @@ -144,8 +144,7 @@ def discover(self) -> Union[ # discover plugin. if self._discover_upgrade: return self._discover_upgrade - else: - return self.step.plan.discover + return self.step.plan.discover @discover.setter def discover(self, plugin: Optional['tmt.steps.discover.DiscoverPlugin']) -> None: @@ -203,7 +202,7 @@ def _get_plan(self, upgrades_repo: Path) -> tmt.base.Plan: if len(plans) == 0: raise tmt.utils.ExecuteError( f"No matching upgrade path found for '{self.upgrade_path}'.") - elif len(plans) > 1: + if len(plans) > 1: names = [plan.name for plan in plans] raise tmt.utils.ExecuteError( f"Ambiguous upgrade path reference, found plans " diff --git a/tmt/steps/finish/__init__.py b/tmt/steps/finish/__init__.py index 499dcb685d..ecb2cc70b9 100644 --- a/tmt/steps/finish/__init__.py +++ b/tmt/steps/finish/__init__.py @@ -9,8 +9,15 @@ import tmt.steps from tmt.options import option from tmt.plugins import PluginRegistry -from tmt.steps import (Action, Method, PhaseQueue, PullTask, QueuedPhase, - TaskOutcome, sync_with_guests) +from tmt.steps import ( + Action, + Method, + PhaseQueue, + PullTask, + QueuedPhase, + TaskOutcome, + sync_with_guests, + ) from tmt.steps.provision import Guest if TYPE_CHECKING: diff --git a/tmt/steps/prepare/__init__.py b/tmt/steps/prepare/__init__.py index e7ab454f82..e6073fc338 100644 --- a/tmt/steps/prepare/__init__.py +++ b/tmt/steps/prepare/__init__.py @@ -1,8 +1,7 @@ import collections import copy import dataclasses -from typing import (TYPE_CHECKING, Any, DefaultDict, Dict, List, Optional, - Type, cast) +from typing import TYPE_CHECKING, Any, DefaultDict, Dict, List, Optional, Type, cast import click import fmf @@ -16,8 +15,14 @@ from tmt.options import option from tmt.plugins import PluginRegistry from tmt.queue import TaskOutcome -from tmt.steps import (Action, PhaseQueue, PullTask, PushTask, QueuedPhase, - sync_with_guests) +from tmt.steps import ( + Action, + PhaseQueue, + PullTask, + PushTask, + QueuedPhase, + sync_with_guests, + ) from tmt.steps.provision import Guest from tmt.utils import uniq @@ -184,36 +189,36 @@ def go(self) -> None: ]) if requires: - data: _RawPrepareStepData = dict( - how='install', - name='requires', - summary='Install required packages', - order=tmt.utils.DEFAULT_PLUGIN_ORDER_REQUIRES, - package=[ + data: _RawPrepareStepData = { + 'how': 'install', + 'name': 'requires', + 'summary': 'Install required packages', + 'order': tmt.utils.DEFAULT_PLUGIN_ORDER_REQUIRES, + 'package': [ require.to_spec() for require in tmt.base.assert_simple_dependencies( requires, 'After beakerlib processing, tests may have only simple requirements', self._logger) - ]) + ]} self._phases.append(PreparePlugin.delegate(self, raw_data=data)) # Recommended packages recommends = uniq(self.plan.discover.recommends()) if recommends: - data = dict( - how='install', - name='recommends', - summary='Install recommended packages', - order=tmt.utils.DEFAULT_PLUGIN_ORDER_RECOMMENDS, - package=[ + data = { + 'how': 'install', + 'name': 'recommends', + 'summary': 'Install recommended packages', + 'order': tmt.utils.DEFAULT_PLUGIN_ORDER_RECOMMENDS, + 'package': [ recommend.to_spec() for recommend in tmt.base.assert_simple_dependencies( recommends, 'After beakerlib processing, tests may have only simple requirements', self._logger) ], - missing='skip') + 'missing': 'skip'} self._phases.append(PreparePlugin.delegate(self, raw_data=data)) # Prepare guests (including workdir sync) diff --git a/tmt/steps/prepare/ansible.py b/tmt/steps/prepare/ansible.py index ceda96a281..082f527dc5 100644 --- a/tmt/steps/prepare/ansible.py +++ b/tmt/steps/prepare/ansible.py @@ -111,8 +111,7 @@ def go( lowercased_playbook = playbook.lower() playbook_path = Path(playbook) - if lowercased_playbook.startswith( - 'http://') or lowercased_playbook.startswith('https://'): + if lowercased_playbook.startswith(('http://', 'https://')): assert self.step.plan.my_run is not None # narrow type assert self.step.plan.my_run.tree is not None # narrow type assert self.step.plan.my_run.tree.root is not None # narrow type diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index c078ae7517..0ab4eebb3b 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -11,8 +11,20 @@ import subprocess import tempfile from shlex import quote -from typing import (TYPE_CHECKING, Any, Dict, Generator, List, Optional, Tuple, - Type, TypeVar, Union, cast, overload) +from typing import ( + TYPE_CHECKING, + Any, + Dict, + Generator, + List, + Optional, + Tuple, + Type, + TypeVar, + Union, + cast, + overload, + ) import click import fmf @@ -225,13 +237,13 @@ def _query( output = self._execute(guest, command) if not output or not output.stdout: - guest.debug('query', f"Command '{str(command)}' produced no usable output.") + guest.debug('query', f"Command '{command!s}' produced no usable output.") continue match = re.search(pattern, output.stdout) if not match: - guest.debug('query', f"Command '{str(command)}' produced no usable output.") + guest.debug('query', f"Command '{command!s}' produced no usable output.") continue return match.group(1) @@ -398,7 +410,7 @@ def _tmt_name(self) -> str: assert parent.plan.my_run is not None # narrow type assert parent.plan.my_run.workdir is not None # narrow type run_id = parent.plan.my_run.workdir.name - return self._random_name(prefix="tmt-{0}-".format(run_id[-3:])) + return self._random_name(prefix=f"tmt-{run_id[-3:]}-") @property def multihost_name(self) -> str: @@ -410,12 +422,12 @@ def multihost_name(self) -> str: def is_ready(self) -> bool: """ Detect guest is ready or not """ - raise NotImplementedError() + raise NotImplementedError @classmethod def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecoratorType]: """ Prepare command line options related to guests """ - return list() + return [] def load(self, data: GuestData) -> None: """ @@ -521,16 +533,14 @@ def _ansible_verbosity(self) -> List[str]: """ Prepare verbose level based on the --debug option count """ if self.opt('debug') < 3: return [] - else: - return ['-' + (self.opt('debug') - 2) * 'v'] + return ['-' + (self.opt('debug') - 2) * 'v'] @staticmethod def _ansible_extra_args(extra_args: Optional[str]) -> List[str]: """ Prepare extra arguments for ansible-playbook""" if extra_args is None: return [] - else: - return shlex.split(str(extra_args)) + return shlex.split(str(extra_args)) def _ansible_summary(self, output: Optional[str]) -> None: """ Check the output for ansible result summary numbers """ @@ -563,8 +573,8 @@ def _prepare_environment( """ Prepare dict of environment variables """ # Prepare environment variables so they can be correctly passed # to shell. Create a copy to prevent modifying source. - environment: tmt.utils.EnvironmentType = dict() - environment.update(execute_environment or dict()) + environment: tmt.utils.EnvironmentType = {} + environment.update(execute_environment or {}) # Plan environment and variables provided on the command line # override environment provided to execute(). # FIXME: cast() - https://github.com/teemtee/tmt/issues/1372 @@ -585,7 +595,7 @@ def _export_environment(environment: tmt.utils.EnvironmentType) -> List[ShellScr def ansible(self, playbook: Path, extra_args: Optional[str] = None) -> None: """ Prepare guest using ansible playbook """ - raise NotImplementedError() + raise NotImplementedError @overload def execute(self, @@ -632,7 +642,7 @@ def execute(self, :param friendly_command: nice, human-friendly representation of the command. """ - raise NotImplementedError() + raise NotImplementedError def push(self, source: Optional[Path] = None, @@ -643,7 +653,7 @@ def push(self, Push files to the guest """ - raise NotImplementedError() + raise NotImplementedError def pull(self, source: Optional[Path] = None, @@ -654,7 +664,7 @@ def pull(self, Pull files from the guest """ - raise NotImplementedError() + raise NotImplementedError def stop(self) -> None: """ @@ -665,7 +675,7 @@ def stop(self) -> None: necessary to store the instance status to disk. """ - raise NotImplementedError() + raise NotImplementedError def reboot( self, @@ -686,7 +696,7 @@ def reboot( wait for the guest to come back up after rebooting. """ - raise NotImplementedError() + raise NotImplementedError def reconnect( self, @@ -711,7 +721,7 @@ def try_whoami() -> None: self.execute(Command('whoami'), silent=True) except tmt.utils.RunError: - raise tmt.utils.WaitingIncomplete() + raise tmt.utils.WaitingIncompleteError try: tmt.utils.wait( @@ -843,10 +853,7 @@ def _ssh_socket(self) -> Path: if not self._ssh_socket_path: # Use '/run/user/uid' if it exists, '/tmp' otherwise run_dir = Path(f"/run/user/{os.getuid()}") - if run_dir.is_dir(): - socket_dir = run_dir / "tmt" - else: - socket_dir = Path("/tmp") + socket_dir = run_dir / "tmt" if run_dir.is_dir() else Path("/tmp") socket_dir.mkdir(exist_ok=True) self._ssh_socket_path = Path(tempfile.mktemp(dir=socket_dir)) return self._ssh_socket_path @@ -909,11 +916,11 @@ def _ssh_command(self) -> Command: @classmethod def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecoratorType]: """ Prepare command line options related to SSH-capable guests """ - return super().options(how=how) + [ - option('--ssh-option', metavar="OPTION", multiple=True, default=[], - help="Specify additional SSH option. " - "Value is passed to SSH's -o option, see ssh_config(5) for " - "supported options. Can be specified multiple times.")] + return [*super().options(how=how), + option('--ssh-option', metavar="OPTION", multiple=True, default=[], + help="Specify additional SSH option. " + "Value is passed to SSH's -o option, see ssh_config(5) for " + "supported options. Can be specified multiple times.")] def ansible(self, playbook: Path, extra_args: Optional[str] = None) -> None: """ Prepare guest using ansible playbook """ @@ -965,9 +972,8 @@ def execute(self, """ # Abort if guest is unavailable - if self.guest is None: - if not self.opt('dry'): - raise tmt.utils.GeneralError('The guest is not available.') + if self.guest is None and not self.opt('dry'): + raise tmt.utils.GeneralError('The guest is not available.') ssh_command: tmt.utils.Command = self._ssh_command() @@ -1038,9 +1044,8 @@ def push(self, sudo on the Guest (e.g. pushing to r/o destination) """ # Abort if guest is unavailable - if self.guest is None: - if not self.opt('dry'): - raise tmt.utils.GeneralError('The guest is not available.') + if self.guest is None and not self.opt('dry'): + raise tmt.utils.GeneralError('The guest is not available.') # Prepare options and the push command options = options or DEFAULT_RSYNC_PUSH_OPTIONS @@ -1106,9 +1111,8 @@ def pull(self, and 'extend_options' to extend them (e.g. by exclude). """ # Abort if guest is unavailable - if self.guest is None: - if not self.opt('dry'): - raise tmt.utils.GeneralError('The guest is not available.') + if self.guest is None and not self.opt('dry'): + raise tmt.utils.GeneralError('The guest is not available.') # Prepare options and the pull command options = options or DEFAULT_RSYNC_PULL_OPTIONS @@ -1248,11 +1252,11 @@ def check_boot_time() -> None: return # Same boot time, reboot didn't happen yet, retrying - raise tmt.utils.WaitingIncomplete() + raise tmt.utils.WaitingIncompleteError except tmt.utils.RunError: self.debug('Failed to connect to the guest.') - raise tmt.utils.WaitingIncomplete() + raise tmt.utils.WaitingIncompleteError timeout = timeout or CONNECTION_TIMEOUT @@ -1404,7 +1408,7 @@ def guest(self) -> Optional[Guest]: Each ProvisionPlugin has to implement this method. Should return a provisioned Guest() instance. """ - raise NotImplementedError() + raise NotImplementedError def requires(self) -> List['tmt.base.Dependency']: """ diff --git a/tmt/steps/provision/artemis.py b/tmt/steps/provision/artemis.py index 9d2248933d..ba11baefc4 100644 --- a/tmt/steps/provision/artemis.py +++ b/tmt/steps/provision/artemis.py @@ -118,12 +118,9 @@ class ProvisionArtemisData(ArtemisGuestData, tmt.steps.provision.ProvisionStepDa # Type annotation for Artemis API `GET /guests/$guestname` response. # Partial, not all fields necessary since plugin ignores most of them. -GuestInspectType = TypedDict( - 'GuestInspectType', { - 'state': str, - 'address': Optional[str] - } - ) +class GuestInspectType(TypedDict): + state: str + address: Optional[str] class ArtemisAPI: @@ -374,7 +371,7 @@ def get_new_state() -> GuestInspectType: if state == 'ready': return current - raise tmt.utils.WaitingIncomplete() + raise tmt.utils.WaitingIncompleteError try: guest_info = tmt.utils.wait( @@ -561,7 +558,7 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor '--watchdog-period-delay', metavar='SECONDS', help='How often (seconds) check that the guest "is-alive".' ), - ] + super().options(how) + *super().options(how)] def go(self) -> None: """ Provision the guest """ diff --git a/tmt/steps/provision/connect.py b/tmt/steps/provision/connect.py index b09134db6c..3f762159f7 100644 --- a/tmt/steps/provision/connect.py +++ b/tmt/steps/provision/connect.py @@ -76,7 +76,7 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor option( '-p', '--password', metavar='PASSWORD', help='Password for login into the guest system.'), - ] + super().options(how) + *super().options(how)] def go(self) -> None: """ Prepare the connection """ diff --git a/tmt/steps/provision/local.py b/tmt/steps/provision/local.py index 8ab825f567..c4ab907470 100644 --- a/tmt/steps/provision/local.py +++ b/tmt/steps/provision/local.py @@ -53,15 +53,11 @@ def execute(self, **kwargs: Any) -> tmt.utils.CommandOutput: """ Execute command on localhost """ # Prepare the environment (plan/cli variables override) - environment: tmt.utils.EnvironmentType = dict() + environment: tmt.utils.EnvironmentType = {} environment.update(env or {}) environment.update(self.parent.plan.environment) - if isinstance(command, Command): - actual_command = command - - else: - actual_command = command.to_shell_command() + actual_command = command if isinstance(command, Command) else command.to_shell_command() if friendly_command is None: friendly_command = str(actual_command) diff --git a/tmt/steps/provision/mrack.py b/tmt/steps/provision/mrack.py index 13a77a2a43..24aa974e99 100644 --- a/tmt/steps/provision/mrack.py +++ b/tmt/steps/provision/mrack.py @@ -117,7 +117,7 @@ def _parse_amount(self, in_string: str) -> Tuple[str, int]: if parts[0]: continue - if any([op in parts[1] for op in op]): + if any(op in parts[1] for op in op): continue result.append(op) @@ -394,8 +394,7 @@ def is_ready(self) -> bool: if state == 'Reserved': return True - else: - return False + return False except mrack.errors.MrackError: return False @@ -458,7 +457,7 @@ def get_new_state() -> GuestInspectType: if state == 'Reserved': return current - raise tmt.utils.WaitingIncomplete() + raise tmt.utils.WaitingIncompleteError try: guest_info = tmt.utils.wait( @@ -545,7 +544,7 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor help=f'How often check Beaker for provisioning status, ' f'{DEFAULT_PROVISION_TICK} seconds by default.', ), - ] + super().options(how) + *super().options(how)] # data argument should be a "Optional[GuestData]" type but we would like to use # BeakerGuestData created here ignoring the override will make mypy calm diff --git a/tmt/steps/provision/podman.py b/tmt/steps/provision/podman.py index 7c3e072484..53212c2c6f 100644 --- a/tmt/steps/provision/podman.py +++ b/tmt/steps/provision/podman.py @@ -285,8 +285,8 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor help='Force pulling a fresh container image.'), option( '-u', '--user', metavar='USER', - help='User to use for all container operations.') - ] + super().options(how) + help='User to use for all container operations.'), + *super().options(how)] def default(self, option: str, default: Any = None) -> Any: """ Return default data for given option """ @@ -306,7 +306,9 @@ def go(self) -> None: # Prepare data for the guest instance data_from_options = { key: self.get(key) - for key in PodmanGuestData.keys() + # SIM118: Use `{key} in {dict}` instead of `{key} in {dict}.keys()`. + # "Type[PodmanGuestData]" has no attribute "__iter__" (not iterable) + for key in PodmanGuestData.keys() # noqa: SIM118 } data = PodmanGuestData(**data_from_options) diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index eb202148f7..209a401d58 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -19,8 +19,7 @@ import tmt.steps.provision import tmt.utils from tmt.options import option -from tmt.utils import (WORKDIR_ROOT, Command, Path, ProvisionError, - ShellScript, retry_session) +from tmt.utils import WORKDIR_ROOT, Command, Path, ProvisionError, ShellScript, retry_session if TYPE_CHECKING: import tmt.base @@ -250,7 +249,7 @@ def is_ready(self) -> bool: # Note the type of variable 'state' is 'Any'. Hence, we don't use: # return state == 'running' # to avoid error from type checking. - return True if state == 'running' else False + return bool(state == "running") except libvirt.libvirtError: return False @@ -268,7 +267,7 @@ def try_get_url() -> requests.Response: except requests.RequestException: pass finally: - raise tmt.utils.WaitingIncomplete() + raise tmt.utils.WaitingIncompleteError try: return tmt.utils.wait( @@ -332,14 +331,14 @@ def prepare_ssh_key(self, key_type: Optional[str] = None) -> None: # Generate new ssh key pair else: self.debug('Generating an ssh key.') - key_name = "id_{}".format(key_type if key_type is not None else 'rsa') + key_name = f"id_{key_type if key_type is not None else 'rsa'}" self.key = [self.workdir / key_name] command = Command("ssh-keygen", "-f", str(self.key[0]), "-N", "") if key_type is not None: command += Command("-t", key_type) self.run(command) self.verbose('key', str(self.key[0]), 'green') - with open(self.workdir / f'{key_name}.pub', 'r') as pubkey_file: + with open(self.workdir / f'{key_name}.pub') as pubkey_file: public_key = pubkey_file.read() # Place public key content into the machine configuration @@ -608,7 +607,7 @@ def options(cls, how: Optional[str] = None) -> List[tmt.options.ClickOptionDecor option( '--list-local-images', is_flag=True, help="List locally available images."), - ] + super().options(how) + *super().options(how)] def go(self) -> None: """ Provision the testcloud instance """ @@ -624,7 +623,9 @@ def go(self) -> None: # Give info about provided data data = TestcloudGuestData(**{ key: self.get(key) - for key in TestcloudGuestData.keys() + # SIM118: Use `{key} in {dict}` instead of `{key} in {dict}.keys()`. + # "Type[TestcloudGuestData]" has no attribute "__iter__" (not iterable) + for key in TestcloudGuestData.keys() # noqa: SIM118 }) # Once plan schema is enforced this won't be necessary diff --git a/tmt/steps/report/polarion.py b/tmt/steps/report/polarion.py index 4304a07f42..a9625f39cd 100644 --- a/tmt/steps/report/polarion.py +++ b/tmt/steps/report/polarion.py @@ -1,8 +1,8 @@ import dataclasses import datetime import os -import xml.etree.ElementTree as ET from typing import Optional +from xml.etree import ElementTree from requests import post @@ -165,7 +165,8 @@ def go(self) -> None: os.getenv( 'TMT_PLUGIN_REPORT_POLARION_TITLE', self.step.plan.name.rsplit('/', 1)[1] + - datetime.datetime.now().strftime("%Y%m%d%H%M%S"))) + # Polarion server running with UTC timezone + datetime.datetime.now(tz=datetime.timezone.utc).strftime("%Y%m%d%H%M%S"))) title = title.replace('-', '_') project_id = self.get('project-id', os.getenv('TMT_PLUGIN_REPORT_POLARION_PROJECT_ID')) upload = self.get('upload') @@ -175,7 +176,7 @@ def go(self) -> None: 'logs', 'compose_id'] junit_suite = make_junit_xml(self) - xml_tree = ET.fromstring(junit_suite.to_xml_string([junit_suite])) + xml_tree = ElementTree.fromstring(junit_suite.to_xml_string([junit_suite])) properties = { 'polarion-project-id': project_id, @@ -191,9 +192,9 @@ def go(self) -> None: if use_facts: properties['polarion_custom_hostname'] = self.step.plan.provision.guests()[0].guest properties['polarion_custom_arch'] = self.step.plan.provision.guests()[0].facts.arch - testsuites_properties = ET.SubElement(xml_tree, 'properties') + testsuites_properties = ElementTree.SubElement(xml_tree, 'properties') for name, value in properties.items(): - ET.SubElement(testsuites_properties, 'property', attrib={ + ElementTree.SubElement(testsuites_properties, 'property', attrib={ 'name': name, 'value': value}) testsuite = xml_tree.find('testsuite') @@ -223,16 +224,16 @@ def go(self) -> None: assert testsuite is not None test_case = testsuite.find(f"*[@name='{result.name}']") assert test_case is not None - properties_elem = ET.SubElement(test_case, 'properties') + properties_elem = ElementTree.SubElement(test_case, 'properties') for name, value in test_properties.items(): - ET.SubElement(properties_elem, 'property', attrib={ + ElementTree.SubElement(properties_elem, 'property', attrib={ 'name': name, 'value': value}) assert self.workdir is not None f_path = self.get("file", self.workdir / DEFAULT_NAME) with open(f_path, 'wb') as fw: - ET.ElementTree(xml_tree).write(fw) + ElementTree.ElementTree(xml_tree).write(fw) if upload: server_url = str(PolarionWorkItem._session._server.url) @@ -245,7 +246,7 @@ def go(self) -> None: response = post( polarion_import_url, auth=auth, - files={'file': ('xunit.xml', ET.tostring(xml_tree))}) + files={'file': ('xunit.xml', ElementTree.tostring(xml_tree))}) self.info( f'Response code is {response.status_code} with text: {response.text}') else: diff --git a/tmt/steps/report/reportportal.py b/tmt/steps/report/reportportal.py index 854aee3da3..664321e142 100644 --- a/tmt/steps/report/reportportal.py +++ b/tmt/steps/report/reportportal.py @@ -113,7 +113,9 @@ def go(self) -> None: # Zip the report bytestream = io.BytesIO() - with zipfile.ZipFile(bytestream, "w", compression=zipfile.ZIP_DEFLATED, + # SIM117: Use a single `with` statement with multiple contexts instead of nested `with` + # statements. Here, `zipstream` is being used in the second nested context. + with zipfile.ZipFile(bytestream, "w", compression=zipfile.ZIP_DEFLATED, # noqa: SIM117 compresslevel=1) as zipstream: # XML file names are irrelevant to ReportPortal with zipstream.open("tests.xml", "w") as entry: @@ -144,7 +146,7 @@ def go(self) -> None: if not response.ok: raise tmt.utils.ReportError( f"Received non-ok status code from ReportPortal, response text is: {message}") - else: - self.debug(f"Response code from the server: {response.status_code}") - self.debug(f"Message from the server: {message}") - self.info("report", "Successfully uploaded.", "yellow") + + self.debug(f"Response code from the server: {response.status_code}") + self.debug(f"Message from the server: {message}") + self.info("report", "Successfully uploaded.", "yellow") diff --git a/tmt/templates.py b/tmt/templates.py index 97e84051be..a4e8b8e5f6 100644 --- a/tmt/templates.py +++ b/tmt/templates.py @@ -1,5 +1,3 @@ -# coding: utf-8 - """ Default Templates """ from typing import Dict @@ -10,8 +8,8 @@ # Test Templates # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -TEST: Dict[str, str] = dict() -TEST_METADATA: Dict[str, str] = dict() +TEST: Dict[str, str] = {} +TEST_METADATA: Dict[str, str] = {} TEST_METADATA['shell'] = """ summary: Concise summary describing what the test does @@ -70,7 +68,7 @@ how: tmt """.lstrip() -PLAN: Dict[str, str] = dict() +PLAN: Dict[str, str] = {} PLAN['mini'] = """ summary: @@ -106,7 +104,7 @@ # Story Templates # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -STORY: Dict[str, str] = dict() +STORY: Dict[str, str] = {} STORY['mini'] = """ story: As a user I want to do this and that. diff --git a/tmt/utils.py b/tmt/utils.py index db96d47093..d510fc5d0b 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -1,4 +1,3 @@ - """ Test Metadata Utilities """ import contextlib @@ -23,11 +22,30 @@ import unicodedata import urllib.parse from collections import OrderedDict +from contextlib import suppress from functools import lru_cache from threading import Thread -from typing import (IO, TYPE_CHECKING, Any, Callable, Dict, Generator, Generic, - Iterable, List, NamedTuple, Optional, Pattern, Sequence, - Tuple, Type, TypeVar, Union, cast, overload) +from typing import ( + IO, + TYPE_CHECKING, + Any, + Callable, + Dict, + Generator, + Generic, + Iterable, + List, + NamedTuple, + Optional, + Pattern, + Sequence, + Tuple, + Type, + TypeVar, + Union, + cast, + overload, + ) import click import fmf @@ -201,10 +219,8 @@ def last_run(self) -> Optional[Path]: def last_run(self, workdir: Path) -> None: """ Set the last run to the given run workdir """ - try: + with suppress(OSError): self._last_run_symlink.unlink() - except OSError: - pass try: self._last_run_symlink.symlink_to(workdir) @@ -458,7 +474,7 @@ def run( # For debugging, we want to save somewhere the actual command rather # than the provided "friendly". Emit the actual command to the debug # log, and the friendly one to the verbose/custom log - logger.debug(f'Run command: {str(self)}', level=2) + logger.debug(f'Run command: {self!s}', level=2) # The friendly command version would be emitted only when we were not # asked to be quiet. @@ -493,7 +509,8 @@ def run( # Run the command in an interactive mode if requested if interactive: - try: + # Interactive mode can return non-zero if the last command failed, ignore errors here. + with suppress(subprocess.CalledProcessError): subprocess.run( self.to_popen(), cwd=cwd, @@ -502,13 +519,7 @@ def run( check=True, executable=executable) - except subprocess.CalledProcessError: - # Interactive mode can return non-zero if the last command - # failed, ignore errors here - pass - - finally: - return CommandOutput(None, None) + return CommandOutput(None, None) # Spawn the child process try: @@ -902,20 +913,18 @@ def opt(self, option: str, default: Optional[Any] = None) -> Any: return parent if parent else local # Special handling for counting options (child overrides the # parent if it was defined) - elif option in ['debug', 'verbose']: + if option in ['debug', 'verbose']: winner = local if local else parent if winner is None: winner = 0 return winner - else: - return parent if parent is not None else local + return parent if parent is not None else local def _level(self) -> int: """ Hierarchy level """ if self.parent is None: return -1 - else: - return self.parent._level() + self._relative_indent + return self.parent._level() + self._relative_indent def _indent( self, @@ -1101,11 +1110,7 @@ def _workdir_init(self, id_: WorkdirArgumentType = None) -> None: # Prepare the workdir name from given id or path if isinstance(id_, Path): # Use provided directory if full path given - if '/' in str(id_): - workdir = id_ - # Construct directory name under workdir root - else: - workdir = workdir_root / id_ + workdir = id_ if '/' in str(id_) else workdir_root / id_ # Resolve any relative paths workdir = workdir.resolve() # Weird workdir id @@ -1128,7 +1133,7 @@ def _check_or_create_workdir_root_with_perms() -> None: # Generated unique id or fail, has to be atomic call for id_bit in range(1, WORKDIR_MAX + 1): - directory = 'run-{}'.format(str(id_bit).rjust(3, '0')) + directory = f"run-{str(id_bit).rjust(3, '0')}" workdir = workdir_root / directory try: # Call is atomic, no race possible @@ -1182,10 +1187,9 @@ def _workdir_load(self, workdir: WorkdirArgumentType) -> None: def _workdir_cleanup(self, path: Optional[Path] = None) -> None: """ Clean up the work directory """ directory = path or self._workdir_name() - if directory is not None: - if directory.is_dir(): - self.debug(f"Clean up workdir '{directory}'.", level=2) - shutil.rmtree(directory) + if directory is not None and directory.is_dir(): + self.debug(f"Clean up workdir '{directory}'.", level=2) + shutil.rmtree(directory) self._workdir = None @property @@ -1335,7 +1339,7 @@ class StructuredFieldError(GeneralError): """ StructuredField parsing error """ -class WaitingIncomplete(GeneralError): +class WaitingIncompleteError(GeneralError): """ Waiting incomplete """ def __init__(self) -> None: @@ -1406,10 +1410,7 @@ def render_run_exception(exception: RunError) -> List[str]: # Check verbosity level used during raising exception, # Supported way to correctly get verbosity is # tmt.util.Common.opt('verbose') - if isinstance(exception.caller, Common): - verbose = exception.caller.opt('verbose') - else: - verbose = 0 + verbose = exception.caller.opt('verbose') if isinstance(exception.caller, Common) else 0 for name, output in (('stdout', exception.stdout), ('stderr', exception.stderr)): if not output: continue @@ -1421,13 +1422,11 @@ def render_run_exception(exception: RunError) -> List[str]: line_summary = f"{min(len(output_lines), OUTPUT_LINES)}/{len(output_lines)}" output_lines = output_lines[-OUTPUT_LINES:] - lines += [ - f'{name} ({line_summary} lines)', - OUTPUT_WIDTH * '~' - ] + output_lines + [ - OUTPUT_WIDTH * '~', - '' - ] + lines += [f'{name} ({line_summary} lines)', + OUTPUT_WIDTH * '~', + *output_lines, + OUTPUT_WIDTH * '~', + ''] return lines @@ -1560,8 +1559,8 @@ def copytree( if not dirs_exist_ok or sys.version_info >= (3, 8): return cast( Path, - shutil.copytree(src=src, dst=dst, symlinks=symlinks, - dirs_exist_ok=dirs_exist_ok)) # type: ignore[call-arg] + shutil.copytree(src=src, dst=dst, symlinks=symlinks, # type: ignore[call-arg] + dirs_exist_ok=dirs_exist_ok)) # Choice was to either copy python implementation and change ONE line # or use rsync (or cp with shell) # We need to copy CONTENT of src into dst @@ -1661,7 +1660,7 @@ def _add_file_vars( f"Invalid variable file specification '{filepath}'.") try: - with open(filepath[1:], 'r') as file: + with open(filepath[1:]) as file: # Handle empty file as an empty environment content = file.read() if not content: @@ -1689,7 +1688,7 @@ def shell_to_dict(variables: Union[str, List[str]]) -> EnvironmentType: """ if not isinstance(variables, (list, tuple)): variables = [variables] - result: EnvironmentType = dict() + result: EnvironmentType = {} for variable in variables: if variable is None: continue @@ -1728,7 +1727,7 @@ def environment_to_dict( if not isinstance(variables, (list, tuple)): variables = [variables] - result: EnvironmentType = dict() + result: EnvironmentType = {} for variable in variables: if variable is None: @@ -1966,7 +1965,7 @@ def yaml_to_dict(data: Any, yaml = YAML(typ=yaml_type) loaded_data = yaml.load(data) if loaded_data is None: - return dict() + return {} if not isinstance(loaded_data, dict): raise GeneralError( f"Expected dictionary in yaml data, " @@ -1984,7 +1983,7 @@ def yaml_to_list(data: Any, raise GeneralError(f"Invalid yaml syntax: {error}") if loaded_data is None: - return list() + return [] if not isinstance(loaded_data, list): raise GeneralError( f"Expected list in yaml data, " @@ -2211,7 +2210,7 @@ def from_spec(cls: Type[SpecBasedContainerT], spec: Any) -> SpecBasedContainerT: See :py:meth:`to_spec` for its counterpart. """ - raise NotImplementedError() + raise NotImplementedError def to_spec(self) -> Dict[str, Any]: """ @@ -2271,8 +2270,9 @@ def extract_from(cls: Type[SerializableContainerDerivedType], """ Extract keys from given object, and save them in a container """ data = cls() - - for key in cls.keys(): + # SIM118: Use `{key} in {dict}` instead of `{key} in {dict}.keys()` + # "NormalizeKeysMixin" has no attribute "__iter__" (not iterable) + for key in cls.keys(): # noqa: SIM118 value = getattr(obj, key) if value is not None: setattr(data, key, value) @@ -2295,7 +2295,7 @@ def to_serialized(self) -> Dict[str, Any]: fields = self.to_dict() - for name in fields.keys(): + for name in fields: field: dataclasses.Field[Any] = dataclass_field_by_name(self, name) serialize_callback = dataclass_field_metadata(field).serialize_callback @@ -2408,13 +2408,13 @@ def markdown_to_html(filename: Path) -> str: raise ConvertError("Install tmt-test-convert to export tests.") try: - with open(filename, 'r') as file: + with open(filename) as file: try: text = file.read() except UnicodeError: raise MetadataError(f"Unable to read '{filename}'.") return markdown.markdown(text) - except IOError: + except OSError: raise ConvertError(f"Unable to open '{filename}'.") @@ -2428,7 +2428,7 @@ def shell_variables( """ # Convert from list/tuple - if isinstance(data, list) or isinstance(data, tuple): + if isinstance(data, (list, tuple)): converted_data = [] for item in data: splitted_item = item.split('=') @@ -2536,7 +2536,7 @@ def format( """ indent_string = (indent + 1) * ' ' # Key - output = '{} '.format(str(key).rjust(indent, ' ')) + output = f"{str(key).rjust(indent, ' ')} " if key_color is not None: output = style(output, fg=key_color) # Bool @@ -2586,17 +2586,16 @@ def create_directory( """ Create a new directory, handle errors """ say = log.debug if quiet else echo if path.is_dir(): - say("Directory '{}' already exists.".format(path)) + say(f"Directory '{path}' already exists.") return if dry: - say("Directory '{}' would be created.".format(path)) + say(f"Directory '{path}' would be created.") return try: path.mkdir(exist_ok=True, parents=True) - say("Directory '{}' created.".format(path)) + say(f"Directory '{path}' created.") except OSError as error: - raise FileError("Failed to create {} '{}' ({})".format( - name, path, error)) from error + raise FileError(f"Failed to create {name} '{path}' ({error})") from error def create_file( @@ -2614,19 +2613,18 @@ def create_file( if force: action = 'would be overwritten' if dry else 'overwritten' else: - raise FileError("File '{}' already exists.".format(path)) + raise FileError(f"File '{path}' already exists.") if dry: - say("{} '{}' {}.".format(name.capitalize(), path, action)) + say(f"{name.capitalize()} '{path}' {action}.") return try: path.write_text(content) - say("{} '{}' {}.".format(name.capitalize(), path, action)) + say(f"{name.capitalize()} '{path}' {action}.") path.chmod(mode) except OSError as error: - raise FileError("Failed to create {} '{}' ({})".format( - name, path, error)) + raise FileError(f"Failed to create {name} '{path}' ({error})") # Avoid multiple subprocess calls for the same url @@ -2808,7 +2806,9 @@ def increment( error = cast(Optional[Exception], kwargs.get('error', None)) # Detect a subset of exception we do not want to follow with a retry. - if error is not None: + # SIM102: Use a single `if` statement instead of nested `if` statements. Keeping for + # readibility. + if error is not None: # noqa: SIM102 # Failed certificate verification - this issue will probably not get any better # should we try again. if isinstance(error, urllib3.exceptions.SSLError) \ @@ -2833,7 +2833,7 @@ def increment( # ignore[type-arg]: base class is a generic class, but we cannot list # its parameter type, because in Python 3.6 the class "is not subscriptable". -class retry_session(contextlib.AbstractContextManager): # type: ignore[type-arg] +class retry_session(contextlib.AbstractContextManager): # type: ignore[type-arg] # noqa: N801 """ Context manager for requests.Session() with retries and timeout """ @@ -2964,7 +2964,7 @@ def parse_yaml(content: str) -> EnvironmentType: yaml_as_dict = YAML(typ="safe").load(content) # Handle empty file as an empty environment if yaml_as_dict is None: - return dict() + return {} if any(isinstance(val, dict) for val in yaml_as_dict.values()): raise GeneralError( "Can't set the environment from the nested yaml config. The " @@ -2985,8 +2985,7 @@ def validate_git_status(test: 'tmt.base.Test') -> Tuple[bool, str]: When all checks pass returns (True, ''). """ - sources = test.node.sources + \ - [os.path.join(test.node.root, '.fmf', 'version')] + sources = [*test.node.sources, os.path.join(test.node.root, '.fmf', 'version')] # Use tmt's run instead of subprocess.run run = Common(logger=test._logger).run @@ -3263,8 +3262,7 @@ def __init__( def __iter__(self) -> Generator[str, None, None]: """ By default iterate through all available section names """ - for section in self._order: - yield section + yield from self._order def __nonzero__(self) -> bool: """ True when any section is defined """ @@ -3314,10 +3312,10 @@ def _load(self, text: str) -> None: # Save header & footer (remove trailing new lines) self._header = re.sub("\n\n$", "\n", matched.groups()[0]) if self._header: - log.debug(u"Parsed header:\n{0}".format(self._header)) + log.debug(f"Parsed header:\n{self._header}") self._footer = re.sub("^\n", "", matched.groups()[2]) if self._footer: - log.debug(u"Parsed footer:\n{0}".format(self._footer)) + log.debug(f"Parsed footer:\n{self._footer}") # Split the content on the section names section = re.compile(r"\n\[([^\]]+)\][ \t]*\n", re.MULTILINE) parts = section.split(matched.groups()[1]) @@ -3329,16 +3327,14 @@ def _load(self, text: str) -> None: "Unable to detect StructuredField version") self.version(int(version_match.groups()[0])) log.debug( - "Detected StructuredField version {0}".format( - self.version())) + f"Detected StructuredField version {self.version()}") # Convert to dictionary, remove escapes and save the order keys = parts[1::2] escape = re.compile(r"^\[structured-field-escape\]", re.MULTILINE) values = [escape.sub("", value) for value in parts[2::2]] for key, value in zip(keys, values): self.set(key, value) - log.debug(u"Parsed sections:\n{0}".format( - pprint.pformat(self._sections))) + log.debug(f"Parsed sections:\n{pprint.pformat(self._sections)}") def _save_version_zero(self) -> str: """ Save version 0 format """ @@ -3346,9 +3342,9 @@ def _save_version_zero(self) -> str: if self._header: result.append(self._header) for section, content in self.iterate(): - result.append(u"[{0}]\n{1}".format(section, content)) + result.append(f"[{section}]\n{content}") if self: - result.append(u"[end]\n") + result.append("[end]\n") if self._footer: result.append(self._footer) return "\n".join(result) @@ -3364,13 +3360,13 @@ def _save(self) -> str: # Sections if self: result.append( - u"[structured-field-start]\n" - u"This is StructuredField version {0}. " - u"Please, edit with care.\n".format(self._version)) + "[structured-field-start]\n" + f"This is StructuredField version {self._version}. " + "Please, edit with care.\n") for section, content in self.iterate(): - result.append(u"[{0}]\n{1}".format(section, escape.sub( + result.append("[{}]\n{}".format(section, escape.sub( "[structured-field-escape]\\1", content))) - result.append(u"[structured-field-end]\n") + result.append("[structured-field-end]\n") # Footer if self._footer: result.append(self._footer) @@ -3388,7 +3384,7 @@ def _read_section(self, content: str) -> Dict[str, SFSectionValueType]: matched = re.search("([^=]+)=(.*)", line) if not matched: raise StructuredFieldError( - "Invalid key/value line: {0}".format(line)) + f"Invalid key/value line: {line}") key = matched.groups()[0].strip() value = matched.groups()[1].strip() # Handle multiple values if enabled @@ -3408,9 +3404,9 @@ def _write_section(self, dictionary: Dict[str, SFSectionValueType]) -> str: for key in dictionary: if isinstance(dictionary[key], list): for value in dictionary[key]: - section += "{0} = {1}\n".format(key, value) + section += f"{key} = {value}\n" else: - section += "{0} = {1}\n".format(key, dictionary[key]) + section += f"{key} = {dictionary[key]}\n" return section # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -3429,7 +3425,7 @@ def version(self, version: Optional[int] = None) -> int: self._version = version else: raise StructuredFieldError( - "Bad StructuredField version: {0}".format(version)) + f"Bad StructuredField version: {version}") return self._version def load(self, text: str, version: Optional[int] = None) -> None: @@ -3447,7 +3443,7 @@ def load(self, text: str, version: Optional[int] = None) -> None: # Make sure the text has a new line at the end if text and text[-1] != "\n": text += "\n" - log.debug(u"Parsing StructuredField\n{0}".format(text)) + log.debug(f"Parsing StructuredField\n{text}") # Parse respective format version if self._version == 0: self._load_version_zero(text) @@ -3458,8 +3454,7 @@ def save(self) -> str: """ Convert the StructuredField into a string """ if self.version() == 0: return self._save_version_zero() - else: - return self._save() + return self._save() def header(self, content: Optional[str] = None) -> str: """ Get or set the header content """ @@ -3486,7 +3481,7 @@ def get( content = self._sections[section] except KeyError: raise StructuredFieldError( - "Section [{0!r}] not found".format(ascii(section))) + f"Section [{ascii(section)!r}] not found") # Return the whole section content if item is None: return content @@ -3495,8 +3490,7 @@ def get( return self._read_section(content)[item] except KeyError: raise StructuredFieldError( - "Unable to read '{0!r}' from section '{1!r}'".format( - ascii(item), ascii(section))) + f"Unable to read '{ascii(item)!r}' from section '{ascii(section)!r}'") def set(self, section: str, content: Any, item: Optional[str] = None) -> None: @@ -3536,7 +3530,7 @@ def remove(self, section: str, item: Optional[str] = None) -> None: del self._order[self._order.index(section)] except KeyError: raise StructuredFieldError( - "Section [{0!r}] not found".format(ascii(section))) + f"Section [{ascii(section)!r}] not found") # Remove only selected item from the section else: try: @@ -3544,7 +3538,7 @@ def remove(self, section: str, item: Optional[str] = None) -> None: del (dictionary[item]) except KeyError: raise StructuredFieldError( - "Unable to remove '{0!r}' from section '{1!r}'".format( + "Unable to remove '{!r}' from section '{!r}'".format( ascii(item), ascii(section))) self._sections[section] = self._write_section(dictionary) @@ -3685,7 +3679,7 @@ def git_clone( # ignore[type-arg]: base class is a generic class, but we cannot list its parameter type, because # in Python 3.6 the class "is not subscriptable". -class updatable_message(contextlib.AbstractContextManager): # type: ignore[type-arg] +class updatable_message(contextlib.AbstractContextManager): # type: ignore[type-arg] # noqa: N801 """ Updatable message suitable for progress-bar-like reporting """ def __init__( @@ -3825,7 +3819,7 @@ def _patch_plan_schema(schema: Schema, store: SchemaStore) -> None: for step in ('discover', 'execute', 'finish', 'prepare', 'provision', 'report'): step_schema_prefix = f'/schemas/{step}/' - step_plugin_schema_ids = [schema_id for schema_id in store.keys() if schema_id.startswith( + step_plugin_schema_ids = [schema_id for schema_id in store if schema_id.startswith( step_schema_prefix) and schema_id not in PLAN_SCHEMA_IGNORED_IDS] refs: List[Schema] = [ @@ -3833,14 +3827,14 @@ def _patch_plan_schema(schema: Schema, store: SchemaStore) -> None: ] schema['properties'][step] = { - 'oneOf': refs + [ - { - 'type': 'array', - 'items': { - 'anyOf': refs - } - } - ] + 'oneOf': [*refs, + { + 'type': 'array', + 'items': { + 'anyOf': refs + } + } + ] } @@ -3856,7 +3850,7 @@ def _load_schema(schema_filepath: Path) -> Schema: / schema_filepath try: - with open(schema_filepath, 'r', encoding='utf-8') as f: + with open(schema_filepath, encoding='utf-8') as f: return cast(Schema, yaml_to_dict(f.read())) except Exception as exc: @@ -4100,9 +4094,9 @@ def wait( if tick <= 0: raise GeneralError('Tick must be a positive integer') - NOW = time.monotonic + monotomic_clock = time.monotonic - deadline = NOW() + timeout.total_seconds() + deadline = monotomic_clock() + timeout.total_seconds() parent.debug( 'wait', @@ -4111,7 +4105,7 @@ def wait( f" checking every {tick:.2f} seconds") while True: - now = NOW() + now = monotomic_clock() if now > deadline: parent.debug( @@ -4126,7 +4120,7 @@ def wait( # Perform one extra check: if `check()` succeeded, but took more time than # allowed, it should be recognized as a failed waiting too. - now = NOW() + now = monotomic_clock() if now > deadline: parent.debug( @@ -4143,10 +4137,10 @@ def wait( return ret - except WaitingIncomplete: + except WaitingIncompleteError: # Update timestamp for more accurate logging - check() could have taken minutes # to complete, using the pre-check timestamp for logging would be misleading. - now = NOW() + now = monotomic_clock() parent.debug( 'wait', @@ -4234,11 +4228,7 @@ def dataclass_normalize_field( if not normalize_callback: normalize_callback = getattr(container, f'_normalize_{keyname}', None) - if normalize_callback: - value = normalize_callback(key_address, raw_value, logger) - - else: - value = raw_value + value = normalize_callback(key_address, raw_value, logger) if normalize_callback else raw_value # As mentioned in BasePlugin._update_data_from_options, the test # performed there is questionable. To gain more visibility into how @@ -4466,7 +4456,7 @@ def _normalize_string_list( return [value] if isinstance(value, (list, tuple)): - return [item for item in value] + return list(value) raise NormalizationError(key_address, value, 'a string or a list of strings') @@ -4548,8 +4538,9 @@ def items(self) -> Generator[Tuple[str, Any], None, None]: Yields: pairs of key name and its value. """ - - for keyname in self.keys(): + # SIM118 Use `{key} in {dict}` instead of `{key} in {dict}.keys(). + # "Type[SerializableContainerDerivedType]" has no attribute "__iter__" (not iterable) + for keyname in self.keys(): # noqa: SIM118 yield (keyname, getattr(self, keyname)) # TODO: exists for backward compatibility for the transition period. Once full @@ -4567,17 +4558,17 @@ def _load_keys( logger: tmt.log.Logger) -> None: """ Extract values for class-level attributes, and verify they match declared types. """ - LOG_SHIFT, LOG_LEVEL = 2, 4 + log_shift, log_level = 2, 4 debug_intro = functools.partial( logger.debug, - shift=LOG_SHIFT - 1, - level=LOG_LEVEL, + shift=log_shift - 1, + level=log_level, topic=tmt.log.Topic.KEY_NORMALIZATION) debug = functools.partial( logger.debug, - shift=LOG_SHIFT, - level=LOG_LEVEL, + shift=log_shift, + level=log_level, topic=tmt.log.Topic.KEY_NORMALIZATION) debug_intro('key source') @@ -4600,7 +4591,7 @@ def _load_keys( value: Any = None # Verbose, let's hide it a bit deeper. - debug('dict', self.__dict__, level=LOG_LEVEL + 1) + debug('dict', self.__dict__, level=log_level + 1) if hasattr(self, keyname): # If the key exists as instance's attribute already, it is because it's been @@ -4888,5 +4879,5 @@ def is_selinux_supported() -> bool: For detection ``/proc/filesystems`` is used, see ``man 5 filesystems`` for details. """ - with open('/proc/filesystems', 'r') as file: + with open('/proc/filesystems') as file: return any('selinuxfs' in line for line in file)