diff --git a/changelog/14004.bugfix.rst b/changelog/14004.bugfix.rst new file mode 100644 index 00000000000..0873d8f239b --- /dev/null +++ b/changelog/14004.bugfix.rst @@ -0,0 +1,5 @@ +Fixed conftest.py fixture scoping when ``testpaths`` points outside ``rootdir``. + +Nodeids for paths outside ``rootdir`` are now computed more meaningfully: +paths in site-packages use a ``site://`` prefix, nearby paths use relative +paths with ``..`` components, and far-away paths use absolute paths. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index d8d19fcac6d..7291aa948d4 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1629,14 +1629,13 @@ def pytest_plugin_registered(self, plugin: _PluggyPlugin, plugin_name: str) -> N # case-insensitive systems (Windows) and other normalization issues # (issue #11816). conftestpath = absolutepath(plugin_name) - try: - nodeid = str(conftestpath.parent.relative_to(self.config.rootpath)) - except ValueError: - nodeid = "" - if nodeid == ".": - nodeid = "" - if os.sep != nodes.SEP: - nodeid = nodeid.replace(os.sep, nodes.SEP) + # initial_paths not available yet at plugin registration time, + # so we skip that step and fall back to bestrelpath + nodeid = nodes.compute_nodeid_prefix_for_path( + path=conftestpath.parent, + rootpath=self.config.rootpath, + invocation_dir=self.config.invocation_params.dir, + ) else: nodeid = None @@ -1644,8 +1643,15 @@ def pytest_plugin_registered(self, plugin: _PluggyPlugin, plugin_name: str) -> N def _getautousenames(self, node: nodes.Node) -> Iterator[str]: """Return the names of autouse fixtures applicable to node.""" + seen_nodeids: set[str] = set() for parentnode in node.listchain(): - basenames = self._nodeid_autousenames.get(parentnode.nodeid) + nodeid = parentnode.nodeid + # Avoid yielding duplicates when multiple nodes share the same nodeid + # (e.g., Session and root Directory both have nodeid ""). + if nodeid in seen_nodeids: + continue + seen_nodeids.add(nodeid) + basenames = self._nodeid_autousenames.get(nodeid) if basenames: yield from basenames diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 6690f6ab1f8..6f174e1bffb 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -38,6 +38,7 @@ from _pytest.mark.structures import NodeKeywords from _pytest.outcomes import fail from _pytest.pathlib import absolutepath +from _pytest.pathlib import bestrelpath from _pytest.stash import Stash from _pytest.warning_types import PytestWarning @@ -557,6 +558,121 @@ def _check_initialpaths_for_relpath( return None +def _get_site_packages_dirs() -> frozenset[Path]: + """Get all site-packages directories as resolved absolute paths.""" + import site + + dirs: set[Path] = set() + # Standard site-packages + for sp in site.getsitepackages(): + try: + dirs.add(Path(sp).resolve()) + except OSError: + pass + # User site-packages + user_site = site.getusersitepackages() + if user_site: + try: + dirs.add(Path(user_site).resolve()) + except OSError: + pass + return frozenset(dirs) + + +# Cache site-packages dirs since they don't change during a run. +_SITE_PACKAGES_DIRS: frozenset[Path] | None = None + + +def _get_cached_site_packages_dirs() -> frozenset[Path]: + """Get cached site-packages directories.""" + global _SITE_PACKAGES_DIRS + if _SITE_PACKAGES_DIRS is None: + _SITE_PACKAGES_DIRS = _get_site_packages_dirs() + return _SITE_PACKAGES_DIRS + + +def _path_in_site_packages( + path: Path, + site_packages: frozenset[Path] | None = None, +) -> tuple[Path, Path] | None: + """Check if path is inside a site-packages directory. + + :param path: The path to check. + :param site_packages: Optional set of site-packages directories to check against. + If None, uses the cached system site-packages directories. + Returns (site_packages_dir, relative_path) if found, None otherwise. + """ + if site_packages is None: + site_packages = _get_cached_site_packages_dirs() + try: + resolved = path.resolve() + except OSError: + return None + + for sp in site_packages: + try: + rel = resolved.relative_to(sp) + return (sp, rel) + except ValueError: + continue + return None + + +def compute_nodeid_prefix_for_path( + path: Path, + rootpath: Path, + invocation_dir: Path, + site_packages: frozenset[Path] | None = None, +) -> str: + """Compute a nodeid prefix for a filesystem path. + + The nodeid prefix is computed based on the path's relationship to: + 1. rootpath - if relative, use simple relative path + 2. site-packages - use "site:///" prefix + 3. invocation_dir - if close by, use relative path with ".." components + 4. Otherwise, use absolute path + + :param path: The path to compute a nodeid prefix for. + :param rootpath: The pytest root path. + :param invocation_dir: The directory from which pytest was invoked. + :param site_packages: Optional set of site-packages directories. If None, + uses the cached system site-packages directories. + + The returned string uses forward slashes as separators regardless of OS. + """ + # 1. Try relative to rootpath (simplest case) + try: + rel = path.relative_to(rootpath) + result = str(rel) + if result == ".": + return "" + return result.replace(os.sep, SEP) + except ValueError: + pass + + # 2. Check if path is in site-packages + site_info = _path_in_site_packages(path, site_packages) + if site_info is not None: + _sp_dir, rel_path = site_info + result = f"site://{rel_path}" + return result.replace(os.sep, SEP) + + # 3. Try relative to invocation_dir if "close by" (i.e., not too many ".." components) + rel_from_invocation = bestrelpath(invocation_dir, path) + # Count the number of ".." components - if it's reasonable, use the relative path + # Also check total path length to avoid overly long relative paths + parts = Path(rel_from_invocation).parts + up_count = sum(1 for p in parts if p == "..") + # Only use relative path if: + # - At most 2 ".." components (close to invocation dir) + # - bestrelpath actually produced a relative path (not the absolute path unchanged) + if up_count <= 2 and rel_from_invocation != str(path): + return rel_from_invocation.replace(os.sep, SEP) + + # 4. Fall back to absolute path + return str(path).replace(os.sep, SEP) + + class FSCollector(Collector, abc.ABC): """Base class for filesystem collectors.""" @@ -597,13 +713,11 @@ def __init__( session = parent.session if nodeid is None: - try: - nodeid = str(self.path.relative_to(session.config.rootpath)) - except ValueError: - nodeid = _check_initialpaths_for_relpath(session._initialpaths, path) - - if nodeid and os.sep != SEP: - nodeid = nodeid.replace(os.sep, SEP) + nodeid = compute_nodeid_prefix_for_path( + path=path, + rootpath=session.config.rootpath, + invocation_dir=session.config.invocation_params.dir, + ) super().__init__( name=name, diff --git a/testing/test_nodes.py b/testing/test_nodes.py index de7875ca427..da24535c127 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -138,3 +138,137 @@ def test_show_wrong_path(private_dir): ) result = pytester.runpytest() result.stdout.fnmatch_lines([str(p) + ":*: AssertionError", "*1 failed in *"]) + + +class TestNodeidPrefixComputation: + """Tests for nodeid prefix computation for paths outside rootdir.""" + + def test_path_in_site_packages_found(self, tmp_path: Path) -> None: + """Test _path_in_site_packages finds paths inside site-packages.""" + fake_site_packages = tmp_path / "site-packages" + fake_site_packages.mkdir() + pkg_path = fake_site_packages / "mypackage" / "tests" / "test_foo.py" + pkg_path.parent.mkdir(parents=True) + pkg_path.touch() + + site_packages = frozenset([fake_site_packages]) + result = nodes._path_in_site_packages(pkg_path, site_packages) + + assert result is not None + sp_dir, rel_path = result + assert sp_dir == fake_site_packages + assert rel_path == Path("mypackage/tests/test_foo.py") + + def test_path_in_site_packages_not_found(self, tmp_path: Path) -> None: + """Test _path_in_site_packages returns None for paths outside site-packages.""" + fake_site_packages = tmp_path / "site-packages" + fake_site_packages.mkdir() + other_path = tmp_path / "other" / "test_foo.py" + other_path.parent.mkdir(parents=True) + other_path.touch() + + site_packages = frozenset([fake_site_packages]) + result = nodes._path_in_site_packages(other_path, site_packages) + + assert result is None + + def test_compute_nodeid_inside_rootpath(self, tmp_path: Path) -> None: + """Test nodeid computation for paths inside rootpath.""" + rootpath = tmp_path / "project" + rootpath.mkdir() + test_file = rootpath / "tests" / "test_foo.py" + test_file.parent.mkdir(parents=True) + test_file.touch() + + result = nodes.compute_nodeid_prefix_for_path( + path=test_file, + rootpath=rootpath, + invocation_dir=rootpath, + site_packages=frozenset(), + ) + + assert result == "tests/test_foo.py" + + def test_compute_nodeid_outside_rootpath(self, tmp_path: Path) -> None: + """Test nodeid computation for paths outside rootpath uses bestrelpath.""" + rootpath = tmp_path / "project" + rootpath.mkdir() + tests_dir = tmp_path / "tests" + tests_dir.mkdir() + test_file = tests_dir / "test_foo.py" + test_file.touch() + + result = nodes.compute_nodeid_prefix_for_path( + path=test_file, + rootpath=rootpath, + invocation_dir=rootpath, + site_packages=frozenset(), + ) + + # Uses bestrelpath since outside rootpath + assert result == "../tests/test_foo.py" + + def test_compute_nodeid_in_site_packages(self, tmp_path: Path) -> None: + """Test nodeid computation for paths in site-packages uses site:// prefix.""" + rootpath = tmp_path / "project" + rootpath.mkdir() + fake_site_packages = tmp_path / "site-packages" + fake_site_packages.mkdir() + pkg_test = fake_site_packages / "mypackage" / "tests" / "test_foo.py" + pkg_test.parent.mkdir(parents=True) + pkg_test.touch() + + result = nodes.compute_nodeid_prefix_for_path( + path=pkg_test, + rootpath=rootpath, + invocation_dir=rootpath, + site_packages=frozenset([fake_site_packages]), + ) + + assert result == "site://mypackage/tests/test_foo.py" + + def test_compute_nodeid_nearby_relative(self, tmp_path: Path) -> None: + """Test nodeid computation for nearby paths uses relative path.""" + rootpath = tmp_path / "project" + rootpath.mkdir() + sibling = tmp_path / "sibling" / "tests" / "test_foo.py" + sibling.parent.mkdir(parents=True) + sibling.touch() + + result = nodes.compute_nodeid_prefix_for_path( + path=sibling, + rootpath=rootpath, + invocation_dir=rootpath, + ) + + assert result == "../sibling/tests/test_foo.py" + + def test_compute_nodeid_far_away_absolute(self, tmp_path: Path) -> None: + """Test nodeid computation for far-away paths uses absolute path.""" + rootpath = tmp_path / "deep" / "nested" / "project" + rootpath.mkdir(parents=True) + far_away = tmp_path / "other" / "location" / "tests" / "test_foo.py" + far_away.parent.mkdir(parents=True) + far_away.touch() + + result = nodes.compute_nodeid_prefix_for_path( + path=far_away, + rootpath=rootpath, + invocation_dir=rootpath, + ) + + # Should use absolute path since it's more than 2 levels up + assert result == str(far_away) + + def test_compute_nodeid_rootpath_itself(self, tmp_path: Path) -> None: + """Test nodeid computation for rootpath itself returns empty string.""" + rootpath = tmp_path / "project" + rootpath.mkdir() + + result = nodes.compute_nodeid_prefix_for_path( + path=rootpath, + rootpath=rootpath, + invocation_dir=rootpath, + ) + + assert result == ""