Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short paths in Windows seem to fail to be collected #11895

Closed
Erotemic opened this issue Jan 30, 2024 · 6 comments · Fixed by #11936
Closed

Short paths in Windows seem to fail to be collected #11895

Erotemic opened this issue Jan 30, 2024 · 6 comments · Fixed by #11936
Assignees
Labels
platform: windows windows platform-specific problem topic: collection related to the collection phase type: regression indicates a problem that was introduced in a release which was working previously

Comments

@Erotemic
Copy link

This is an initial report of an issue I observed when updating xdoctest to support pytest 8.0.

What I found is that when I ran a test effectivly using the invocation:

C:\hostedtoolcache\windows\Python\3.12.1\x64\python.exe -m pytest C:\Users\RUNNER~1\AppData\Local\Temp\tmpcttcs8zz

Pytest would collect 0 tests even though there was a python file with a test in that directory.

By adding this code to convert the short path to a long path, the xdoctest failure went away, and things seem to be working now.

dpath = tempfile.mkdtemp()
# https://stackoverflow.com/questions/11420689/how-to-get-long-file-system-path-from-python-on-windows
from ctypes import create_unicode_buffer, windll
BUFFER_SIZE = 500
buffer = create_unicode_buffer(BUFFER_SIZE)
get_long_path_name = windll.kernel32.GetLongPathNameW
get_long_path_name(dpath, buffer, BUFFER_SIZE)
dpath = buffer.value

I've documented more of the problem here Erotemic/xdoctest#151

I don't have a MWE as I don't have a windows machine, but I'm reasonably confident that something in pytest 8 broke short path recognition on windows. I'll leave it to other devs to test further.

@bluetech
Copy link
Member

Thanks for the report.

I don't have a MWE as I don't have a windows machine, but I'm reasonably confident that something in pytest 8 broke short path recognition on windows. I'll leave it to other devs to test further.

