diff --git a/changelog/13598.bugfix.rst b/changelog/13598.bugfix.rst new file mode 100644 index 00000000000..dd195112eb8 --- /dev/null +++ b/changelog/13598.bugfix.rst @@ -0,0 +1 @@ +Fixed possible collection confusion on Windows when short paths and symlinks are involved. diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 77d8b52ca46..6e35c887814 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -39,6 +39,7 @@ from _pytest.pathlib import bestrelpath from _pytest.pathlib import fnmatch_ex from _pytest.pathlib import safe_exists +from _pytest.pathlib import samefile_nofollow from _pytest.pathlib import scandir from _pytest.reports import CollectReport from _pytest.reports import TestReport @@ -935,14 +936,10 @@ def collect(self) -> Iterator[nodes.Item | nodes.Collector]: is_match = node.path == matchparts[0] if sys.platform == "win32" and not is_match: # In case the file paths do not match, fallback to samefile() to - # account for short-paths on Windows (#11895). - same_file = os.path.samefile(node.path, matchparts[0]) - # We don't want to match links to the current node, - # otherwise we would match the same file more than once (#12039). - is_match = same_file and ( - os.path.islink(node.path) - == os.path.islink(matchparts[0]) - ) + # account for short-paths on Windows (#11895). But use a version + # which doesn't resolve symlinks, otherwise we might match the + # same file more than once (#12039). + is_match = samefile_nofollow(node.path, matchparts[0]) # Name part e.g. `TestIt` in `/a/b/test_file.py::TestIt::test_it`. else: diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index b69e85404e7..d6b90687a6f 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -1053,3 +1053,11 @@ def safe_exists(p: Path) -> bool: # ValueError: stat: path too long for Windows # OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect return False + + +def samefile_nofollow(p1: Path, p2: Path) -> bool: + """Test whether two paths reference the same actual file or directory. + + Unlike Path.samefile(), does not resolve symlinks. + """ + return os.path.samestat(p1.lstat(), p2.lstat()) diff --git a/testing/test_collection.py b/testing/test_collection.py index 76091744dc6..2214c130a05 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1780,6 +1780,41 @@ def test_collect_short_file_windows(pytester: Pytester) -> None: assert result.parseoutcomes() == {"passed": 1} +def test_collect_short_file_windows_multi_level_symlink( + pytester: Pytester, + request: FixtureRequest, +) -> None: + """Regression test for multi-level Windows short-path comparison with + symlinks. + + Previously, when matching collection arguments against collected nodes on + Windows, the short path fallback resolved symlinks. With a chain a -> b -> + target, comparing 'a' against 'b' would incorrectly succeed because both + resolved to 'target', which could cause incorrect matching or duplicate + collection. + """ + # Prepare target directory with a test file. + short_path = Path(tempfile.mkdtemp()) + request.addfinalizer(lambda: shutil.rmtree(short_path, ignore_errors=True)) + target = short_path / "target" + target.mkdir() + (target / "test_chain.py").write_text("def test_chain(): pass", encoding="UTF-8") + + # Create multi-level symlink chain: a -> b -> target. + b = short_path / "b" + a = short_path / "a" + symlink_or_skip(target, b, target_is_directory=True) + symlink_or_skip(b, a, target_is_directory=True) + + # Collect via the first symlink; should find exactly one test. + result = pytester.runpytest(a) + result.assert_outcomes(passed=1) + + # Collect via the intermediate symlink; also exactly one test. + result = pytester.runpytest(b) + result.assert_outcomes(passed=1) + + def test_pyargs_collection_tree(pytester: Pytester, monkeypatch: MonkeyPatch) -> None: """When using `--pyargs`, the collection tree of a pyargs collection argument should only include parents in the import path, not up to confcutdir.