Address ruff PT (#1761)

* Address ruff PT exceptions

* Fixes for PT004

* More fixes for PT004

* Revert PT019

* Fix PT012

pyproject.toml

@@ -318,15 +318,7 @@ ignore = [
   'PLW0120', # `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
   'PLW0603', # Using the global statement to update `DIAGNOSTIC_FAILURES` is discouraged
   'PLW2901', # `for` loop variable `info_name` overwritten by assignment target
-  'PT001', # [*] Use `@pytest.fixture()` over `@pytest.fixture`
-  'PT003', # [*] `scope='function'` is implied in `@pytest.fixture()`
-  'PT004', # Fixture `patch_curses` does not return anything, add leading underscore
   'PT005', # Fixture `_settings_samples` returns a value, remove leading underscore
-  'PT006', # [*] Wrong name(s) type in `@pytest.mark.parametrize`, expected `tuple`
-  'PT007', # Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `tuple`
-  'PT009', # [*] Use a regular `assert` instead of unittest-style `assertIn`
-  'PT011', # `pytest.raises(ValueError)` is too broad, set the `match` parameter or use a more specific exception
-  'PT012', # `pytest.raises()` block should contain a single simple statement
   '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`
   'PTH100', # `os.path.abspath()` should be replaced by `Path.resolve()`

tests/conftest.py

@@ -107,7 +107,7 @@ def fixture_small_image_name() -> str:
     return small_image_name()
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture()
 def locked_directory(tmpdir: str) -> Generator[str, None, None]:
     """Directory without read-write for throwing errors.
 
@@ -131,8 +131,8 @@ def pullable_image(valid_container_engine: str) -> Generator[str, None, None]:
     subprocess.run([valid_container_engine, "image", "rm", image], check=True)
 
 
-@pytest.fixture
-def patch_curses(monkeypatch: pytest.MonkeyPatch) -> None:
+@pytest.fixture()
+def _patch_curses(monkeypatch: pytest.MonkeyPatch) -> None:
     """Patch curses so it doesn't traceback during tests.
 
     :param monkeypatch: Fixture for patching
@@ -191,7 +191,7 @@ def settings_env_var_to_full(
     return settings_path, settings_contents
 
 
-@pytest.fixture
+@pytest.fixture()
 def test_dir_fixture_dir(request: pytest.FixtureRequest) -> Path:
     """Provide the fixture directory for a given test directory.
 
@@ -297,7 +297,7 @@ def _cmd_in_tty(
     return result[m_stdout].decode("utf-8"), result[m_stderr].decode("utf-8"), proc.returncode
 
 
-@pytest.fixture
+@pytest.fixture()
 def cmd_in_tty() -> Generator[Callable[..., tuple[str, str, int]], None, None]:
     """Provide the cmd in tty function as a fixture.
 
@@ -409,8 +409,8 @@ def pytest_unconfigure(config: pytest.Config) -> None:
         os.environ[key] = value
 
 
-@pytest.fixture(scope="function")
-def skip_if_already_failed(
+@pytest.fixture()
+def skip_if_already_failed(  # noqa: PT004
     request: pytest.FixtureRequest, failed: set[str] = set()
 ) -> Generator[None, None, None]:
     """Fixture that stops parametrized tests running on first failure."""

tests/integration/_cli2runner.py

@@ -18,7 +18,7 @@ class RunnerTestError(Exception):
     """Custom exception for runner to throw."""
 
 
-@pytest.mark.usefixtures("patch_curses")
+@pytest.mark.usefixtures("_patch_curses")
 class Cli2Runner:
     """A base class which mocks the runner calls."""
 

tests/integration/actions/collections/test_mode_stdout.py

@@ -11,9 +11,9 @@
 from tests.integration.conftest import CliRunner
 
 
-@pytest.mark.parametrize("cwd", [True, False], ids=["adjacent", "not_adjacent"])
-@pytest.mark.parametrize("exec_env", [True, False], ids=["ee", "no_ee"])
-@pytest.mark.parametrize("stdout_format", ["json", "yaml"], ids=["json", "yaml"])
+@pytest.mark.parametrize("cwd", (True, False), ids=["adjacent", "not_adjacent"])
+@pytest.mark.parametrize("exec_env", (True, False), ids=["ee", "no_ee"])
+@pytest.mark.parametrize("stdout_format", ("json", "yaml"), ids=["json", "yaml"])
 def test_stdout_output(
     cwd: bool,
     exec_env: bool,

tests/integration/conftest.py

@@ -25,7 +25,7 @@
 EXECUTION_MODES = ["interactive", "stdout"]
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture()
 def action_run_stdout() -> Generator[type[ActionRunTest], None, None]:
     """Create a fixture for ActionRunTest.
 
@@ -135,7 +135,7 @@ def run(self) -> tuple[str, str, int]:
         return cmd_in_tty(self.to_cmdline(), cwd=self.cwd)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture()
 def cli_runner(request: pytest.FixtureRequest) -> CliRunner:
     """Create a fixture for the cli runner.
 

tests/smoke/no_test_dependencies.py

@@ -110,14 +110,10 @@ def test(self) -> None:
             assert isinstance(command, NavigatorCommand)
             with self.subTest():
                 print(command.command)
-                self.assertIn(
-                    command.find,
-                    command.stdout,
-                    msg=(
-                        f"command: {command.command},"
-                        f" stdout: {command.stdout},"
-                        f" stderr: {command.stderr}"
-                    ),
+                assert command.find in command.stdout, (
+                    f"command: {command.command}, "
+                    f"stdout: {command.stdout}, "
+                    f"stderr: {command.stderr}"
                 )
 
 

tests/unit/actions/collections/test_collection_doc_cache_path.py

@@ -30,7 +30,7 @@ class DuplicateMountError(RuntimeError):
 
 
 @pytest.mark.parametrize("doc_cache_path", DOC_CACHE_PATHS)
-@pytest.mark.usefixtures("patch_curses")
+@pytest.mark.usefixtures("_patch_curses")
 def test_for_duplicates_sources(
     doc_cache_path: TstData,
     monkeypatch: pytest.MonkeyPatch,

tests/unit/configuration_subsystem/conftest.py

@@ -107,8 +107,8 @@ def fixture_generate_config() -> Callable[..., GenerateConfigResponse]:
     return _generate_config
 
 
-@pytest.fixture
-def ansible_version(monkeypatch: pytest.MonkeyPatch) -> None:
+@pytest.fixture()
+def _ansible_version(monkeypatch: pytest.MonkeyPatch) -> None:
     """Path the ansible --version call to avoid the subprocess calls.
 
     :param monkeypatch: Fixture for patching

tests/unit/configuration_subsystem/test_ansible_config_parse.py

@@ -166,7 +166,7 @@ def test_invalid_configurator(
     assert "12345" in exit_messages[3].message
 
 
-@pytest.mark.usefixtures("ansible_version")
+@pytest.mark.usefixtures("_ansible_version")
 @ee_states
 def test_config_none(ee_enabled: bool) -> None:
     """Confirm a invalid ansible.cfg raises errors.

tests/unit/configuration_subsystem/test_configurator.py

@@ -22,9 +22,9 @@
 
 def test_mutual_exclusivity_for_configuration_init() -> None:
     """Ensure the configuration cannot be initiated with apply_previous_cli_entries and initial."""
+    application_configuration = deepcopy(NavigatorConfiguration)
+    application_configuration.internals.initializing = True
     with pytest.raises(ValueError, match="cannot be used while initializing"):
-        application_configuration = deepcopy(NavigatorConfiguration)
-        application_configuration.internals.initializing = True
         Configurator(
             params=None,
             application_configuration=application_configuration,
@@ -34,9 +34,9 @@ def test_mutual_exclusivity_for_configuration_init() -> None:
 
 def test_apply_before_initial_saved() -> None:
     """Ensure the apply_previous_cli_entries can't be used before initial."""
+    application_configuration = deepcopy(NavigatorConfiguration)
+    application_configuration.internals.initializing = False
     with pytest.raises(ValueError, match="enabled prior to"):
-        application_configuration = deepcopy(NavigatorConfiguration)
-        application_configuration.internals.initializing = False
         Configurator(
             params=None,
             application_configuration=application_configuration,

tests/unit/configuration_subsystem/test_posix_message_queue.py

@@ -8,10 +8,10 @@
 import pytest
 
 
-@pytest.mark.parametrize("is_dir", [True, False], ids=["is_dir", "no_is_dir"])
-@pytest.mark.parametrize("ee_support", [True, False], ids=["ee", "no-ee"])
-@pytest.mark.parametrize("engine", ["podman", "docker"], ids=["podman", "docker"])
-@pytest.mark.parametrize("platform", ["linux", "darwin"], ids=["linux", "darwin"])
+@pytest.mark.parametrize("is_dir", (True, False), ids=["is_dir", "no_is_dir"])
+@pytest.mark.parametrize("ee_support", (True, False), ids=["ee", "no-ee"])
+@pytest.mark.parametrize("engine", ("podman", "docker"), ids=["podman", "docker"])
+@pytest.mark.parametrize("platform", ("linux", "darwin"), ids=["linux", "darwin"])
 def test_posix_message_queue_ee(
     monkeypatch: pytest.MonkeyPatch,
     is_dir: bool,

tests/unit/configuration_subsystem/test_precedence.py

@@ -48,9 +48,9 @@ def isfile(*_args: Any, **_kwargs: dict[str, Any]) -> bool:
     return True
 
 
-@pytest.mark.usefixtures("ansible_version")
+@pytest.mark.usefixtures("_ansible_version")
 @pytest.mark.parametrize("base", (None, BASE_SHORT_CLI, BASE_LONG_CLI), ids=id_for_base)
-@pytest.mark.parametrize("cli_entry, expected", CLI_DATA)
+@pytest.mark.parametrize(("cli_entry", "expected"), CLI_DATA)
 def test_all_entries_reflect_cli_given_env_vars(
     monkeypatch: pytest.MonkeyPatch,
     generate_config: GenerateConfigCallable,
@@ -102,10 +102,10 @@ def test_all_entries_reflect_cli_given_env_vars(
             assert entry.value.source is C.ENVIRONMENT_VARIABLE, entry.name
 
 
-@pytest.mark.usefixtures("ansible_version")
-@pytest.mark.parametrize("settings, settings_file_type", SETTINGS)
+@pytest.mark.usefixtures("_ansible_version")
+@pytest.mark.parametrize(("settings", "settings_file_type"), SETTINGS)
 @pytest.mark.parametrize("base", (None, BASE_SHORT_CLI, BASE_LONG_CLI), ids=id_for_base)
-@pytest.mark.parametrize("cli_entry, expected", CLI_DATA)
+@pytest.mark.parametrize(("cli_entry", "expected"), CLI_DATA)
 def test_all_entries_reflect_cli_given_settings(
     monkeypatch: pytest.MonkeyPatch,
     generate_config: GenerateConfigCallable,
@@ -158,10 +158,10 @@ def test_all_entries_reflect_cli_given_settings(
                 assert entry.value.source is C.USER_CFG, entry.name
 
 
-@pytest.mark.usefixtures("ansible_version")
-@pytest.mark.parametrize("settings, source_other", SETTINGS)
+@pytest.mark.usefixtures("_ansible_version")
+@pytest.mark.parametrize(("settings", "source_other"), SETTINGS)
 @pytest.mark.parametrize("base", (None, BASE_SHORT_CLI, BASE_LONG_CLI), ids=id_for_base)
-@pytest.mark.parametrize("cli_entry, expected", CLI_DATA)
+@pytest.mark.parametrize(("cli_entry", "expected"), CLI_DATA)
 def test_all_entries_reflect_cli_given_settings_and_env_vars(
     monkeypatch: pytest.MonkeyPatch,
     generate_config: GenerateConfigCallable,
@@ -220,7 +220,7 @@ def test_all_entries_reflect_cli_given_settings_and_env_vars(
             assert entry.value.source is C.ENVIRONMENT_VARIABLE, entry.name
 
 
-@pytest.mark.usefixtures("ansible_version")
+@pytest.mark.usefixtures("_ansible_version")
 @pytest.mark.parametrize("entry", NavigatorConfiguration.entries, ids=id_for_name)
 def test_all_entries_reflect_default(
     monkeypatch: pytest.MonkeyPatch,
@@ -252,9 +252,9 @@ def test_all_entries_reflect_default(
             assert configured_entry.value.current == entry.value.default, configured_entry
 
 
-@pytest.mark.usefixtures("ansible_version")
-@pytest.mark.parametrize("settings, settings_file_type", SETTINGS)
-@pytest.mark.parametrize("entry, value, expected", ENV_VAR_DATA)
+@pytest.mark.usefixtures("_ansible_version")
+@pytest.mark.parametrize(("settings", "settings_file_type"), SETTINGS)
+@pytest.mark.parametrize(("entry", "value", "expected"), ENV_VAR_DATA)
 def test_all_entries_reflect_env_var_given_settings(
     monkeypatch: pytest.MonkeyPatch,
     generate_config: GenerateConfigCallable,
@@ -303,7 +303,7 @@ def test_all_entries_reflect_env_var_given_settings(
                 assert other_entry.value.source is C.USER_CFG, other_entry.name
 
 
-@pytest.mark.usefixtures("ansible_version")
+@pytest.mark.usefixtures("_ansible_version")
 @pytest.mark.parametrize("entry", NavigatorConfiguration.entries, ids=id_for_name)
 def test_all_entries_reflect_settings_given_settings(
     monkeypatch: pytest.MonkeyPatch,

tests/unit/image_manager/test_image_puller.py

@@ -144,7 +144,7 @@ def test_will_have(valid_container_engine: str, pullable_image: str, data: TstPu
 
 @pytest.mark.parametrize(
     ("image", "expected_tag"),
-    [
+    (
         pytest.param("foo", "latest", id="simple-image-name:no-tag-specified"),
         pytest.param("foo:bar", "bar", id="simple-image-name:with-tag"),
         pytest.param(
@@ -157,7 +157,7 @@ def test_will_have(valid_container_engine: str, pullable_image: str, data: TstPu
             "latest",
             id="complex-image-URL:with-port-and-tag",
         ),
-    ],
+    ),
 )
 def test_tag_parsing(image: str, expected_tag: str) -> None:
     """Test that we parse image tags in a reasonable way.

tests/unit/test_cli.py

@@ -20,7 +20,7 @@
 
 
 @pytest.mark.parametrize(
-    "given, argname, expected",
+    ("given", "argname", "expected"),
     (
         pytest.param(
             ["doc", "-t", "callback", "oneline"],

tests/unit/utils/conftest.py

@@ -8,7 +8,7 @@
 from ansible_navigator.utils.key_value_store import KeyValueStore
 
 
-@pytest.fixture
+@pytest.fixture()
 def empty_kvs(tmp_path: Path) -> Generator[KeyValueStore, None, None]:
     """Give us a temporary, empty KeyValueStore.
 
@@ -19,7 +19,7 @@ def empty_kvs(tmp_path: Path) -> Generator[KeyValueStore, None, None]:
     yield KeyValueStore(database_path)
 
 
-@pytest.fixture
+@pytest.fixture()
 def kvs(tmp_path: Path) -> Generator[KeyValueStore, None, None]:
     """Give us a temporary KeyValueStore with some data.
 

tests/unit/utils/test_dot_paths.py

@@ -309,7 +309,7 @@ def test_place_at_path_raises(scenario: Scenario) -> None:
 
     :param scenario: Test data.
     """
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError):  # noqa: PT011
         place_at_path(
             behaviors=scenario.behaviors,
             content=scenario.content,

tests/unit/utils/test_functions.py

@@ -69,7 +69,7 @@ def check_path_exists(arg: Any) -> bool:
 
 
 @pytest.mark.parametrize(
-    "set_env, file_path, anticipated_result",
+    ("set_env", "file_path", "anticipated_result"),
     (
         (True, os.path.abspath(__file__), os.path.abspath(__file__)),
         (True, "", None),
@@ -105,7 +105,7 @@ def test_env_var_is_file_path(
 
 
 @pytest.mark.parametrize(
-    "value, anticipated_result",
+    ("value", "anticipated_result"),
     (
         ([1, 2, 3], [1, 2, 3]),
         ([1, 2, [3]], [1, 2, 3]),
@@ -291,7 +291,7 @@ def test_now_iso(caplog: pytest.LogCaptureFixture, time_zone: str) -> None:
 
 
 @pytest.mark.parametrize(
-    "data,output",
+    ("data", "output"),
     (
         pytest.param({}, {}, id="0"),
         pytest.param(None, None, id="1"),

tests/unit/utils/test_serialize_yaml.py

@@ -35,7 +35,7 @@ def _aliases_allowed(monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureReq
 
 @pytest.mark.parametrize(
     "aliases_allowed",
-    [True, False],
+    (True, False),
     ids=["pyyaml-original", "no-aliases-or-anchors"],
     indirect=("aliases_allowed",),
 )