Skip to content

Conversation

@noobCodec
Copy link

@noobCodec noobCodec commented Nov 18, 2025

Fixes #19956. Updated this_file_dir in conf.py to use PurePath instead of realpath respecting symlink directories in tests.
This is an update of an earlier PR. Decided to use PurePath from UrlLib instead after a commend from A5rocks.
@wyattscarpenter Please retest.

@wyattscarpenter
Copy link
Contributor

OK, I've tested it and this does, indeed, fix the problem on Windows!

This is the old pr, for reference: #20161

Comment on lines 10 to 11
this_file_dir = os.path.dirname(PurePath(__file__))
PREFIX = os.path.dirname(os.path.dirname(this_file_dir))
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I was thinking that we can replace os.path.dirname with the relevant pathlib operation (this_file_dir = (Path(__file__) / '..').resolve()?)

Copy link
Author

Choose a reason for hiding this comment

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

is there an advantage of doing this over just this_file_dir = (Path(__file__)).resolve().parent ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No I just don't know pathlib APIs...

Copy link
Collaborator

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

This looks better to me! A couple nitpicks but I like this more than what was here before.

@wyattscarpenter could you make sure it still fixes #19956?

FYI I'm unable to resolve comments, so feel free to.

else:
this_file_dir = os.path.dirname(os.path.realpath(__file__))
this_file_dir = (Path(__file__)).resolve().parent
PREFIX = os.path.dirname(os.path.dirname(this_file_dir))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can also use .parent. Maybe you'll need to do str(...) to make sure it's a string path at the end, though...

On the other hand, maybe it would be nice to replace all os.path with pathlib.Path native things in this file? Up to you!

@noobCodec
Copy link
Author

I wouldn't merge this one yet looks like it breaks on certain windows machines with this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Certain tests can't deal with symlinks on Windows

3 participants