From d417b44e4d4c800e7736968a6345119c7013000a Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Wed, 28 Feb 2024 21:48:37 +0100 Subject: [PATCH 1/7] Do not collect symlinked tests under Windows The check for short paths under Windows via os.path.samefile, introduced in #11936, also found similar tests in symlinked tests in the GH Actions CI. This checks additionally that one of the files is not a symlink. Fixes #12039. --- AUTHORS | 1 + changelog/12039.bugfix.rst | 1 + src/_pytest/main.py | 9 ++++++++- testing/test_collection.py | 11 ++++++++++- 4 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 changelog/12039.bugfix.rst diff --git a/AUTHORS b/AUTHORS index f78c4b3f94b..4c4d68df147 100644 --- a/AUTHORS +++ b/AUTHORS @@ -283,6 +283,7 @@ Mike Hoyle (hoylemd) Mike Lundy Milan Lesnek Miro Hrončok +mrbean-bremen Nathaniel Compton Nathaniel Waisbrot Ned Batchelder diff --git a/changelog/12039.bugfix.rst b/changelog/12039.bugfix.rst new file mode 100644 index 00000000000..038d9b755eb --- /dev/null +++ b/changelog/12039.bugfix.rst @@ -0,0 +1 @@ +Fixed a regression in 8.0.2 where tests have been collected multiple times in the CI under Windows \ No newline at end of file diff --git a/src/_pytest/main.py b/src/_pytest/main.py index b7ed72ddc3b..67d6e142a18 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -924,7 +924,14 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: 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). - is_match = os.path.samefile(node.path, matchparts[0]) + same_file = os.path.samefile(node.path, matchparts[0]) + # we don't want to find links, so we at least + # exclude symlinks to regular directories + is_match = ( + same_file and + os.path.islink(node.path) == os.path.islink(matchparts[0]) + ) + # Name part e.g. `TestIt` in `/a/b/test_file.py::TestIt::test_it`. else: # TODO: Remove parametrized workaround once collection structure contains diff --git a/testing/test_collection.py b/testing/test_collection.py index fbc8543e9c4..910dba3e662 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1765,7 +1765,7 @@ def test_foo(): assert True @pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows only") def test_collect_short_file_windows(pytester: Pytester) -> None: - """Reproducer for #11895: short paths not colleced on Windows.""" + """Reproducer for #11895: short paths not collected on Windows.""" short_path = tempfile.mkdtemp() if "~" not in short_path: # pragma: no cover if running_on_ci(): @@ -1832,3 +1832,12 @@ def test_pyargs_collection_tree(pytester: Pytester, monkeypatch: MonkeyPatch) -> ], consecutive=True, ) + + +def test_collect_symlinks(pytester: Pytester, tmpdir) -> None: + """Regression test for #12039: Tests collected multiple times under Windows.""" + test_file = Path(tmpdir) / "symlink_collection_test.py" + test_file.write_text("def test(): pass", encoding="UTF-8") + result = pytester.runpytest(tmpdir) + # this failed in CI only (GitHub actions) + assert result.parseoutcomes() == {"passed": 1} From 9f2668dc080834da95d0e3afc22cfbfafb45243b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 3 Mar 2024 07:42:33 +0000 Subject: [PATCH 2/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- changelog/12039.bugfix.rst | 2 +- src/_pytest/main.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/changelog/12039.bugfix.rst b/changelog/12039.bugfix.rst index 038d9b755eb..1ff40b17719 100644 --- a/changelog/12039.bugfix.rst +++ b/changelog/12039.bugfix.rst @@ -1 +1 @@ -Fixed a regression in 8.0.2 where tests have been collected multiple times in the CI under Windows \ No newline at end of file +Fixed a regression in 8.0.2 where tests have been collected multiple times in the CI under Windows diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 67d6e142a18..a6e860bb651 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -927,10 +927,9 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: same_file = os.path.samefile(node.path, matchparts[0]) # we don't want to find links, so we at least # exclude symlinks to regular directories - is_match = ( - same_file and - os.path.islink(node.path) == os.path.islink(matchparts[0]) - ) + is_match = same_file and os.path.islink( + node.path + ) == os.path.islink(matchparts[0]) # Name part e.g. `TestIt` in `/a/b/test_file.py::TestIt::test_it`. else: From 96f72c150625bf85dfb1fd64f4f44b5e4480008c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 3 Mar 2024 08:57:48 -0300 Subject: [PATCH 3/7] Add test and tweak changelog --- changelog/12039.bugfix.rst | 2 +- testing/test_collection.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/changelog/12039.bugfix.rst b/changelog/12039.bugfix.rst index 1ff40b17719..267eae6b8b2 100644 --- a/changelog/12039.bugfix.rst +++ b/changelog/12039.bugfix.rst @@ -1 +1 @@ -Fixed a regression in 8.0.2 where tests have been collected multiple times in the CI under Windows +Fixed a regression in ``8.0.2`` where tests created using :fixture:`tmp_path` have been collected multiple times in CI under Windows. diff --git a/testing/test_collection.py b/testing/test_collection.py index 910dba3e662..d384ae97b03 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1789,6 +1789,26 @@ def test_collect_short_file_windows(pytester: Pytester) -> None: assert result.parseoutcomes() == {"passed": 1} +def test_not_collect_symlink_syblings( + pytester: Pytester, tmp_path: Path, request: pytest.FixtureRequest +) -> None: + """ + Do not collect from directories that are symlinks to other directories in the same path. + + The check for short paths under Windows via os.path.samefile, introduced in #11936, also finds the symlinked + directory created by tmp_path/tmpdir. + + #12039 + """ + # Use tmp_path because it creates a symlink with the name "current" next to the directory it creates. + assert tmp_path.parent.joinpath(f"{request.node.name}current").is_symlink() is True + + tmp_path.joinpath("test_foo.py").write_text("def test(): pass", encoding="UTF-8") + + result = pytester.runpytest_subprocess(tmp_path, "-sv") + 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. From bb48b7529aa6eeb7203e908ea1cc80653dcb2968 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 3 Mar 2024 09:03:34 -0300 Subject: [PATCH 4/7] Adjust existing test --- testing/test_collection.py | 43 ++++++++++++++------------------------ 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/testing/test_collection.py b/testing/test_collection.py index d384ae97b03..fe260c6afc2 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1789,26 +1789,6 @@ def test_collect_short_file_windows(pytester: Pytester) -> None: assert result.parseoutcomes() == {"passed": 1} -def test_not_collect_symlink_syblings( - pytester: Pytester, tmp_path: Path, request: pytest.FixtureRequest -) -> None: - """ - Do not collect from directories that are symlinks to other directories in the same path. - - The check for short paths under Windows via os.path.samefile, introduced in #11936, also finds the symlinked - directory created by tmp_path/tmpdir. - - #12039 - """ - # Use tmp_path because it creates a symlink with the name "current" next to the directory it creates. - assert tmp_path.parent.joinpath(f"{request.node.name}current").is_symlink() is True - - tmp_path.joinpath("test_foo.py").write_text("def test(): pass", encoding="UTF-8") - - result = pytester.runpytest_subprocess(tmp_path, "-sv") - 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. @@ -1854,10 +1834,19 @@ def test_pyargs_collection_tree(pytester: Pytester, monkeypatch: MonkeyPatch) -> ) -def test_collect_symlinks(pytester: Pytester, tmpdir) -> None: - """Regression test for #12039: Tests collected multiple times under Windows.""" - test_file = Path(tmpdir) / "symlink_collection_test.py" - test_file.write_text("def test(): pass", encoding="UTF-8") - result = pytester.runpytest(tmpdir) - # this failed in CI only (GitHub actions) - assert result.parseoutcomes() == {"passed": 1} +def test_do_not_collect_symlink_syblings( + pytester: Pytester, tmp_path: Path, request: pytest.FixtureRequest +) -> None: + """ + Regression test for #12039: Do not collect from directories that are symlinks to other directories in the same path. + + The check for short paths under Windows via os.path.samefile, introduced in #11936, also finds the symlinked + directory created by tmp_path/tmpdir. + """ + # Use tmp_path because it creates a symlink with the name "current" next to the directory it creates. + assert tmp_path.parent.joinpath(f"{request.node.name}current").is_symlink() is True + + tmp_path.joinpath("test_foo.py").write_text("def test(): pass", encoding="UTF-8") + + result = pytester.runpytest_subprocess(tmp_path, "-sv") + result.assert_outcomes(passed=1) From 0ac1c3750d85a68824c14f6bb62889eef0f0f4b2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 3 Mar 2024 09:06:26 -0300 Subject: [PATCH 5/7] Tweak comment --- src/_pytest/main.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index a6e860bb651..d8cd023cc6e 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -925,11 +925,12 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: # 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 find links, so we at least - # exclude symlinks to regular directories - is_match = same_file and os.path.islink( - node.path - ) == os.path.islink(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]) + ) # Name part e.g. `TestIt` in `/a/b/test_file.py::TestIt::test_it`. else: From 44f52b4ca6cc608a68b316b7064c405bb091fade Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 3 Mar 2024 09:09:47 -0300 Subject: [PATCH 6/7] Fix typo --- testing/test_collection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_collection.py b/testing/test_collection.py index fe260c6afc2..09df197e373 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1834,7 +1834,7 @@ def test_pyargs_collection_tree(pytester: Pytester, monkeypatch: MonkeyPatch) -> ) -def test_do_not_collect_symlink_syblings( +def test_do_not_collect_symlink_siblings( pytester: Pytester, tmp_path: Path, request: pytest.FixtureRequest ) -> None: """ From d473bf13e71ccf1cef754c49e30be3c3961fb86c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 3 Mar 2024 09:21:08 -0300 Subject: [PATCH 7/7] Improve test --- testing/test_collection.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/testing/test_collection.py b/testing/test_collection.py index 09df197e373..1491ec85990 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1844,9 +1844,16 @@ def test_do_not_collect_symlink_siblings( directory created by tmp_path/tmpdir. """ # Use tmp_path because it creates a symlink with the name "current" next to the directory it creates. - assert tmp_path.parent.joinpath(f"{request.node.name}current").is_symlink() is True + symlink_path = tmp_path.parent / (tmp_path.name[:-1] + "current") + assert symlink_path.is_symlink() is True + # Create test file. tmp_path.joinpath("test_foo.py").write_text("def test(): pass", encoding="UTF-8") - result = pytester.runpytest_subprocess(tmp_path, "-sv") + # Ensure we collect it only once if we pass the tmp_path. + result = pytester.runpytest(tmp_path, "-sv") + result.assert_outcomes(passed=1) + + # Ensure we collect it only once if we pass the symlinked directory. + result = pytester.runpytest(symlink_path, "-sv") result.assert_outcomes(passed=1)