Skip to content

bpo-32907: pathlib: Fix test_resolve_common failure on Windows #5812

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.

Sign up for GitHub

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

Closed
izbyshev wants to merge 2 commits into from
Closed

bpo-32907: pathlib: Fix test_resolve_common failure on Windows #5812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c02f43a
bpo-32907: pathlib: Fix test_resolve_common failure on Windows
Feb 22, 2018
c72f2c0
Add NEWS entry
Feb 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Lib/test/test_pathlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,11 @@ def test_resolve_common(self):
self._check_resolve_relative(p, P(BASE, 'foo', 'in', 'spam'), False)
# Now create absolute symlinks
d = tempfile.mkdtemp(suffix='-dirD')
# bpo-32907: Canonicalize the temporary directory path since
# it is used in comparisons with other canonicalized paths below.
# Use Path.resolve() instead of os.path.realpath() to ensure that
# symlinks and "short" (8.3) filenames are resolved on Windows.
d = str(P(d).resolve())

@eryksun eryksun Feb 22, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolving makes sense in general. For example, "C:\Users" could be a junction or the user's %LocalAppData% may be a junction or relocated to a path that traverses a junction, or the user's %TEMP% could be non-standard.

That said, it might be useful in other cases to add nt._getlongpathname and nt._getshortpathname functions that call WinAPI GetLongPathName and GetShortPathName, and add methods to get the short and long paths to pathlib's Windows flavour. In particular I can see someone wanting the long path, but not the fully resolved path.

@izbyshev izbyshev Feb 22, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I've changed the wording a bit to make it look less like a Windows-specific fix. Thank you!

self.addCleanup(support.rmtree, d)
os.symlink(os.path.join(d), join('dirA', 'linkX'))
os.symlink(join('dirB'), os.path.join(d, 'linkY'))
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Tests/2018-02-23-02-01-40.bpo-32907.kno5Ua.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix test_pathlib to not fail if tempfile.mkdtemp() returns a non-canonical
path.