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 84f90f946be..e8d7b458869 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1629,23 +1629,107 @@ 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 = "" - elif nodeid: - nodeid = nodes.norm_sep(nodeid) + nodeid = self._compute_conftest_nodeid(conftestpath.parent) else: nodeid = None self.parsefactories(plugin, nodeid) + def _compute_conftest_nodeid(self, conftest_dir: Path) -> str: + """Compute nodeid for a conftest directory. + + The nodeid must match how FSCollector computes nodeids so that + fixture scoping works correctly. This is especially important when + testpaths points outside rootdir (issue #14004). + + This mirrors the logic in FSCollector.__init__: + 1. Try relative to rootpath + 2. Fall back to _check_initialpaths_for_relpath logic + """ + rootpath = self.config.rootpath + invocation_dir = self.config.invocation_params.dir + + # First, try relative to rootpath (same as FSCollector) + try: + nodeid = str(conftest_dir.relative_to(rootpath)) + if nodeid == ".": + return "" + return nodes.norm_sep(nodeid) + except ValueError: + pass + + # Path is outside rootpath. Use the same logic as FSCollector's + # _check_initialpaths_for_relpath fallback. + # + # During collection, _initialpaths is available and contains the + # resolved collection targets (including --pyargs targets). This + # allows conftests in those targets to get the correct nodeid. + initialpaths = self.session._initialpaths + if initialpaths: + # Same logic as _check_initialpaths_for_relpath in nodes.py + if conftest_dir in initialpaths: + return "" + for initialpath in initialpaths: + try: + rel = conftest_dir.relative_to(initialpath) + nodeid = str(rel) + if nodeid == ".": + return "" + return nodes.norm_sep(nodeid) + except ValueError: + continue + + # During initial conftest loading (before collection), _initialpaths + # is empty. Fall back to checking testpaths configuration. + testpaths = self.config.getini("testpaths") + if testpaths: + for testpath_str in testpaths: + # Resolve testpath relative to invocation_dir + testpath = (invocation_dir / testpath_str).resolve() + # Only consider testpaths that are outside rootpath + try: + testpath.relative_to(rootpath) + # testpath is under rootpath, skip + continue + except ValueError: + pass + # testpath is outside rootpath, check if conftest is under it + try: + rel = conftest_dir.relative_to(testpath) + nodeid = str(rel) + if nodeid == ".": + return "" + return nodes.norm_sep(nodeid) + except ValueError: + continue + + # Path is outside rootpath, not under any initialpath or testpath. + # Check if rootpath is under conftest_dir (conftest is a parent). + # In this case, the conftest should be global (nodeid=""). + try: + rootpath.relative_to(conftest_dir) + # rootpath is under conftest_dir, so conftest is a parent + return "" + except ValueError: + pass + + # For all other cases (e.g., site-packages, unrelated paths), + # return empty nodeid so fixtures are globally visible. + # This matches the behavior when FSCollector's + # _check_initialpaths_for_relpath returns None. + return "" + 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 bc1dfc90d96..e006d2d8fd3 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 @@ -571,6 +572,127 @@ 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() + + def add_from(getter: Callable[[], list[str]]) -> None: + """Call getter and add resolved paths to dirs. + + Handles AttributeError (function missing in some virtualenvs) + and OSError (path resolution failures). + """ + try: + paths = getter() + except AttributeError: + return + for p in paths: + try: + dirs.add(Path(p).resolve()) + except OSError: + pass + + add_from(lambda: site.getsitepackages()) + add_from(lambda: [site.getusersitepackages()]) + 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.""" @@ -612,7 +734,7 @@ def __init__( if nodeid is None: try: - nodeid = str(self.path.relative_to(session.config.rootpath)) + nodeid = str(path.relative_to(session.config.rootpath)) except ValueError: nodeid = _check_initialpaths_for_relpath(session._initialpaths, path) diff --git a/testing/test_conftest.py b/testing/test_conftest.py index 4de61bceb90..350947fb564 100644 --- a/testing/test_conftest.py +++ b/testing/test_conftest.py @@ -779,3 +779,161 @@ def pytest_addoption(parser): result = pytester.runpytest("-h", x) result.stdout.no_fnmatch_line("*argument --xyz is required*") assert "general:" in result.stdout.str() + + +def test_conftest_outside_rootdir_fixture_scoping(pytester: Pytester) -> None: + """Test that conftest fixtures are correctly scoped when testpaths points outside rootdir. + + Regression test for issue #14004: conftest fixtures from nested directories + should not leak to sibling directories when testpaths points outside rootdir. + """ + # Structure: + # sdk/ + # pyproject.toml (rootdir, testpaths = ["../tests/sdk"]) + # tests/ + # sdk/ + # conftest.py (outer_fixture) + # test_outer.py + # inner/ + # conftest.py (inner_fixture - would fail if applied to outer) + # test_inner.py + + sdk = pytester.mkdir("sdk") + tests_sdk = pytester.mkdir("tests").joinpath("sdk") + tests_sdk.mkdir() + inner = tests_sdk.joinpath("inner") + inner.mkdir() + + # Config in sdk/ pointing to ../tests/sdk + sdk.joinpath("pyproject.toml").write_text( + textwrap.dedent( + """\ + [tool.pytest.ini_options] + testpaths = ["../tests/sdk"] + """ + ), + encoding="utf-8", + ) + + # Outer conftest with autouse fixture + tests_sdk.joinpath("conftest.py").write_text( + textwrap.dedent( + """\ + import pytest + + @pytest.fixture(autouse=True) + def outer_fixture(): + pass # This should run for all tests + """ + ), + encoding="utf-8", + ) + + # Inner conftest with autouse fixture that sets a marker + # If this leaks to outer tests, test_outer will fail + inner.joinpath("conftest.py").write_text( + textwrap.dedent( + """\ + import pytest + + @pytest.fixture(autouse=True) + def inner_fixture(request): + # Mark that inner_fixture ran + request.node.inner_fixture_ran = True + """ + ), + encoding="utf-8", + ) + + # Outer test checks that inner_fixture did NOT run + tests_sdk.joinpath("test_outer.py").write_text( + textwrap.dedent( + """\ + def test_outer(request): + # inner_fixture should NOT have run for this test + assert not hasattr(request.node, 'inner_fixture_ran'), \ + "inner_fixture leaked to outer test!" + """ + ), + encoding="utf-8", + ) + + # Inner test checks that inner_fixture DID run + inner.joinpath("test_inner.py").write_text( + textwrap.dedent( + """\ + def test_inner(request): + # inner_fixture should have run for this test + assert getattr(request.node, 'inner_fixture_ran', False), \ + "inner_fixture did not run for inner test!" + """ + ), + encoding="utf-8", + ) + + # Run from sdk/ directory using testpaths + os.chdir(sdk) + result = pytester.runpytest("-v") + + result.assert_outcomes(passed=2) + + +def test_conftest_symlink_fixture_scoping(pytester: Pytester) -> None: + """Test that conftest fixtures are correctly scoped when using symlinks. + + Ensures that symlinked test directories still have proper conftest scoping. + """ + # Structure: + # real/ + # conftest.py (real_fixture) + # tests/ + # conftest.py (tests_fixture) + # test_it.py + # link -> real/tests (symlink) + + real = pytester.mkdir("real") + real_tests = real.joinpath("tests") + real_tests.mkdir() + + real.joinpath("conftest.py").write_text( + textwrap.dedent( + """\ + import pytest + + @pytest.fixture(autouse=True) + def real_fixture(): + print("REAL_FIXTURE") + """ + ), + encoding="utf-8", + ) + + real_tests.joinpath("conftest.py").write_text( + textwrap.dedent( + """\ + import pytest + + @pytest.fixture(autouse=True) + def tests_fixture(): + print("TESTS_FIXTURE") + """ + ), + encoding="utf-8", + ) + + real_tests.joinpath("test_it.py").write_text( + "def test_it(): pass", + encoding="utf-8", + ) + + # Create symlink to real/tests + link = pytester.path.joinpath("link") + symlink_or_skip(real_tests, link) + + # Running via symlink should still find conftest fixtures in the real directory + os.chdir(pytester.path) + result = pytester.runpytest("-s", "-v", "link") + + # The tests_fixture from real/tests/conftest.py should be found + result.stdout.fnmatch_lines(["*TESTS_FIXTURE*"]) + result.assert_outcomes(passed=1) diff --git a/testing/test_nodes.py b/testing/test_nodes.py index f66a11ce5c8..6c91f47cdc2 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -1,6 +1,7 @@ # mypy: allow-untyped-defs from __future__ import annotations +import os from pathlib import Path import re import warnings @@ -169,3 +170,138 @@ 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 + # Use nodes.SEP for cross-platform compatibility (nodeids always use forward slashes) + assert result == str(far_away).replace(os.sep, nodes.SEP) + + 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 == ""