Skip to content

Commit 585cdc4

Browse files
RonnyPfannschmidtCursor AIclaude
committed
improve: compute meaningful nodeids for paths outside rootdir
- 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 <ai@cursor.sh> Co-authored-by: Anthropic Claude <claude@anthropic.com>
1 parent f859c93 commit 585cdc4

4 files changed

Lines changed: 301 additions & 10 deletions

File tree

changelog/14004.bugfix.rst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,7 @@ Fixed conftest.py fixture scoping when ``testpaths`` points outside ``rootdir``.
33
Previously, fixtures from nested conftest.py files would incorrectly leak to sibling directories
44
when using a relative ``testpaths`` like ``../tests/sdk``.
55

6-
Conftest fixtures are now parsed during Directory collection, using the Directory node's
7-
nodeid for proper scoping.
6+
Conftest fixtures are now parsed during Directory collection, using the Directory node for
7+
proper scoping. Additionally, nodeids for paths outside ``rootdir`` are now computed more
8+
meaningfully: paths in site-packages use a ``site://`` prefix, nearby paths use relative
9+
paths with ``..`` components, and far-away paths use absolute paths.

src/_pytest/fixtures.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1672,8 +1672,15 @@ def pytest_make_collect_report(
16721672

16731673
def _getautousenames(self, node: nodes.Node) -> Iterator[str]:
16741674
"""Return the names of autouse fixtures applicable to node."""
1675+
seen_nodeids: set[str] = set()
16751676
for parentnode in node.listchain():
1676-
basenames = self._nodeid_autousenames.get(parentnode.nodeid)
1677+
nodeid = parentnode.nodeid
1678+
# Avoid yielding duplicates when multiple nodes share the same nodeid
1679+
# (e.g., Session and root Directory both have nodeid "").
1680+
if nodeid in seen_nodeids:
1681+
continue
1682+
seen_nodeids.add(nodeid)
1683+
basenames = self._nodeid_autousenames.get(nodeid)
16771684
if basenames:
16781685
yield from basenames
16791686

src/_pytest/nodes.py

Lines changed: 130 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from _pytest.mark.structures import NodeKeywords
3939
from _pytest.outcomes import fail
4040
from _pytest.pathlib import absolutepath
41+
from _pytest.pathlib import bestrelpath
4142
from _pytest.stash import Stash
4243
from _pytest.warning_types import PytestWarning
4344

@@ -571,6 +572,129 @@ def _check_initialpaths_for_relpath(
571572
return None
572573

573574

575+
def _get_site_packages_dirs() -> frozenset[Path]:
576+
"""Get all site-packages directories as resolved absolute paths."""
577+
import site
578+
579+
dirs: set[Path] = set()
580+
# Standard site-packages
581+
for sp in site.getsitepackages():
582+
try:
583+
dirs.add(Path(sp).resolve())
584+
except OSError:
585+
pass
586+
# User site-packages
587+
user_site = site.getusersitepackages()
588+
if user_site:
589+
try:
590+
dirs.add(Path(user_site).resolve())
591+
except OSError:
592+
pass
593+
return frozenset(dirs)
594+
595+
596+
# Cache site-packages dirs since they don't change during a run.
597+
_SITE_PACKAGES_DIRS: frozenset[Path] | None = None
598+
599+
600+
def _get_cached_site_packages_dirs() -> frozenset[Path]:
601+
"""Get cached site-packages directories."""
602+
global _SITE_PACKAGES_DIRS
603+
if _SITE_PACKAGES_DIRS is None:
604+
_SITE_PACKAGES_DIRS = _get_site_packages_dirs()
605+
return _SITE_PACKAGES_DIRS
606+
607+
608+
def _path_in_site_packages(
609+
path: Path,
610+
site_packages: frozenset[Path] | None = None,
611+
) -> tuple[Path, Path] | None:
612+
"""Check if path is inside a site-packages directory.
613+
614+
:param path: The path to check.
615+
:param site_packages: Optional set of site-packages directories to check against.
616+
If None, uses the cached system site-packages directories.
617+
Returns (site_packages_dir, relative_path) if found, None otherwise.
618+
"""
619+
if site_packages is None:
620+
site_packages = _get_cached_site_packages_dirs()
621+
try:
622+
resolved = path.resolve()
623+
except OSError:
624+
return None
625+
626+
for sp in site_packages:
627+
try:
628+
rel = resolved.relative_to(sp)
629+
return (sp, rel)
630+
except ValueError:
631+
continue
632+
return None
633+
634+
635+
def compute_nodeid_prefix_for_path(
636+
path: Path,
637+
rootpath: Path,
638+
invocation_dir: Path,
639+
initial_paths: frozenset[Path],
640+
site_packages: frozenset[Path] | None = None,
641+
) -> str:
642+
"""Compute a nodeid prefix for a filesystem path.
643+
644+
The nodeid prefix is computed based on the path's relationship to:
645+
1. rootpath - if relative, use simple relative path
646+
2. initial_paths - if relative to an initial path, use that
647+
3. site-packages - use "site://<package>/<path>" prefix
648+
4. invocation_dir - if close by, use relative path with ".." components
649+
5. Otherwise, use absolute path
650+
651+
:param path: The path to compute a nodeid prefix for.
652+
:param rootpath: The pytest root path.
653+
:param invocation_dir: The directory from which pytest was invoked.
654+
:param initial_paths: The initial paths (testpaths or command line args).
655+
:param site_packages: Optional set of site-packages directories. If None,
656+
uses the cached system site-packages directories.
657+
658+
The returned string uses forward slashes as separators regardless of OS.
659+
"""
660+
# 1. Try relative to rootpath (simplest case)
661+
try:
662+
rel = path.relative_to(rootpath)
663+
result = str(rel)
664+
if result == ".":
665+
return ""
666+
return result.replace(os.sep, SEP)
667+
except ValueError:
668+
pass
669+
670+
# 2. Try relative to initial_paths
671+
nodeid = _check_initialpaths_for_relpath(initial_paths, path)
672+
if nodeid is not None:
673+
return nodeid.replace(os.sep, SEP) if nodeid else ""
674+
675+
# 3. Check if path is in site-packages
676+
site_info = _path_in_site_packages(path, site_packages)
677+
if site_info is not None:
678+
_sp_dir, rel_path = site_info
679+
result = f"site://{rel_path}"
680+
return result.replace(os.sep, SEP)
681+
682+
# 4. Try relative to invocation_dir if "close by" (i.e., not too many ".." components)
683+
rel_from_invocation = bestrelpath(invocation_dir, path)
684+
# Count the number of ".." components - if it's reasonable, use the relative path
685+
# Also check total path length to avoid overly long relative paths
686+
parts = Path(rel_from_invocation).parts
687+
up_count = sum(1 for p in parts if p == "..")
688+
# Only use relative path if:
689+
# - At most 2 ".." components (close to invocation dir)
690+
# - bestrelpath actually produced a relative path (not the absolute path unchanged)
691+
if up_count <= 2 and rel_from_invocation != str(path):
692+
return rel_from_invocation.replace(os.sep, SEP)
693+
694+
# 5. Fall back to absolute path
695+
return str(path).replace(os.sep, SEP)
696+
697+
574698
class FSCollector(Collector, abc.ABC):
575699
"""Base class for filesystem collectors."""
576700

@@ -611,13 +735,12 @@ def __init__(
611735
session = parent.session
612736

613737
if nodeid is None:
614-
try:
615-
nodeid = str(self.path.relative_to(session.config.rootpath))
616-
except ValueError:
617-
nodeid = _check_initialpaths_for_relpath(session._initialpaths, path)
618-
619-
if nodeid:
620-
nodeid = norm_sep(nodeid)
738+
nodeid = compute_nodeid_prefix_for_path(
739+
path=path,
740+
rootpath=session.config.rootpath,
741+
invocation_dir=session.config.invocation_params.dir,
742+
initial_paths=session._initialpaths,
743+
)
621744

622745
super().__init__(
623746
name=name,

testing/test_nodes.py

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,162 @@ def test_show_wrong_path(private_dir):
169169
)
170170
result = pytester.runpytest()
171171
result.stdout.fnmatch_lines([str(p) + ":*: AssertionError", "*1 failed in *"])
172+
173+
174+
class TestNodeidPrefixComputation:
175+
"""Tests for nodeid prefix computation for paths outside rootdir."""
176+
177+
def test_path_in_site_packages_found(self, tmp_path: Path) -> None:
178+
"""Test _path_in_site_packages finds paths inside site-packages."""
179+
fake_site_packages = tmp_path / "site-packages"
180+
fake_site_packages.mkdir()
181+
pkg_path = fake_site_packages / "mypackage" / "tests" / "test_foo.py"
182+
pkg_path.parent.mkdir(parents=True)
183+
pkg_path.touch()
184+
185+
site_packages = frozenset([fake_site_packages])
186+
result = nodes._path_in_site_packages(pkg_path, site_packages)
187+
188+
assert result is not None
189+
sp_dir, rel_path = result
190+
assert sp_dir == fake_site_packages
191+
assert rel_path == Path("mypackage/tests/test_foo.py")
192+
193+
def test_path_in_site_packages_not_found(self, tmp_path: Path) -> None:
194+
"""Test _path_in_site_packages returns None for paths outside site-packages."""
195+
fake_site_packages = tmp_path / "site-packages"
196+
fake_site_packages.mkdir()
197+
other_path = tmp_path / "other" / "test_foo.py"
198+
other_path.parent.mkdir(parents=True)
199+
other_path.touch()
200+
201+
site_packages = frozenset([fake_site_packages])
202+
result = nodes._path_in_site_packages(other_path, site_packages)
203+
204+
assert result is None
205+
206+
def test_compute_nodeid_inside_rootpath(self, tmp_path: Path) -> None:
207+
"""Test nodeid computation for paths inside rootpath."""
208+
rootpath = tmp_path / "project"
209+
rootpath.mkdir()
210+
test_file = rootpath / "tests" / "test_foo.py"
211+
test_file.parent.mkdir(parents=True)
212+
test_file.touch()
213+
214+
result = nodes.compute_nodeid_prefix_for_path(
215+
path=test_file,
216+
rootpath=rootpath,
217+
invocation_dir=rootpath,
218+
initial_paths=frozenset(),
219+
site_packages=frozenset(),
220+
)
221+
222+
assert result == "tests/test_foo.py"
223+
224+
def test_compute_nodeid_in_initial_paths(self, tmp_path: Path) -> None:
225+
"""Test nodeid computation for paths relative to initial_paths."""
226+
rootpath = tmp_path / "project"
227+
rootpath.mkdir()
228+
tests_dir = tmp_path / "tests"
229+
tests_dir.mkdir()
230+
test_file = tests_dir / "test_foo.py"
231+
test_file.touch()
232+
233+
result = nodes.compute_nodeid_prefix_for_path(
234+
path=test_file,
235+
rootpath=rootpath,
236+
invocation_dir=rootpath,
237+
initial_paths=frozenset([tests_dir]),
238+
site_packages=frozenset(),
239+
)
240+
241+
assert result == "test_foo.py"
242+
243+
def test_compute_nodeid_in_site_packages(self, tmp_path: Path) -> None:
244+
"""Test nodeid computation for paths in site-packages uses site:// prefix."""
245+
rootpath = tmp_path / "project"
246+
rootpath.mkdir()
247+
fake_site_packages = tmp_path / "site-packages"
248+
fake_site_packages.mkdir()
249+
pkg_test = fake_site_packages / "mypackage" / "tests" / "test_foo.py"
250+
pkg_test.parent.mkdir(parents=True)
251+
pkg_test.touch()
252+
253+
result = nodes.compute_nodeid_prefix_for_path(
254+
path=pkg_test,
255+
rootpath=rootpath,
256+
invocation_dir=rootpath,
257+
initial_paths=frozenset(),
258+
site_packages=frozenset([fake_site_packages]),
259+
)
260+
261+
assert result == "site://mypackage/tests/test_foo.py"
262+
263+
def test_compute_nodeid_nearby_relative(self, tmp_path: Path) -> None:
264+
"""Test nodeid computation for nearby paths uses relative path."""
265+
rootpath = tmp_path / "project"
266+
rootpath.mkdir()
267+
sibling = tmp_path / "sibling" / "tests" / "test_foo.py"
268+
sibling.parent.mkdir(parents=True)
269+
sibling.touch()
270+
271+
result = nodes.compute_nodeid_prefix_for_path(
272+
path=sibling,
273+
rootpath=rootpath,
274+
invocation_dir=rootpath,
275+
initial_paths=frozenset(),
276+
site_packages=frozenset(),
277+
)
278+
279+
assert result == "../sibling/tests/test_foo.py"
280+
281+
def test_compute_nodeid_far_away_absolute(self, tmp_path: Path) -> None:
282+
"""Test nodeid computation for far-away paths uses absolute path."""
283+
rootpath = tmp_path / "deep" / "nested" / "project"
284+
rootpath.mkdir(parents=True)
285+
far_away = tmp_path / "other" / "location" / "tests" / "test_foo.py"
286+
far_away.parent.mkdir(parents=True)
287+
far_away.touch()
288+
289+
result = nodes.compute_nodeid_prefix_for_path(
290+
path=far_away,
291+
rootpath=rootpath,
292+
invocation_dir=rootpath,
293+
initial_paths=frozenset(),
294+
site_packages=frozenset(),
295+
)
296+
297+
# Should use absolute path since it's more than 2 levels up
298+
assert result == str(far_away)
299+
300+
def test_compute_nodeid_rootpath_itself(self, tmp_path: Path) -> None:
301+
"""Test nodeid computation for rootpath itself returns empty string."""
302+
rootpath = tmp_path / "project"
303+
rootpath.mkdir()
304+
305+
result = nodes.compute_nodeid_prefix_for_path(
306+
path=rootpath,
307+
rootpath=rootpath,
308+
invocation_dir=rootpath,
309+
initial_paths=frozenset(),
310+
site_packages=frozenset(),
311+
)
312+
313+
assert result == ""
314+
315+
def test_compute_nodeid_initial_path_itself(self, tmp_path: Path) -> None:
316+
"""Test nodeid computation for initial_path itself returns empty string."""
317+
rootpath = tmp_path / "project"
318+
rootpath.mkdir()
319+
tests_dir = tmp_path / "tests"
320+
tests_dir.mkdir()
321+
322+
result = nodes.compute_nodeid_prefix_for_path(
323+
path=tests_dir,
324+
rootpath=rootpath,
325+
invocation_dir=rootpath,
326+
initial_paths=frozenset([tests_dir]),
327+
site_packages=frozenset(),
328+
)
329+
330+
assert result == ""

0 commit comments

Comments
 (0)