From a85dde52c982124513bf4a42e212b07c946f104e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Thu, 12 Sep 2024 13:46:56 +0200 Subject: [PATCH] Forbid use of os.path, point user to tmt._compat.Path --- pyproject.toml | 1 + tests/discover/data/distgit_test_plugin.py | 11 ++++----- tests/unit/test_cli.py | 5 ++-- tests/unit/test_schemas.py | 13 ++++++----- tmt/steps/discover/fmf.py | 7 +++--- tmt/steps/provision/mrack.py | 27 ++++++++++------------ 6 files changed, 31 insertions(+), 33 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 711bd25e27..0a81534d60 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -405,6 +405,7 @@ builtins-ignorelist = ["help", "format", "input", "filter", "copyright", "max"] "pathlib.Path".msg = "Use tmt._compat.pathlib.Path instead." "pathlib.PosixPath".msg = "Use tmt._compat.pathlib.Path instead." "warnings.deprecated".msg = "Use tmt._compat.warnings.deprecated instead." +"os.path.join".msg = "Use tmt._compat.pathlib.Path and pathlib instead." [tool.ruff.lint.isort] known-first-party = ["tmt"] diff --git a/tests/discover/data/distgit_test_plugin.py b/tests/discover/data/distgit_test_plugin.py index 6e6bce89df..0af5a4e2db 100644 --- a/tests/discover/data/distgit_test_plugin.py +++ b/tests/discover/data/distgit_test_plugin.py @@ -1,6 +1,5 @@ -import os -from tmt.utils import DistGitHandler +from tmt.utils import DistGitHandler, Path MOCK_SOURCES_FILENAME = 'mock_sources' SERVER_PORT = 9000 @@ -17,9 +16,9 @@ class TestDistGit(DistGitHandler): usage_name = "TESTING" server = f"http://localhost:{SERVER_PORT}" - def url_and_name(self, cwd='.'): - with open(os.path.join(cwd, MOCK_SOURCES_FILENAME)) as f: - data = f.read() + def url_and_name(self, cwd=None): + cwd = cwd or Path.cwd() + data = (cwd / MOCK_SOURCES_FILENAME).read_text() for line in data.splitlines(): split = line.split(' ', maxsplit=2) url = split[0] @@ -27,4 +26,4 @@ def url_and_name(self, cwd='.'): src_name = split[1] except IndexError: src_name = url - yield (os.path.join(self.server, url), src_name) + yield (Path(self.server) / url, src_name) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 6cfbcf6605..fb5139fe2b 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -10,14 +10,15 @@ import tmt.cli import tmt.log from tests import CliRunner +from tmt.utils import Path # Prepare path to examples -PATH = os.path.dirname(os.path.realpath(__file__)) +PATH = Path(__file__).resolve().parent def example(name): """ Return path to given example """ - return os.path.join(PATH, "../../examples/", name) + return PATH / "../../examples/" / name runner = CliRunner() diff --git a/tests/unit/test_schemas.py b/tests/unit/test_schemas.py index 1df7041a5d..d89bea5c7c 100644 --- a/tests/unit/test_schemas.py +++ b/tests/unit/test_schemas.py @@ -8,9 +8,10 @@ import tmt import tmt.log import tmt.utils +from tmt.utils import Path -PATH = os.path.dirname(os.path.realpath(__file__)) -ROOTDIR = os.path.join(PATH, "../..") +PATH = Path(__file__).resolve().parent +ROOTDIR = PATH.parent.parent LOGGER = tmt.log.Logger.create(verbose=0, debug=0, quiet=False) @@ -44,7 +45,7 @@ def _iter_trees(): # But the code below works, like a charm, and we will need to cover more trees than # just the root one, so leaving the code here but disabled. if False: - for dirpath, dirnames, _ in os.walk(os.path.join(ROOTDIR, 'tests')): + for dirpath, dirnames, _ in os.walk(ROOTDIR / 'tests'): if '.fmf' in dirnames: yield tmt.Tree(path=dirpath) @@ -193,7 +194,7 @@ def test_hw_schema_examples(hw: str, request) -> None: validate_node( tree, node, - os.path.join('provision', 'artemis.yaml'), + Path('provision') / 'artemis.yaml', 'HW requirements', request.node.callspec.id ) @@ -248,7 +249,7 @@ def test_ks_schema_examples(ks: str, request) -> None: validate_node( tree, node, - os.path.join('provision', 'artemis.yaml'), + Path('provision') / 'artemis.yaml', 'Kickstart requirements', request.node.callspec.id ) @@ -272,7 +273,7 @@ def test_watchdog_specification() -> None: validate_node( tree, node, - os.path.join('provision', 'artemis.yaml'), + Path('provision') / 'artemis.yaml', 'Watchdog specification', 'both-wd-options' ) diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index d176fc66fe..9dfeddf576 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -650,8 +650,8 @@ def post_dist_git(self, created_content: list[Path]) -> None: # glob otherwise if dist_git_extract and dist_git_extract != '/': try: - dist_git_extract = Path(glob.glob(os.path.join( - sourcedir, dist_git_extract.lstrip('/')))[0]) + + dist_git_extract = Path(glob.glob(sourcedir / dist_git_extract.lstrip('/'))[0]) except IndexError: raise tmt.utils.DiscoverError( f"Couldn't glob '{dist_git_extract}' " @@ -673,8 +673,7 @@ def post_dist_git(self, created_content: list[Path]) -> None: except tmt.utils.MetadataError: self.warn("No fmf root to remove, there isn't one already.") if not self.is_dry_run: - shutil.rmtree(os.path.join( - dist_git_extract or extracted_fmf_root, '.fmf')) + shutil.rmtree((dist_git_extract or extracted_fmf_root) / '.fmf') if not dist_git_extract: try: top_fmf_root = tmt.utils.find_fmf_root(sourcedir, ignore_paths=[sourcedir])[0] diff --git a/tmt/steps/provision/mrack.py b/tmt/steps/provision/mrack.py index 305154b24a..7d2fe2245f 100644 --- a/tmt/steps/provision/mrack.py +++ b/tmt/steps/provision/mrack.py @@ -15,7 +15,7 @@ import tmt.steps import tmt.steps.provision import tmt.utils -from tmt.utils import Command, ProvisionError, ShellScript, UpdatableMessage, field +from tmt.utils import Command, Path, ProvisionError, ShellScript, UpdatableMessage, field mrack: Any providers: Any @@ -824,28 +824,25 @@ async def __init__(self, guest: 'GuestBeaker') -> None: # type: ignore[misc] # use global context class global_context = mrack.context.global_context - mrack_config = "" - if os.path.exists(os.path.join(os.path.dirname(__file__), "mrack/mrack.conf")): - mrack_config = os.path.join( - os.path.dirname(__file__), - "mrack/mrack.conf", - ) - - if os.path.exists("/etc/tmt/mrack.conf"): - mrack_config = "/etc/tmt/mrack.conf" + mrack_config_locations = [ + Path(__file__).parent / "mrack/mrack.conf", + Path("/etc/tmt/mrack.conf"), + Path("~/.mrack/mrack.conf").expanduser(), + Path.cwd() / "mrack.conf" + ] - if os.path.exists(os.path.join(os.path.expanduser("~"), ".mrack/mrack.conf")): - mrack_config = os.path.join(os.path.expanduser("~"), ".mrack/mrack.conf") + mrack_config: Optional[Path] = None - if os.path.exists(os.path.join(os.getcwd(), "mrack.conf")): - mrack_config = os.path.join(os.getcwd(), "mrack.conf") + for potential_location in mrack_config_locations: + if potential_location.exists(): + mrack_config = potential_location if not mrack_config: raise ProvisionError("Configuration file 'mrack.conf' not found.") try: - global_context.init(mrack_config) + global_context.init(str(mrack_config)) except mrack.errors.ConfigError as mrack_conf_err: raise ProvisionError(mrack_conf_err)