From 7ff4b6e38f86021ecd75a4348eb5df45d68b3dca Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Tue, 4 Jun 2024 19:40:51 +0530 Subject: [PATCH 1/4] Address ruff PTH103 --- pyproject.toml | 1 - src/ansible_navigator/actions/run.py | 2 +- .../configuration_subsystem/navigator_post_processor.py | 2 +- src/ansible_navigator/initialization.py | 2 +- tests/integration/_common.py | 2 +- tests/integration/actions/collections/base.py | 3 ++- tests/integration/conftest.py | 4 ++-- 7 files changed, 8 insertions(+), 8 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/integration/_common.py b/tests/integration/_common.py index a95a4ea83..2205a19d1 100644 --- a/tests/integration/_common.py +++ b/tests/integration/_common.py @@ -209,7 +209,7 @@ 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) + Path(dst).mkdir(parents=True, exist_ok=dirs_exist_ok) errors = [] for name in names: if name in ignored_names: diff --git a/tests/integration/actions/collections/base.py b/tests/integration/actions/collections/base.py index 76dfae9a1..8f242d403 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,7 +104,7 @@ 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"), diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9f4c1d2eb..7f393f3f1 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"] @@ -62,7 +62,7 @@ def os_independent_tmp() -> str: an_tmp = os.path.join(tmp_real, "an") else: an_tmp = os.path.join("/tmp", "private", "an") - os.makedirs(an_tmp, exist_ok=True) + Path(an_tmp).mkdir(parents=True, exist_ok=True) return an_tmp From 88d3a1c20e2af9cf60e6b4ea0747b23f58261ae4 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Thu, 6 Jun 2024 11:09:20 +0530 Subject: [PATCH 2/4] Test changes --- tests/defaults.py | 6 ++---- tests/integration/_common.py | 16 ++++++++-------- tests/integration/actions/collections/base.py | 2 +- tests/integration/conftest.py | 12 +++++------- 4 files changed, 16 insertions(+), 20 deletions(-) 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 2205a19d1..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() - Path(dst).mkdir(parents=True, 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 8f242d403..462b7dd20 100644 --- a/tests/integration/actions/collections/base.py +++ b/tests/integration/actions/collections/base.py @@ -107,7 +107,7 @@ def fixture_tmux_session( 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 7f393f3f1..f63392da2 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -57,13 +57,11 @@ 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") - Path(an_tmp).mkdir(parents=True, exist_ok=True) - return an_tmp + tmp_real = Path("/tmp").resolve() + # tmp_real = os.path.realpath("/tmp") + 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") From 53a3f87cdcffefd702c83775738a0ab0824bc9b6 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Thu, 6 Jun 2024 14:01:04 +0530 Subject: [PATCH 3/4] Fix monkeypatch setattr to use path from pathlib --- tests/unit/actions/run/test_artifact.py | 3 ++- tests/unit/configuration_subsystem/test_invalid_params.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) 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() From 6814a8b07d6426a22fac1a8cbe1ee7d5e71c37af Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Thu, 6 Jun 2024 14:12:55 +0530 Subject: [PATCH 4/4] Remove commented code --- tests/integration/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index f63392da2..a8d976c82 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -58,7 +58,6 @@ def os_independent_tmp() -> str: :return: The os independent tmp directory. """ tmp_real = Path("/tmp").resolve() - # tmp_real = os.path.realpath("/tmp") 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)