♻️ refactor(tests): stop importing private modules directly

Use from-imports and mocker.patch string paths instead of importing
private modules as objects for monkeypatching.

Author: gaborbernat

pyproject.toml

@@ -74,51 +74,23 @@ lint.select = [
   "ALL",
 ]
 lint.ignore = [
-  "ANN001",  # Missing type annotation for function argument
-  "ANN002",  # Missing type annotation for *args
-  "ANN202",  # Missing return type annotation for private function
-  "ANN204",  # Missing return type annotation for special method
-  "ANN205",  # Missing return type annotation for staticmethod
-  "ANN206",  # Missing return type annotation for classmethod
-  "ANN401",  # Dynamically typed expressions (typing.Any)
-  "ARG001",  # Unused function argument
-  "ARG005",  # Unused lambda argument
-  "B905",    # zip-without-explicit-strict (conflicts with ty on 3.8)
-  "C901",    # Too complex
-  "COM812",  # Conflict with formatter
-  "CPY",     # No copyright statements
-  "D",       # Docstring rules
-  "DOC",     # Docstring content rules
-  "E501",    # Line too long (formatter handles it)
-  "FBT",     # Boolean positional arguments
-  "INP001",  # Implicit namespace package
-  "ISC001",  # Conflict with formatter
-  "PLC0415", # import at top level
-  "PLR0904", # Too many public methods
-  "PLR0911", # Too many return statements
-  "PLR0912", # Too many branches
-  "PLR0913", # Too many arguments
-  "PLR0914", # Too many local variables
-  "PLR0915", # Too many statements
-  "PLR0917", # Too many positional arguments
-  "PLR1702", # Too many nested blocks
-  "PLR2004", # Magic value used in comparison
-  "PLR6301", # Method could be a function
-  "PTH",     # Use pathlib (existing code uses os.path)
-  "PYI024",  # Use `typing.NamedTuple` instead of `collections.namedtuple`
-  "RUF067",  # __init__ module should only contain re-exports
-  "S104",    # Possible binding to all interface
-  "S404",    # subprocess module is possibly insecure
-  "S603",    # `subprocess` call: check for execution of untrusted input
-  "SLF001",  # Private member accessed
-  "TID252",  # Prefer absolute imports over relative (internal package uses relative)
-  "TRY301",  # Abstract `raise` to an inner function
+  "COM812", # Conflict with formatter
+  "CPY",    # No copyright statements
+]
+lint.per-file-ignores."src/py_discovery/_discovery.py" = [
+  "PTH",  # shim resolution uses string-based os.path for consistency with env variables
+]
+lint.per-file-ignores."src/py_discovery/_py_info.py" = [
+  "PTH",  # must use os.path — file runs as subprocess script with only stdlib
+]
+lint.per-file-ignores."src/py_discovery/_windows/_pep514.py" = [
+  "PTH",  # os.path.exists is monkeypatched in tests; pathlib.Path.exists bypasses the mock
 ]
 lint.per-file-ignores."tests/**/*.py" = [
   "INP001",  # no implicit namespace
   "PLC2701", # Private imports
+  "PTH",     # os.path used intentionally for string-based path operations
   "S101",    # asserts allowed in tests
-  "S102",    # use of exec
 ]
 lint.per-file-ignores."tests/windows/winreg_mock_values.py" = [
   "F821", # undefined name (winreg available only on Windows)
@@ -173,5 +145,15 @@ paths.source = [
 src.exclude = [ "tests/windows/winreg_mock_values.py" ]
 
 [[tool.ty.overrides]]
-include = [ "src/py_discovery/_py_info.py", "tests/test_py_info_extra.py" ]
+include = [ "src/py_discovery/_py_info.py", "src/py_discovery/_py_spec.py" ]
+rules.unused-ignore-comment = "ignore"
+rules.invalid-argument-type = "ignore"
+rules.invalid-return-type = "ignore"
+rules.no-matching-overload = "ignore"
+
+[[tool.ty.overrides]]
+include = [ "tests/**/*.py" ]
 rules.unused-ignore-comment = "ignore"
+rules.invalid-argument-type = "ignore"
+rules.no-matching-overload = "ignore"
+rules.unresolved-attribute = "ignore"

src/py_discovery/_cache.py

@@ -6,13 +6,13 @@
 import logging
 from contextlib import contextmanager, suppress
 from hashlib import sha256
-from typing import TYPE_CHECKING, Protocol, runtime_checkable
+from typing import TYPE_CHECKING, Final, Protocol, runtime_checkable
 
 if TYPE_CHECKING:
     from collections.abc import Generator
     from pathlib import Path
 
-LOGGER = logging.getLogger(__name__)
+_LOGGER: Final[logging.Logger] = logging.getLogger(__name__)
 
 
 @runtime_checkable
@@ -61,9 +61,9 @@ def read(self) -> dict | None:
         except ValueError:
             bad_format = True
         except OSError:
-            LOGGER.debug("failed to read %s", self._file, exc_info=True)
+            _LOGGER.debug("failed to read %s", self._file, exc_info=True)
         else:
-            LOGGER.debug("got python info from %s", self._file)
+            _LOGGER.debug("got python info from %s", self._file)
             return data
         if bad_format:
             with suppress(OSError):
@@ -73,12 +73,12 @@ def read(self) -> dict | None:
     def write(self, content: dict) -> None:
         self._folder.mkdir(parents=True, exist_ok=True)
         self._file.write_text(json.dumps(content, sort_keys=True, indent=2), encoding="utf-8")
-        LOGGER.debug("wrote python info at %s", self._file)
+        _LOGGER.debug("wrote python info at %s", self._file)
 
     def remove(self) -> None:
         with suppress(OSError):
             self._file.unlink()
-        LOGGER.debug("removed python info at %s", self._file)
+        _LOGGER.debug("removed python info at %s", self._file)
 
     @contextmanager
     def locked(self) -> Generator[None]:
@@ -111,10 +111,10 @@ def py_info(self, path: Path) -> DiskContentStore:
     def py_info_clear(self) -> None:
         folder = self._py_info_dir
         if folder.exists():
-            for f in folder.iterdir():
-                if f.suffix == ".json":
+            for entry in folder.iterdir():
+                if entry.suffix == ".json":
                     with suppress(OSError):
-                        f.unlink()
+                        entry.unlink()
 
 
 class NoOpContentStore:

src/py_discovery/_cached_py_info.py

@@ -80,7 +80,7 @@ def _get_via_file_cache(
         path_modified = path.stat().st_mtime
     except OSError:
         path_modified = -1
-    py_info_script = Path(os.path.abspath(__file__)).parent / "_py_info.py"
+    py_info_script = Path(Path(__file__).resolve()).parent / "_py_info.py"
     try:
         py_info_hash: str | None = hashlib.sha256(py_info_script.read_bytes()).hexdigest()
     except OSError:
@@ -132,7 +132,7 @@ def _load_cached_py_info(
     except (KeyError, TypeError):
         py_info_store.remove()
         return None
-    if (sys_exe := py_info.system_executable) is not None and not os.path.exists(sys_exe):
+    if (sys_exe := py_info.system_executable) is not None and not Path(sys_exe).exists():
         py_info_store.remove()
         return None
     return py_info
@@ -147,7 +147,7 @@ def gen_cookie() -> str:
 
 @contextmanager
 def _resolve_py_info_script() -> Generator[Path]:
-    py_info_script = Path(os.path.abspath(__file__)).parent / "_py_info.py"
+    py_info_script = Path(Path(__file__).resolve()).parent / "_py_info.py"
     if py_info_script.is_file():
         yield py_info_script
     else:
@@ -161,7 +161,7 @@ def _resolve_py_info_script() -> Generator[Path]:
             os.close(fd)
             yield Path(tmp)
         finally:
-            os.unlink(tmp)
+            Path(tmp).unlink()
 
 
 def _run_subprocess(

src/py_discovery/_compat.py

@@ -4,7 +4,7 @@
 
 import functools
 import logging
-import os
+import pathlib
 import tempfile
 from typing import Final
 
@@ -14,7 +14,7 @@
 @functools.lru_cache(maxsize=1)
 def fs_is_case_sensitive() -> bool:
     with tempfile.NamedTemporaryFile(prefix="TmP") as tmp_file:
-        result = not os.path.exists(tmp_file.name.lower())
+        result = not pathlib.Path(tmp_file.name.lower()).exists()
     _LOGGER.debug("filesystem is %scase-sensitive", "" if result else "not ")
     return result
 

src/py_discovery/_discovery.py

@@ -43,7 +43,7 @@ def _find_interpreter(
 ) -> PythonInfo | None:
     spec = PythonSpec.from_string_spec(key)
     _LOGGER.info("find interpreter for spec %r", spec)
-    proposed_paths: set[tuple[str, bool]] = set()
+    proposed_paths: set[tuple[str | None, bool]] = set()
     env = os.environ if env is None else env
     for interpreter, impl_must_match in propose_interpreters(spec, try_first_with, cache, env):
         if interpreter is None:  # pragma: no cover
@@ -65,12 +65,12 @@ def _check_exe(path: str, tested_exes: set[str]) -> str | None:
         os.lstat(path)
     except OSError:
         return None
-    exe_raw = os.path.abspath(path)
-    exe_id = fs_path_id(exe_raw)
+    resolved = str(Path(path).resolve())
+    exe_id = fs_path_id(resolved)
     if exe_id in tested_exes:
         return None
     tested_exes.add(exe_id)
-    return exe_raw
+    return str(Path(path).absolute())
 
 
 def _is_new_exe(exe_raw: str, tested_exes: set[str]) -> bool:
@@ -96,7 +96,7 @@ def propose_interpreters(
         return
 
     for py_exe in try_first_with:
-        if exe_raw := _check_exe(os.path.abspath(py_exe), tested_exes):
+        if exe_raw := _check_exe(str(Path(py_exe).resolve()), tested_exes):
             yield PythonInfo.from_exe(exe_raw, cache, env=env), True
 
     if spec.path is not None:
@@ -151,10 +151,10 @@ def get_paths(env: Mapping[str, str]) -> Generator[Path, None, None]:
         except (AttributeError, ValueError):  # pragma: no cover # Windows only (no confstr)
             path = os.defpath
     if path:
-        for p in map(Path, path.split(os.pathsep)):
+        for entry in map(Path, path.split(os.pathsep)):
             with suppress(OSError):
-                if p.is_dir() and next(p.iterdir(), None):
-                    yield p
+                if entry.is_dir() and next(entry.iterdir(), None):
+                    yield entry
 
 
 class LazyPathDump:
@@ -226,7 +226,7 @@ def _resolve_shim(exe_path: str, env: Mapping[str, str]) -> str | None:
 def _resolve_shim_to_binary(exe_name: str, versions_dir: str, env: Mapping[str, str]) -> str | None:
     for version in _active_versions(env):
         resolved = os.path.join(versions_dir, version, "bin", exe_name)
-        if os.path.isfile(resolved) and os.access(resolved, os.X_OK):
+        if Path(resolved).is_file() and os.access(resolved, os.X_OK):
             return resolved
     return None
 
@@ -236,7 +236,7 @@ def _active_versions(env: Mapping[str, str]) -> Generator[str, None, None]:
     if pyenv_version := env.get("PYENV_VERSION"):
         yield from pyenv_version.split(":")
         return
-    if versions := _read_python_version_file(os.getcwd()):
+    if versions := _read_python_version_file(Path.cwd()):
         yield from versions
         return
     if (pyenv_root := env.get("PYENV_ROOT")) and (
@@ -245,18 +245,18 @@ def _active_versions(env: Mapping[str, str]) -> Generator[str, None, None]:
         yield from versions
 
 
-def _read_python_version_file(start: str, *, search_parents: bool = True) -> list[str] | None:
+def _read_python_version_file(start: str | Path, *, search_parents: bool = True) -> list[str] | None:
     """Read a ``.python-version`` file, optionally searching parent directories."""
     current = start
     while True:
-        candidate = os.path.join(current, ".python-version") if os.path.isdir(current) else current
-        if os.path.isfile(candidate):
-            with open(candidate, encoding="utf-8") as f:
-                if versions := [v for line in f if (v := line.strip()) and not v.startswith("#")]:
+        candidate = os.path.join(current, ".python-version") if Path(current).is_dir() else current
+        if Path(candidate).is_file():
+            with Path(candidate).open(encoding="utf-8") as fh:
+                if versions := [v for line in fh if (v := line.strip()) and not v.startswith("#")]:
                     return versions
         if not search_parents:
             return None
-        parent = os.path.dirname(current)
+        parent = Path(current).parent
         if parent == current:
             return None
         current = parent