From dc3130be67eca2fa00b2ccbd3f5c7f9e5bbb6660 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 5 Feb 2024 18:14:13 -0300 Subject: [PATCH 1/5] Add test for #11895 --- testing/test_collection.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/testing/test_collection.py b/testing/test_collection.py index 98cff8fe9a1..1d18ee75dca 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -4,9 +4,11 @@ import pprint import shutil import sys +import tempfile import textwrap from typing import List +from _pytest.assertion.util import running_on_ci from _pytest.config import ExitCode from _pytest.fixtures import FixtureRequest from _pytest.main import _in_venv @@ -1759,3 +1761,29 @@ def test_foo(): assert True assert result.ret == ExitCode.OK assert result.parseoutcomes() == {"passed": 1} + + +@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.""" + short_path = tempfile.mkdtemp() + if "~" not in short_path: + if running_on_ci(): + # On CI, we are expecting that under the current GitHub actions configuration, + # tempfile.mkdtemp() is producing short paths, so we want to fail to prevent + # this from silently changing without us noticing. + pytest.fail( + f"tempfile.mkdtemp() failed to produce a short path on CI: {short_path}" + ) + else: + # We want to skip failing this test locally in this situation because + # depending on the local configuration tempfile.mkdtemp() might not produce a short path: + # For example, user might have configured %TEMP% exactly to avoid generating short paths. + pytest.skip( + f"tempfile.mkdtemp() failed to produce a short path: {short_path}, skipping" + ) + + test_file = Path(short_path).joinpath("test_collect_short_file_windows.py") + test_file.write_text("def test(): pass", encoding="UTF-8") + result = pytester.runpytest(short_path) + assert result.parseoutcomes() == {"passed": 1} From c95c76bfb6ef49c6ca19328a336b7ed6d0c8d7e4 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 5 Feb 2024 17:49:40 -0300 Subject: [PATCH 2/5] Fix collection of short paths on Windows Passing a short path in the command line was causing the matchparts check to fail, because ``Path(short_path) != Path(long_path)``. Using ``os.path.samefile`` as fallback ensures the comparsion works on Windows when comparing short/long paths. Fix #11895 --- changelog/11895.bugfix.rst | 1 + src/_pytest/main.py | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 changelog/11895.bugfix.rst diff --git a/changelog/11895.bugfix.rst b/changelog/11895.bugfix.rst new file mode 100644 index 00000000000..4211213c1e5 --- /dev/null +++ b/changelog/11895.bugfix.rst @@ -0,0 +1 @@ +Fix collection on Windows where initial paths contain the short version of a path (for example ``c:\PROGRA~1\tests``). diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 5c70ad74e40..edb1a69e2b7 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -901,6 +901,10 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: # Path part e.g. `/a/b/` in `/a/b/test_file.py::TestIt::test_it`. if isinstance(matchparts[0], Path): 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). + is_match = os.path.samefile(node.path, 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 From f5dc4c9d39177a3412373fbb3d196c65743073aa Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 5 Feb 2024 19:35:20 -0300 Subject: [PATCH 3/5] Ignore guard code in test --- 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 1d18ee75dca..5cd9208f575 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1767,7 +1767,7 @@ def test_foo(): assert True def test_collect_short_file_windows(pytester: Pytester) -> None: """Reproducer for #11895: short paths not colleced on Windows.""" short_path = tempfile.mkdtemp() - if "~" not in short_path: + if "~" not in short_path: # pragma: no cover if running_on_ci(): # On CI, we are expecting that under the current GitHub actions configuration, # tempfile.mkdtemp() is producing short paths, so we want to fail to prevent From 2e8f9572a4aab7b0156fa4c448ee5c88a726915b Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 9 Feb 2024 18:00:08 -0300 Subject: [PATCH 4/5] Expand to long paths when resolving collection arguments --- src/_pytest/compat.py | 35 +++++++++++++++++++- src/_pytest/main.py | 7 ++-- testing/test_collection.py | 65 ++++++++++++++++++++++++-------------- 3 files changed, 78 insertions(+), 29 deletions(-) diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index fa387f6db12..e1a33ec4369 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -1,5 +1,5 @@ # mypy: allow-untyped-defs -"""Python version compatibility code.""" +"""Python version and platform compatibility code.""" from __future__ import annotations import dataclasses @@ -301,6 +301,39 @@ def get_user_id() -> int | None: return uid if uid != ERROR else None +if sys.platform == "win32": + from ctypes import create_unicode_buffer + from ctypes import windll + + def ensure_long_path(p: Path) -> Path: + """ + Returns the given path in its long form in Windows. + + Short-paths follow the DOS restriction of 8 characters + 3 chars for file extension, + and are still supported by Windows. + """ + # If the path does not exist, we cannot discover its long path. + if not p.exists(): + return p + short_path = os.fspath(p) + # Use a buffer twice the size of the original path size to (reasonably) ensure we will be able + # to hold the long path. + buffer_size = len(short_path) * 2 + buffer = create_unicode_buffer(buffer_size) + windll.kernel32.GetLongPathNameW(short_path, buffer, buffer_size) + long_path_str = buffer.value + # If we could not convert it, probably better to hard-crash this now rather + # than later. + assert long_path_str, f"Failed to convert short path to long path form:\n(size: {len(short_path)}):{short_path}" + return Path(buffer.value) + +else: + + def ensure_long_path(p: Path) -> Path: + """No-op in other platforms.""" + return p + + # Perform exhaustiveness checking. # # Consider this example: diff --git a/src/_pytest/main.py b/src/_pytest/main.py index edb1a69e2b7..154ff2710af 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -28,6 +28,7 @@ from _pytest import nodes import _pytest._code +from _pytest.compat import ensure_long_path from _pytest.config import Config from _pytest.config import directory_arg from _pytest.config import ExitCode @@ -901,10 +902,6 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: # Path part e.g. `/a/b/` in `/a/b/test_file.py::TestIt::test_it`. if isinstance(matchparts[0], Path): 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). - is_match = os.path.samefile(node.path, 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 @@ -1012,4 +1009,6 @@ def resolve_collection_argument( else "directory argument cannot contain :: selection parts: {arg}" ) raise UsageError(msg.format(arg=arg)) + # Ensure we expand short paths to long paths on Windows (#11895). + fspath = ensure_long_path(fspath) return fspath, parts diff --git a/testing/test_collection.py b/testing/test_collection.py index 5cd9208f575..78e3672d6b0 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -9,6 +9,7 @@ from typing import List from _pytest.assertion.util import running_on_ci +from _pytest.compat import ensure_long_path from _pytest.config import ExitCode from _pytest.fixtures import FixtureRequest from _pytest.main import _in_venv @@ -1763,27 +1764,43 @@ def test_foo(): assert True assert result.parseoutcomes() == {"passed": 1} -@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.""" - short_path = tempfile.mkdtemp() - if "~" not in short_path: # pragma: no cover - if running_on_ci(): - # On CI, we are expecting that under the current GitHub actions configuration, - # tempfile.mkdtemp() is producing short paths, so we want to fail to prevent - # this from silently changing without us noticing. - pytest.fail( - f"tempfile.mkdtemp() failed to produce a short path on CI: {short_path}" - ) - else: - # We want to skip failing this test locally in this situation because - # depending on the local configuration tempfile.mkdtemp() might not produce a short path: - # For example, user might have configured %TEMP% exactly to avoid generating short paths. - pytest.skip( - f"tempfile.mkdtemp() failed to produce a short path: {short_path}, skipping" - ) - - test_file = Path(short_path).joinpath("test_collect_short_file_windows.py") - test_file.write_text("def test(): pass", encoding="UTF-8") - result = pytester.runpytest(short_path) - assert result.parseoutcomes() == {"passed": 1} +class TestCollectionShortPaths: + @pytest.fixture + def short_path(self) -> Path: + short_path = tempfile.mkdtemp() + if "~" not in short_path: # pragma: no cover + if running_on_ci(): + # On CI, we are expecting that under the current GitHub actions configuration, + # tempfile.mkdtemp() is producing short paths, so we want to fail to prevent + # this from silently changing without us noticing. + pytest.fail( + f"tempfile.mkdtemp() failed to produce a short path on CI: {short_path}" + ) + else: + # We want to skip failing this test locally in this situation because + # depending on the local configuration tempfile.mkdtemp() might not produce a short path: + # For example, user might have configured %TEMP% exactly to avoid generating short paths. + pytest.skip( + f"tempfile.mkdtemp() failed to produce a short path: {short_path}, skipping" + ) + return Path(short_path) + + @pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows only") + def test_ensure_long_path_win(self, short_path: Path) -> None: + long_path = ensure_long_path(short_path) + assert len(os.fspath(long_path)) > len(os.fspath(short_path)) + + @pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows only") + def test_collect_short_file_windows( + self, pytester: Pytester, short_path: Path + ) -> None: + """Reproducer for #11895: short paths not collected on Windows.""" + test_file = short_path.joinpath("test_collect_short_file_windows.py") + test_file.write_text("def test(): pass", encoding="UTF-8") + result = pytester.runpytest(short_path) + assert result.parseoutcomes() == {"passed": 1} + + def test_ensure_long_path_general(self, tmp_path: Path) -> None: + """Sanity check: a normal path to ensure_long_path works on all platforms.""" + assert ensure_long_path(tmp_path) == tmp_path + assert ensure_long_path(tmp_path / "non-existent") == tmp_path / "non-existent" From abb43a612e06c0b66a542ec576240a52ec6f2302 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 17 Feb 2024 17:50:39 -0300 Subject: [PATCH 5/5] Revert "Expand to long paths when resolving collection arguments" This reverts commit 2e8f9572a4aab7b0156fa4c448ee5c88a726915b. --- src/_pytest/compat.py | 35 +------------------- src/_pytest/main.py | 7 ++-- testing/test_collection.py | 65 ++++++++++++++------------------------ 3 files changed, 29 insertions(+), 78 deletions(-) diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index e1a33ec4369..fa387f6db12 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -1,5 +1,5 @@ # mypy: allow-untyped-defs -"""Python version and platform compatibility code.""" +"""Python version compatibility code.""" from __future__ import annotations import dataclasses @@ -301,39 +301,6 @@ def get_user_id() -> int | None: return uid if uid != ERROR else None -if sys.platform == "win32": - from ctypes import create_unicode_buffer - from ctypes import windll - - def ensure_long_path(p: Path) -> Path: - """ - Returns the given path in its long form in Windows. - - Short-paths follow the DOS restriction of 8 characters + 3 chars for file extension, - and are still supported by Windows. - """ - # If the path does not exist, we cannot discover its long path. - if not p.exists(): - return p - short_path = os.fspath(p) - # Use a buffer twice the size of the original path size to (reasonably) ensure we will be able - # to hold the long path. - buffer_size = len(short_path) * 2 - buffer = create_unicode_buffer(buffer_size) - windll.kernel32.GetLongPathNameW(short_path, buffer, buffer_size) - long_path_str = buffer.value - # If we could not convert it, probably better to hard-crash this now rather - # than later. - assert long_path_str, f"Failed to convert short path to long path form:\n(size: {len(short_path)}):{short_path}" - return Path(buffer.value) - -else: - - def ensure_long_path(p: Path) -> Path: - """No-op in other platforms.""" - return p - - # Perform exhaustiveness checking. # # Consider this example: diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 154ff2710af..edb1a69e2b7 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -28,7 +28,6 @@ from _pytest import nodes import _pytest._code -from _pytest.compat import ensure_long_path from _pytest.config import Config from _pytest.config import directory_arg from _pytest.config import ExitCode @@ -902,6 +901,10 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: # Path part e.g. `/a/b/` in `/a/b/test_file.py::TestIt::test_it`. if isinstance(matchparts[0], Path): 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). + is_match = os.path.samefile(node.path, 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 @@ -1009,6 +1012,4 @@ def resolve_collection_argument( else "directory argument cannot contain :: selection parts: {arg}" ) raise UsageError(msg.format(arg=arg)) - # Ensure we expand short paths to long paths on Windows (#11895). - fspath = ensure_long_path(fspath) return fspath, parts diff --git a/testing/test_collection.py b/testing/test_collection.py index 78e3672d6b0..5cd9208f575 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -9,7 +9,6 @@ from typing import List from _pytest.assertion.util import running_on_ci -from _pytest.compat import ensure_long_path from _pytest.config import ExitCode from _pytest.fixtures import FixtureRequest from _pytest.main import _in_venv @@ -1764,43 +1763,27 @@ def test_foo(): assert True assert result.parseoutcomes() == {"passed": 1} -class TestCollectionShortPaths: - @pytest.fixture - def short_path(self) -> Path: - short_path = tempfile.mkdtemp() - if "~" not in short_path: # pragma: no cover - if running_on_ci(): - # On CI, we are expecting that under the current GitHub actions configuration, - # tempfile.mkdtemp() is producing short paths, so we want to fail to prevent - # this from silently changing without us noticing. - pytest.fail( - f"tempfile.mkdtemp() failed to produce a short path on CI: {short_path}" - ) - else: - # We want to skip failing this test locally in this situation because - # depending on the local configuration tempfile.mkdtemp() might not produce a short path: - # For example, user might have configured %TEMP% exactly to avoid generating short paths. - pytest.skip( - f"tempfile.mkdtemp() failed to produce a short path: {short_path}, skipping" - ) - return Path(short_path) - - @pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows only") - def test_ensure_long_path_win(self, short_path: Path) -> None: - long_path = ensure_long_path(short_path) - assert len(os.fspath(long_path)) > len(os.fspath(short_path)) - - @pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows only") - def test_collect_short_file_windows( - self, pytester: Pytester, short_path: Path - ) -> None: - """Reproducer for #11895: short paths not collected on Windows.""" - test_file = short_path.joinpath("test_collect_short_file_windows.py") - test_file.write_text("def test(): pass", encoding="UTF-8") - result = pytester.runpytest(short_path) - assert result.parseoutcomes() == {"passed": 1} - - def test_ensure_long_path_general(self, tmp_path: Path) -> None: - """Sanity check: a normal path to ensure_long_path works on all platforms.""" - assert ensure_long_path(tmp_path) == tmp_path - assert ensure_long_path(tmp_path / "non-existent") == tmp_path / "non-existent" +@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.""" + short_path = tempfile.mkdtemp() + if "~" not in short_path: # pragma: no cover + if running_on_ci(): + # On CI, we are expecting that under the current GitHub actions configuration, + # tempfile.mkdtemp() is producing short paths, so we want to fail to prevent + # this from silently changing without us noticing. + pytest.fail( + f"tempfile.mkdtemp() failed to produce a short path on CI: {short_path}" + ) + else: + # We want to skip failing this test locally in this situation because + # depending on the local configuration tempfile.mkdtemp() might not produce a short path: + # For example, user might have configured %TEMP% exactly to avoid generating short paths. + pytest.skip( + f"tempfile.mkdtemp() failed to produce a short path: {short_path}, skipping" + ) + + test_file = Path(short_path).joinpath("test_collect_short_file_windows.py") + test_file.write_text("def test(): pass", encoding="UTF-8") + result = pytester.runpytest(short_path) + assert result.parseoutcomes() == {"passed": 1}