From a74d72f1b5900f9ea40446c7905d82584ec90800 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 11 Jan 2026 10:59:42 +0100 Subject: [PATCH 1/2] improve: compute meaningful nodeids for paths outside rootdir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add compute_nodeid_prefix_for_path() for better nodeid computation: - Paths in site-packages use site:// prefix - Nearby paths (≤2 levels up) use relative paths with .. - Far-away paths use absolute paths - Add _path_in_site_packages() with optional site_packages parameter for testing - Fix _getautousenames() to skip duplicate nodeids (Session and root Dir both have nodeid='') - Add unit tests for nodeid prefix computation Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude --- changelog/14004.bugfix.rst | 5 ++ src/_pytest/fixtures.py | 9 ++- src/_pytest/nodes.py | 137 ++++++++++++++++++++++++++++++-- testing/test_nodes.py | 159 +++++++++++++++++++++++++++++++++++++ 4 files changed, 302 insertions(+), 8 deletions(-) create mode 100644 changelog/14004.bugfix.rst 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..690106f244d 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1644,8 +1644,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..798e854a335 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,129 @@ 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, + initial_paths: frozenset[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. initial_paths - if relative to an initial path, use that + 3. site-packages - use "site:///" prefix + 4. invocation_dir - if close by, use relative path with ".." components + 5. 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 initial_paths: The initial paths (testpaths or command line args). + :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. Try relative to initial_paths + nodeid = _check_initialpaths_for_relpath(initial_paths, path) + if nodeid is not None: + return nodeid.replace(os.sep, SEP) if nodeid else "" + + # 3. 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) + + # 4. 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) + + # 5. Fall back to absolute path + return str(path).replace(os.sep, SEP) + + class FSCollector(Collector, abc.ABC): """Base class for filesystem collectors.""" @@ -597,13 +721,12 @@ 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, + initial_paths=session._initialpaths, + ) super().__init__( name=name, diff --git a/testing/test_nodes.py b/testing/test_nodes.py index de7875ca427..d8a701dce40 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -138,3 +138,162 @@ 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, + initial_paths=frozenset(), + site_packages=frozenset(), + ) + + assert result == "tests/test_foo.py" + + def test_compute_nodeid_in_initial_paths(self, tmp_path: Path) -> None: + """Test nodeid computation for paths relative to initial_paths.""" + 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, + initial_paths=frozenset([tests_dir]), + site_packages=frozenset(), + ) + + assert result == "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, + initial_paths=frozenset(), + 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, + initial_paths=frozenset(), + site_packages=frozenset(), + ) + + 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, + initial_paths=frozenset(), + site_packages=frozenset(), + ) + + # 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, + initial_paths=frozenset(), + site_packages=frozenset(), + ) + + assert result == "" + + def test_compute_nodeid_initial_path_itself(self, tmp_path: Path) -> None: + """Test nodeid computation for initial_path itself returns empty string.""" + rootpath = tmp_path / "project" + rootpath.mkdir() + tests_dir = tmp_path / "tests" + tests_dir.mkdir() + + result = nodes.compute_nodeid_prefix_for_path( + path=tests_dir, + rootpath=rootpath, + invocation_dir=rootpath, + initial_paths=frozenset([tests_dir]), + site_packages=frozenset(), + ) + + assert result == "" From 1cde9aeb74a709ee78599cf44b3b72035dd74841 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sat, 17 Jan 2026 13:04:36 +0100 Subject: [PATCH 2/2] fix: compute consistent nodeids for paths outside rootpath Fix conftest fixture scoping when testpaths points outside rootdir (#14004). Previously, conftests outside rootpath got nodeid '' (empty string), making their fixtures global and leaking to sibling directories. Changes: - Remove initial_paths from compute_nodeid_prefix_for_path since nodeids relate to collection tree structure, not command-line paths - Use compute_nodeid_prefix_for_path in pytest_plugin_registered for consistent conftest baseid computation - Paths outside rootpath now consistently use bestrelpath from invocation_dir This ensures conftest fixtures are properly scoped regardless of whether tests are inside or outside rootpath. Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Opus 4 --- src/_pytest/fixtures.py | 15 +++++++-------- src/_pytest/nodes.py | 21 ++++++--------------- testing/test_nodes.py | 33 ++++----------------------------- 3 files changed, 17 insertions(+), 52 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 690106f244d..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 diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 798e854a335..6f174e1bffb 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -622,22 +622,19 @@ def compute_nodeid_prefix_for_path( path: Path, rootpath: Path, invocation_dir: Path, - initial_paths: frozenset[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. initial_paths - if relative to an initial path, use that - 3. site-packages - use "site:///" prefix - 4. invocation_dir - if close by, use relative path with ".." components - 5. Otherwise, use absolute 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 initial_paths: The initial paths (testpaths or command line args). :param site_packages: Optional set of site-packages directories. If None, uses the cached system site-packages directories. @@ -653,19 +650,14 @@ def compute_nodeid_prefix_for_path( except ValueError: pass - # 2. Try relative to initial_paths - nodeid = _check_initialpaths_for_relpath(initial_paths, path) - if nodeid is not None: - return nodeid.replace(os.sep, SEP) if nodeid else "" - - # 3. Check if path is in site-packages + # 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) - # 4. Try relative to invocation_dir if "close by" (i.e., not too many ".." components) + # 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 @@ -677,7 +669,7 @@ def compute_nodeid_prefix_for_path( if up_count <= 2 and rel_from_invocation != str(path): return rel_from_invocation.replace(os.sep, SEP) - # 5. Fall back to absolute path + # 4. Fall back to absolute path return str(path).replace(os.sep, SEP) @@ -725,7 +717,6 @@ def __init__( path=path, rootpath=session.config.rootpath, invocation_dir=session.config.invocation_params.dir, - initial_paths=session._initialpaths, ) super().__init__( diff --git a/testing/test_nodes.py b/testing/test_nodes.py index d8a701dce40..da24535c127 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -184,14 +184,13 @@ def test_compute_nodeid_inside_rootpath(self, tmp_path: Path) -> None: path=test_file, rootpath=rootpath, invocation_dir=rootpath, - initial_paths=frozenset(), site_packages=frozenset(), ) assert result == "tests/test_foo.py" - def test_compute_nodeid_in_initial_paths(self, tmp_path: Path) -> None: - """Test nodeid computation for paths relative to initial_paths.""" + 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" @@ -203,11 +202,11 @@ def test_compute_nodeid_in_initial_paths(self, tmp_path: Path) -> None: path=test_file, rootpath=rootpath, invocation_dir=rootpath, - initial_paths=frozenset([tests_dir]), site_packages=frozenset(), ) - assert result == "test_foo.py" + # 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.""" @@ -223,7 +222,6 @@ def test_compute_nodeid_in_site_packages(self, tmp_path: Path) -> None: path=pkg_test, rootpath=rootpath, invocation_dir=rootpath, - initial_paths=frozenset(), site_packages=frozenset([fake_site_packages]), ) @@ -241,8 +239,6 @@ def test_compute_nodeid_nearby_relative(self, tmp_path: Path) -> None: path=sibling, rootpath=rootpath, invocation_dir=rootpath, - initial_paths=frozenset(), - site_packages=frozenset(), ) assert result == "../sibling/tests/test_foo.py" @@ -259,8 +255,6 @@ def test_compute_nodeid_far_away_absolute(self, tmp_path: Path) -> None: path=far_away, rootpath=rootpath, invocation_dir=rootpath, - initial_paths=frozenset(), - site_packages=frozenset(), ) # Should use absolute path since it's more than 2 levels up @@ -275,25 +269,6 @@ def test_compute_nodeid_rootpath_itself(self, tmp_path: Path) -> None: path=rootpath, rootpath=rootpath, invocation_dir=rootpath, - initial_paths=frozenset(), - site_packages=frozenset(), - ) - - assert result == "" - - def test_compute_nodeid_initial_path_itself(self, tmp_path: Path) -> None: - """Test nodeid computation for initial_path itself returns empty string.""" - rootpath = tmp_path / "project" - rootpath.mkdir() - tests_dir = tmp_path / "tests" - tests_dir.mkdir() - - result = nodes.compute_nodeid_prefix_for_path( - path=tests_dir, - rootpath=rootpath, - invocation_dir=rootpath, - initial_paths=frozenset([tests_dir]), - site_packages=frozenset(), ) assert result == ""