In order to narrow down on the issue, we'll probably need to bisect. I don't have windows either, but I can try with Wine. Can you provide instructions for reproducing the CI failure locally? (Doesn't have to be minimal).

@bluetech bluetech added topic: collection related to the collection phase type: regression indicates a problem that was introduced in a release which was working previously labels Jan 31, 2024
@nicoddemus nicoddemus added the status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity label Jan 31, 2024
@Erotemic
Copy link
Author

I think this should work to reproduce it. On a window machine:

diff --git a/src/xdoctest/utils/util_path.py b/src/xdoctest/utils/util_path.py
index d49b371..83a0230 100644
--- a/src/xdoctest/utils/util_path.py
+++ b/src/xdoctest/utils/util_path.py
@@ -34,19 +34,8 @@ class TempDir(object):
 
     def ensure(self):
         import tempfile
-        import sys
         if not self.dpath:
             dpath = tempfile.mkdtemp()
-            if sys.platform.startswith('win32'):
-                # Force a long path
-                # References:
-                # https://stackoverflow.com/questions/11420689/how-to-get-long-file-system-path-from-python-on-windows
-                from ctypes import create_unicode_buffer, windll
-                BUFFER_SIZE = 500
-                buffer = create_unicode_buffer(BUFFER_SIZE)
-                get_long_path_name = windll.kernel32.GetLongPathNameW
-                get_long_path_name(dpath, buffer, BUFFER_SIZE)
-                dpath = buffer.value
             self.dpath = dpath
         return self.dpath
  • install the version of pytest you want to test

  • Run the tests that cause the error (relative to the xdoctest repo root):

pytest tests/test_pytest_cli.py -k test_simple_pytest_import_error_cli -s
pytest tests/test_pytest_cli.py -k test_simple_pytest_cli -s

You can likely make this much more minimal by using tempfile.mkdtemp() to make a directory, record the path that it returns (on the actions CLI it was a short path, so I expect it will be the same on a different machine). Then write a small test file that pytest should pickup to that directory and then run pytest <path> to that directory. That is effectively what the above xdoctest instructions are doing.

@nicoddemus
Copy link
Member

You can likely make this much more minimal by using tempfile.mkdtemp() to make a directory, record the path that it returns (on the actions CLI it was a short path, so I expect it will be the same on a different machine). Then write a small test file that pytest should pickup to that directory and then run pytest to that directory. That is effectively what the above xdoctest instructions are doing.

I did not investigate into details, but I cannot reproduce the behavior using a very short path:

λ pwd
W:\

λ cat a\test_foo.py
def test(): pass

λ pytest w:\a
======================== test session starts ========================
platform win32 -- Python 3.10.9, pytest-8.0.0, pluggy-1.4.0
rootdir: w:\a
collected 1 item

a\test_foo.py .                                                [100%]

========================= 1 passed in 0.00s =========================

λ pytest a
======================== test session starts ========================
platform win32 -- Python 3.10.9, pytest-8.0.0, pluggy-1.4.0
rootdir: W:\a
collected 1 item

a\test_foo.py .                                                [100%]

========================= 1 passed in 0.00s =========================

I'm afraid we will need more details to reproduce the issue.

@Erotemic
Copy link
Author

Erotemic commented Feb 1, 2024

There's a misunderstanding on what I mean by "short path". It's not a path with a short length, it's a windows shorthand to refer to a real path that's longer. It's like an autocomlete. E.g.

c:\PROGRA~2\Android\ANDROI~1 resolves to c:\Program Files (x86)\Android\android-sdk

Reference: https://superuser.com/questions/348079/how-can-i-find-the-short-path-of-a-windows-directory-file

It's a silly feature, but it seems tempfile.mkdtemp is able to return one, so it probably makes sense to support it.

@nicoddemus
Copy link
Member

Ahh of course, brain fart on my part.

I can reproduce the issue:

(.env310) λ dir c:\PORTAB~1\test\ /b
test_foo.py
__pycache__
>>> from pathlib import Path
>>> p=Path(r'c:\PORTAB~1\test')
>>> p.is_dir()
True
>>> (p / "test_foo.py").is_file()
True
>>>
λ pytest c:\PORTAB~1\test\
======================== test session starts ========================
platform win32 -- Python 3.10.10, pytest-8.1.0.dev108+gca3790102, pluggy-1.4.0
rootdir: e:\projects\pytest
plugins: hypothesis-6.84.0, replay-1.4.0
collected 0 items

======================= no tests ran in 0.02s =======================
ERROR: not found: c:\PORTAB~1\test
(no match in any of [<Dir >])

I will investigate this when I find some time, thanks for the report!

@nicoddemus nicoddemus self-assigned this Feb 1, 2024
@nicoddemus nicoddemus removed the status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity label Feb 1, 2024
@bluetech
Copy link
Member

bluetech commented Feb 1, 2024

I'm surprised this ever worked. Windows paths - the gift that keeps on giving :)

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 5, 2024
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 5, 2024
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`` ensures the comparsion works on Windows.

Fix pytest-dev#11895
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 5, 2024
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`` ensures the comparsion works on Windows.

Fix pytest-dev#11895
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 5, 2024
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 pytest-dev#11895
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 5, 2024
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 5, 2024
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 pytest-dev#11895
@bluetech bluetech added the platform: windows windows platform-specific problem label Feb 17, 2024
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 17, 2024
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 17, 2024
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 pytest-dev#11895
nicoddemus added a commit that referenced this issue Feb 23, 2024
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
flying-sheep pushed a commit to flying-sheep/pytest that referenced this issue Apr 9, 2024
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 pytest-dev#11895
Zeitsperre added a commit to Ouranosinc/xclim that referenced this issue Jul 31, 2024
### What kind of change does this PR introduce?

* Unpins the `pytest` version now that `xdoctest` supports it.
* Pins to the latest `xdoctest` supporting `pytest~=8.0`.

### Does this PR introduce a breaking change?

No.

### Other information:

FYI @RondeauG 

The newest `pytest` (v8.0.0) introduced some regressions for Windows
users. An issue is already up and a patch should arrive shortly
(pytest-dev/pytest#11895). Until then, use
`pytest<8.0` on Windows.

See also: pytest-dev/pytest#11969
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: windows windows platform-specific problem topic: collection related to the collection phase type: regression indicates a problem that was introduced in a release which was working previously
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants