diff --git a/AUTHORS b/AUTHORS index f3c8d016c28..4cf19209192 100644 --- a/AUTHORS +++ b/AUTHORS @@ -170,6 +170,7 @@ Fabien Zarifian Fabio Zadrozny Farbod Ahmadian faph +Fazeel Usmani Felix Hofstätter Felix Nieuwenhuizen Feng Ma diff --git a/changelog/12303.bugfix.rst b/changelog/12303.bugfix.rst new file mode 100644 index 00000000000..19bf96540d9 --- /dev/null +++ b/changelog/12303.bugfix.rst @@ -0,0 +1,7 @@ +Fixed ``--import-mode=importlib`` shadowing stdlib and installed packages when test directories share their name (e.g. ``test/``). + +Test modules whose top-level directory collides with an external package are now +registered in ``sys.modules`` under a ``_pytest_shadow_`` prefix (e.g. +``_pytest_shadow_test.test_demo`` instead of ``test.test_demo``). This is an +internal detail and should not affect test behaviour, but it will appear in +tracebacks and ``__name__``. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 3619d8cd3fc..68be2739ff7 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -11,6 +11,7 @@ from errno import ENOENT from errno import ENOTDIR import fnmatch +from functools import cache from functools import partial from importlib.machinery import ModuleSpec from importlib.machinery import PathFinder @@ -542,19 +543,29 @@ def import_path( except CouldNotResolvePathError: pass else: - # If the given module name is already in sys.modules, do not import it again. - with contextlib.suppress(KeyError): - return sys.modules[module_name] - - mod = _import_module_using_spec( - module_name, path, pkg_root, insert_modules=False - ) - if mod is not None: - return mod + # Skip dotted names that would shadow stdlib/installed packages (#12303). + if "." not in module_name or not _top_level_shadows_external( + module_name, pkg_root + ): + # If the given module name is already in sys.modules, do not import it again. + with contextlib.suppress(KeyError): + return sys.modules[module_name] + + mod = _import_module_using_spec( + module_name, path, pkg_root, insert_modules=False + ) + if mod is not None: + return mod # Could not import the module with the current sys.path, so we fall back # to importing the file as a single module, not being a part of a package. module_name = module_name_from_path(path, root) + + # Prefix to avoid shadowing stdlib/installed packages (#12303). + if "." in module_name and _top_level_shadows_external(module_name, root): + top, _, rest = module_name.partition(".") + module_name = f"_pytest_shadow_{top}.{rest}" + with contextlib.suppress(KeyError): return sys.modules[module_name] @@ -754,6 +765,112 @@ def spec_matches_module_path(module_spec: ModuleSpec | None, module_path: Path) return False +def _top_level_shadows_external(module_name: str, local_root: Path) -> bool: + """Return True if the top-level component of *module_name* would collide + with a stdlib or installed package that lives outside *local_root*. + See #12303.""" + top = module_name.partition(".")[0] + return _top_shadows_external_cached(top, local_root) + + +@cache +def _top_shadows_external_cached(top: str, local_root: Path) -> bool: + """Cached core of :func:`_top_level_shadows_external`. + + Keyed on the top-level name and root so that every test file under the + same directory reuses a single ``find_spec`` + ``iterdir`` result.""" + try: + with warnings.catch_warnings(): + warnings.simplefilter("ignore", ImportWarning) + existing_spec = importlib.util.find_spec(top) + except (ImportError, ValueError, AttributeError): + return False + if existing_spec is None: + return False + + # Built-in or frozen modules are always external. + if existing_spec.origin in ("built-in", "frozen"): + return True + + # Also pick up dirs whose name normalizes to `top` (e.g. ".tests" → "_tests"). + # Resolve everything so symlinks like /var → /private/var don't trip us up. + local_candidates: list[Path] = [local_root / top] + try: + for child in local_root.iterdir(): + if ( + child.is_dir() + and child.name.replace(".", "_") == top + and child not in local_candidates + ): + local_candidates.append(child) + except OSError: + pass + resolved_candidates = [c.resolve() for c in local_candidates if c.exists()] + + def _is_local(p: Path) -> bool: + rp = p.resolve() + for candidate in resolved_candidates: + try: + rp.relative_to(candidate) + return True + except ValueError: + pass + return False + + # Check whether the spec found by find_spec points into local_root. + spec_is_local = False + if existing_spec.origin is not None: + if _is_local(Path(existing_spec.origin)): + spec_is_local = True + if not spec_is_local and existing_spec.submodule_search_locations: + for loc in existing_spec.submodule_search_locations: + if _is_local(Path(loc)): + spec_is_local = True + break + + if not spec_is_local: + # The module found by find_spec lives outside local_root → shadow. + return True + + # find_spec returned the local package (e.g. because the project root is + # reachable via '' or an explicit sys.path entry). An external module + # with the same name may still exist behind it. Search only non-local + # sys.path entries via PathFinder so we never modify sys.path. + local_root_resolved = local_root.resolve() + + def _is_local_path_entry(entry: str) -> bool: + p = Path.cwd() if entry == "" else Path(entry) + try: + p.resolve().relative_to(local_root_resolved) + return True + except ValueError: + return False + + non_local = [p for p in sys.path if not _is_local_path_entry(p)] + try: + behind = PathFinder.find_spec(top, path=non_local) + except (ImportError, ValueError, AttributeError): + behind = None + + if behind is None: + return False + + # Guard against namespace packages that span multiple project directories + # (e.g. dist1/com + dist2/com). If the "behind" spec's locations are all + # already present in the original spec, it is the same package — not a + # genuinely external shadow. + def _spec_locations(spec: ModuleSpec) -> set[str]: + locs: set[str] = set() + if spec.origin is not None: + locs.add(str(Path(spec.origin).resolve())) + if spec.submodule_search_locations: + for loc in spec.submodule_search_locations: + locs.add(str(Path(loc).resolve())) + return locs + + return bool(_spec_locations(behind) - _spec_locations(existing_spec)) + + # Implement a special _is_same function on Windows which returns True if the two filenames # compare equal, to circumvent os.path.samefile returning False for mounts in UNC (#7678). if sys.platform.startswith("win"): @@ -817,8 +934,10 @@ def insert_missing_modules(modules: dict[str, ModuleType], module_name: str) -> # a warning and raise ModuleNotFoundError. To avoid the # warning, we check sys.meta_path explicitly and raise the error # ourselves to fall back to creating a dummy module. - if not sys.meta_path: + if not sys.meta_path: # pragma: no cover raise ModuleNotFoundError + # May import an unrelated module on name collision; + # callers use the _pytest_shadow_ prefix to avoid this (#12303). parent_module = importlib.import_module(parent_module_name) except ModuleNotFoundError: parent_module = ModuleType( diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index be20f3fea48..2c9b04b7782 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -20,6 +20,8 @@ from _pytest.config import ExitCode from _pytest.monkeypatch import MonkeyPatch from _pytest.pathlib import _import_module_using_spec +from _pytest.pathlib import _top_level_shadows_external +from _pytest.pathlib import _top_shadows_external_cached from _pytest.pathlib import bestrelpath from _pytest.pathlib import commonpath from _pytest.pathlib import compute_module_name @@ -973,10 +975,171 @@ def test_demo(): consider_namespace_packages=ns_param, ) + # stdlib module must still be the real one, not a local dummy (#12303). + stdlib_mod = sys.modules[name] + assert not (getattr(stdlib_mod, "__file__", "") or "").startswith( + str(pytester.path) + ) + # E2E test result = pytester.runpytest("--import-mode=importlib") result.stdout.fnmatch_lines("* 1 passed *") + @pytest.mark.parametrize("subdir", ["", "subdir/"]) + def test_importlib_does_not_shadow_stdlib( + self, pytester, ns_param: bool, subdir: str + ): + """Regression test for #12303.""" + file_path = pytester.path / f"test/{subdir}test_demo.py" + file_path.parent.mkdir(parents=True) + file_path.write_text( + dedent( + """ + import test.support + + def test_stdlib_accessible(): + assert hasattr(test.support, "verbose") + # Must be the real stdlib, not a pytest-generated dummy. + assert "site-packages" not in (getattr(test, "__file__", "") or "") + """ + ), + encoding="utf-8", + ) + + ns_opt = str(ns_param).lower() + result = pytester.runpytest_subprocess( + "--import-mode=importlib", + "-o", + f"consider_namespace_packages={ns_opt}", + ) + result.stdout.fnmatch_lines("* 1 passed *") + + def test_importlib_does_not_shadow_installed_package( + self, pytester, ns_param: bool + ): + """Regression test for #12303 (installed packages).""" + pytest.importorskip("packaging.version") + + file_path = pytester.path / "packaging/test_demo.py" + file_path.parent.mkdir(parents=True) + file_path.write_text( + dedent( + """ + from packaging.version import Version + + def test_installed_pkg(): + assert str(Version("1.0")) == "1.0" + """ + ), + encoding="utf-8", + ) + + ns_opt = str(ns_param).lower() + result = pytester.runpytest_subprocess( + "--import-mode=importlib", + "-o", + f"consider_namespace_packages={ns_opt}", + ) + result.stdout.fnmatch_lines("* 1 passed *") + + def test_importlib_canonical_name_preserved( + self, pytester, ns_param: bool, monkeypatch: MonkeyPatch + ): + """Shadow detection must not fire for real local packages (#12303).""" + pkg = pytester.path / "myapp" + pkg.mkdir() + (pkg / "__init__.py").touch() + test_file = pkg / "test_core.py" + test_file.write_text( + dedent( + """ + def test_canonical(): + pass + """ + ), + encoding="utf-8", + ) + # Make the package importable via sys.path. + monkeypatch.syspath_prepend(pytester.path) + + mod = import_path( + test_file, + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=ns_param, + ) + # Must use the real dotted name, not a shadow-prefixed name. + assert mod.__name__ == "myapp.test_core" + + def test_importlib_shadow_skips_standard_import_path( + self, pytester, ns_param: bool + ): + """When test/__init__.py exists AND the name shadows stdlib, + the standard-import path (Path 1) must be skipped so that + the fallback prefixes the name instead. This exercises the + shadow check inside the ``else`` block of + ``resolve_pkg_root_and_module_name`` (#12303).""" + test_dir = pytester.path / "test" + test_dir.mkdir() + (test_dir / "__init__.py").touch() + file_path = test_dir / "test_demo.py" + file_path.write_text( + dedent( + """ + def test_passes(): + pass + """ + ), + encoding="utf-8", + ) + + mod = import_path( + file_path, + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=ns_param, + ) + # Must NOT be imported as "test.test_demo" (would shadow stdlib). + assert not mod.__name__.startswith("test.") + + def test_importlib_path1_spec_returns_none_fallback( + self, pytester: Pytester, ns_param: bool, monkeypatch: MonkeyPatch + ) -> None: + """When _import_module_using_spec returns None in path 1 (insert_modules=False), + import_path must fall through to path 2 and still import the module (#12303).""" + import _pytest.pathlib as pathlib_module + + pkg = pytester.path / "mypkg" + pkg.mkdir() + (pkg / "__init__.py").touch() + test_file = pkg / "test_core.py" + test_file.write_text("def test(): pass", encoding="ascii") + monkeypatch.syspath_prepend(pytester.path) + + real_fn = pathlib_module._import_module_using_spec + + def _mock( + module_name: str, + path: Any, + module_location: Any, + *, + insert_modules: bool, + ) -> Any: + if module_name == "mypkg.test_core" and not insert_modules: + return None # simulate path-1 spec failure → triggers 557->562 branch + return real_fn( + module_name, path, module_location, insert_modules=insert_modules + ) + + monkeypatch.setattr(pathlib_module, "_import_module_using_spec", _mock) + mod = import_path( + test_file, + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=ns_param, + ) + assert mod.__name__ == "mypkg.test_core" + def create_installed_doctests_and_tests_dir( self, path: Path, monkeypatch: MonkeyPatch ) -> tuple[Path, Path, Path]: @@ -1736,6 +1899,164 @@ def test_compute_module_name(tmp_path: Path) -> None: ) +class TestTopLevelShadowsExternal: + """Unit tests for ``_top_level_shadows_external`` to cover all branches.""" + + def test_no_external_module(self, tmp_path: Path) -> None: + """When find_spec returns None (no such external module), return False.""" + (tmp_path / "zzz_nonexistent_pkg").mkdir() + assert not _top_level_shadows_external("zzz_nonexistent_pkg.foo", tmp_path) + + def test_builtin_or_frozen(self, tmp_path: Path) -> None: + """Built-in/frozen modules (e.g. 'sys', 'os') are always external.""" + (tmp_path / "sys").mkdir() + assert _top_level_shadows_external("sys.something", tmp_path) + + def test_find_spec_raises(self, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + """When find_spec raises, treat as no external module (return False).""" + import importlib.util + + def _raise(*a: Any, **kw: Any) -> None: + raise ValueError("broken") + + monkeypatch.setattr(importlib.util, "find_spec", _raise) + assert not _top_level_shadows_external("whatever.foo", tmp_path) + + def test_normalized_dir_name( + self, tmp_path: Path, monkeypatch: MonkeyPatch + ) -> None: + """A dir named '.xmod' normalizes to '_xmod' and should be + recognized as local when the spec points inside it.""" + dot_dir = tmp_path / ".xmod" + dot_dir.mkdir() + + # Fake an already-imported module whose search location is inside + # the normalized dir — this exercises the ``iterdir`` + + # ``name.replace(".", "_")`` branch (line 795 of pathlib.py). + dummy = ModuleType("_xmod") + dummy.__path__ = [str(dot_dir)] + spec = importlib.machinery.ModuleSpec("_xmod", None, origin=None) + spec.submodule_search_locations = [str(dot_dir)] + dummy.__spec__ = spec + monkeypatch.setitem(sys.modules, "_xmod", dummy) + + assert not _top_level_shadows_external("_xmod.foo", tmp_path) + + def test_external_package_detected(self, tmp_path: Path) -> None: + """An installed package at a different location is external.""" + (tmp_path / "test").mkdir() + assert _top_level_shadows_external("test.support", tmp_path) + + def test_external_detected_when_root_on_sys_path( + self, tmp_path: Path, monkeypatch: MonkeyPatch + ) -> None: + """When local_root is on sys.path (e.g. via '' / CWD) and test/ + has __init__.py, find_spec sees the local package first. + The function must still detect the stdlib shadow behind it.""" + test_dir = tmp_path / "test" + test_dir.mkdir() + (test_dir / "__init__.py").touch() + monkeypatch.syspath_prepend(tmp_path) + _top_shadows_external_cached.cache_clear() + try: + assert _top_level_shadows_external("test.test_demo", tmp_path) + finally: + _top_shadows_external_cached.cache_clear() + + def test_local_package_not_external( + self, tmp_path: Path, monkeypatch: MonkeyPatch + ) -> None: + """A package whose spec resolves inside local_root is not external.""" + pkg = tmp_path / "mypkg" + pkg.mkdir() + (pkg / "__init__.py").touch() + monkeypatch.syspath_prepend(tmp_path) + assert not _top_level_shadows_external("mypkg.sub", tmp_path) + + def test_pathfinder_find_spec_raises( + self, tmp_path: Path, monkeypatch: MonkeyPatch + ) -> None: + """When PathFinder.find_spec raises after the initial find_spec + returned a local spec, treat as no external shadow (return False).""" + from importlib.machinery import PathFinder + + # Set up a local package so find_spec returns a local spec + # and the code reaches the PathFinder.find_spec call. + pkg = tmp_path / "test" + pkg.mkdir() + (pkg / "__init__.py").touch() + monkeypatch.syspath_prepend(tmp_path) + _top_shadows_external_cached.cache_clear() + + real_pathfinder_find_spec = PathFinder.find_spec + + def _raise(name: str, path: Any = None, target: Any = None) -> Any: + if path is not None: + # The "behind" search — raise to exercise the except branch. + raise ValueError("broken PathFinder") + return real_pathfinder_find_spec(name, path, target) + + monkeypatch.setattr(PathFinder, "find_spec", staticmethod(_raise)) + try: + # With PathFinder broken, the function can't find anything + # behind the local spec → returns False. + assert not _top_level_shadows_external("test.test_demo", tmp_path) + finally: + _top_shadows_external_cached.cache_clear() + + def test_iterdir_oserror(self, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + """When iterdir raises (e.g. PermissionError), fall through gracefully.""" + (tmp_path / "test").mkdir() + _top_shadows_external_cached.cache_clear() + + def _broken_iterdir(self: Path) -> Any: + raise PermissionError("denied") + + monkeypatch.setattr(Path, "iterdir", _broken_iterdir) + # Should still detect the shadow (stdlib test exists externally) + # even though iterdir failed — the base candidate local_root/top + # is added unconditionally before iterdir. + try: + assert _top_level_shadows_external("test.support", tmp_path) + finally: + _top_shadows_external_cached.cache_clear() + + def test_spec_locations_degenerate_behind( + self, tmp_path: Path, monkeypatch: MonkeyPatch + ) -> None: + """When the 'behind' spec has no origin and no submodule_search_locations + (degenerate namespace package), _spec_locations returns an empty set so + no genuine external shadow is reported. + + This exercises the False-branch of both + ``if spec.origin is not None:`` and ``if spec.submodule_search_locations:`` + inside _spec_locations (#12303).""" + from importlib.machinery import PathFinder + + test_dir = tmp_path / "test" + test_dir.mkdir() + (test_dir / "__init__.py").touch() + monkeypatch.syspath_prepend(tmp_path) + _top_shadows_external_cached.cache_clear() + + # A spec with no origin and no submodule_search_locations. + degenerate = importlib.machinery.ModuleSpec("test", None, origin=None) + + real_find_spec = PathFinder.find_spec + + def _degenerate_behind(name: str, path: Any = None, target: Any = None) -> Any: + if path is not None: + return degenerate + return real_find_spec(name, path, target) + + monkeypatch.setattr(PathFinder, "find_spec", staticmethod(_degenerate_behind)) + try: + # _spec_locations(behind) == {} → no new external locations → no shadow. + assert not _top_level_shadows_external("test.demo", tmp_path) + finally: + _top_shadows_external_cached.cache_clear() + + def validate_namespace_package( pytester: Pytester, paths: Sequence[Path], modules: Sequence[str] ) -> RunResult: