From 65e3e398e9c773b001e33e2b4e686220079df58a Mon Sep 17 00:00:00 2001 From: Shatakshi Mishra Date: Thu, 6 Jun 2024 14:57:05 +0530 Subject: [PATCH] Address ruff `PTH103` (#1789) * Address ruff PTH103 * Test changes * Fix monkeypatch setattr to use path from pathlib * Remove commented code --- pyproject.toml | 1 - src/ansible_navigator/actions/run.py | 2 +- .../navigator_post_processor.py | 2 +- src/ansible_navigator/initialization.py | 2 +- tests/defaults.py | 6 ++---- tests/integration/_common.py | 16 ++++++++-------- tests/integration/actions/collections/base.py | 5 +++-- tests/integration/conftest.py | 13 +++++-------- tests/unit/actions/run/test_artifact.py | 3 ++- .../test_invalid_params.py | 2 +- 10 files changed, 24 insertions(+), 28 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index aac08e098..96ebfd0ca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -299,7 +299,6 @@ ignore = [ 'PT005', # Fixture `_settings_samples` returns a value, remove leading underscore 'PT019', # Fixture `_mocked_func` without value is injected as parameter, use `@pytest.mark.usefixtures` instead 'PT022', # [*] No teardown in fixture `cmd_in_tty`, use `return` instead of `yield` - 'PTH103', # `os.makedirs()` should be replaced by `Path.mkdir(parents=True)` 'PTH109', # `os.getcwd()` should be replaced by `Path.cwd()` 'PTH110', # `os.path.exists()` should be replaced by `Path.exists()` 'PTH111', # `os.path.expanduser()` should be replaced by `Path.expanduser()` diff --git a/src/ansible_navigator/actions/run.py b/src/ansible_navigator/actions/run.py index c1246f028..4735a8727 100644 --- a/src/ansible_navigator/actions/run.py +++ b/src/ansible_navigator/actions/run.py @@ -896,7 +896,7 @@ def write_artifact(self, filename: str | None = None) -> None: self._logger.debug("Resolved artifact file name set to %s", filename) try: - os.makedirs(os.path.dirname(filename), exist_ok=True) + Path(os.path.dirname(filename)).mkdir(parents=True, exist_ok=True) artifact = { "version": "2.0.0", "plays": self._plays.value, diff --git a/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py b/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py index 7b74135bc..da6a11224 100644 --- a/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py +++ b/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py @@ -736,7 +736,7 @@ def log_file(entry: SettingsEntry, config: ApplicationConfiguration) -> PostProc exit_messages: list[ExitMessage] = [] entry.value.current = abs_user_path(entry.value.current) try: - os.makedirs(os.path.dirname(entry.value.current), exist_ok=True) + Path(os.path.dirname(entry.value.current)).mkdir(parents=True, exist_ok=True) Path(entry.value.current).touch() except (OSError, FileNotFoundError) as exc: exit_msgs = [ diff --git a/src/ansible_navigator/initialization.py b/src/ansible_navigator/initialization.py index e494bd5e3..c9c626d9d 100644 --- a/src/ansible_navigator/initialization.py +++ b/src/ansible_navigator/initialization.py @@ -109,7 +109,7 @@ def get_and_check_collection_doc_cache( path_errors = [] doc_cache_dir = os.path.dirname(collection_doc_cache_path) try: - os.makedirs(doc_cache_dir, exist_ok=True) + Path(doc_cache_dir).mkdir(parents=True, exist_ok=True) except OSError as exc: path_errors.append(f"Problem making directory: {doc_cache_dir}") path_errors.append(f"Error was: {exc!s}") diff --git a/tests/defaults.py b/tests/defaults.py index f98690436..5d9285362 100644 --- a/tests/defaults.py +++ b/tests/defaults.py @@ -10,10 +10,8 @@ FIXTURES_DIR = str(expand_path(os.path.join(os.path.dirname(__file__), "fixtures"))) -FIXTURES_COLLECTION_DIR = str( - expand_path( - os.path.join(os.path.dirname(__file__), "fixtures", "common", "collections"), - ), +FIXTURES_COLLECTION_DIR = expand_path( + os.path.join(os.path.dirname(__file__), "fixtures", "common", "collections"), ) FIXTURES_COLLECTION_PATH = Path(FIXTURES_COLLECTION_DIR) diff --git a/tests/integration/_common.py b/tests/integration/_common.py index a95a4ea83..37007c37a 100644 --- a/tests/integration/_common.py +++ b/tests/integration/_common.py @@ -171,8 +171,8 @@ def sanitize_output(output: list[str]) -> list[str]: def copytree( - src: str, - dst: str, + src: Path, + dst: Path, symlinks: bool = False, ignore: Callable[..., Any] | None = None, dirs_exist_ok: bool = False, @@ -209,18 +209,18 @@ def copytree( names = os.listdir(src) ignored_names = ignore(src, names) if ignore is not None else set() - os.makedirs(dst, exist_ok=dirs_exist_ok) + dst.mkdir(parents=True, exist_ok=dirs_exist_ok) errors = [] for name in names: if name in ignored_names: continue - source_path = os.path.join(src, name) - destination_path = os.path.join(dst, name) + source_path = src / name + destination_path = dst / name try: - if symlinks and Path(source_path).is_symlink(): - source_link = Path(source_path).readlink() + if symlinks and source_path.is_symlink(): + source_link = source_path.readlink() os.symlink(source_link, destination_path) - elif Path(source_path).is_dir(): + elif source_path.is_dir(): copytree( source_path, destination_path, diff --git a/tests/integration/actions/collections/base.py b/tests/integration/actions/collections/base.py index 76dfae9a1..462b7dd20 100644 --- a/tests/integration/actions/collections/base.py +++ b/tests/integration/actions/collections/base.py @@ -4,6 +4,7 @@ import os from collections.abc import Generator +from pathlib import Path import pytest @@ -103,10 +104,10 @@ def fixture_tmux_session( :yields: A tmux session """ tmp_coll_dir = os.path.join(os_independent_tmp, request.node.name, "") - os.makedirs(tmp_coll_dir, exist_ok=True) + Path(tmp_coll_dir).mkdir(parents=True, exist_ok=True) copytree( FIXTURES_COLLECTION_DIR, - os.path.join(tmp_coll_dir, "collections"), + Path(tmp_coll_dir) / "collections", dirs_exist_ok=True, ) params: TmuxSessionKwargs = { diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9f4c1d2eb..a8d976c82 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -5,6 +5,7 @@ import os from copy import deepcopy +from pathlib import Path from typing import TYPE_CHECKING import pytest @@ -23,7 +24,6 @@ if TYPE_CHECKING: from collections.abc import Generator - from pathlib import Path EXECUTION_MODES = ["interactive", "stdout"] @@ -57,13 +57,10 @@ def os_independent_tmp() -> str: :return: The os independent tmp directory. """ - tmp_real = os.path.realpath("/tmp") - if tmp_real == "/private/tmp": - an_tmp = os.path.join(tmp_real, "an") - else: - an_tmp = os.path.join("/tmp", "private", "an") - os.makedirs(an_tmp, exist_ok=True) - return an_tmp + tmp_real = Path("/tmp").resolve() + an_tmp = tmp_real / "an" if tmp_real == "/private/tmp" else Path("/tmp") / "private" / "an" + an_tmp.mkdir(parents=True, exist_ok=True) + return str(an_tmp) @pytest.mark.usefixtures("cmd_in_tty") diff --git a/tests/unit/actions/run/test_artifact.py b/tests/unit/actions/run/test_artifact.py index 341099584..551ca8f53 100644 --- a/tests/unit/actions/run/test_artifact.py +++ b/tests/unit/actions/run/test_artifact.py @@ -4,6 +4,7 @@ import logging import os +import pathlib import re from copy import deepcopy @@ -201,7 +202,7 @@ def test_artifact_path( """ caplog.set_level(logging.DEBUG) monkeypatch.setenv("HOME", "/home/test_user") - monkeypatch.setattr(os, "makedirs", make_dirs) + monkeypatch.setattr(pathlib.Path, "mkdir", make_dirs) monkeypatch.setattr(action, "_get_status", get_status) mocked_write = mocker.patch( "ansible_navigator.actions.run.serialize_write_file", diff --git a/tests/unit/configuration_subsystem/test_invalid_params.py b/tests/unit/configuration_subsystem/test_invalid_params.py index 50cb87fd6..8a249ba71 100644 --- a/tests/unit/configuration_subsystem/test_invalid_params.py +++ b/tests/unit/configuration_subsystem/test_invalid_params.py @@ -128,7 +128,7 @@ def makedirs(*args: Any, **kwargs: dict[str, Any]) -> None: """ raise OSError - monkeypatch.setattr("os.makedirs", makedirs) + monkeypatch.setattr("pathlib.Path.mkdir", makedirs) monkeypatch.setattr("shutil.which", which) response = generate_config()