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

Add specific tests for PurePosixPath and PureWindowsPath #1066

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

lengau
Copy link
Contributor

@lengau lengau commented Oct 4, 2024

Describe the changes

Adds tests specific to PurePosixPath and PureWindowsPath to catch issues like #1065

Tasks

  • Unit tests added that reproduce the issue or prove feature is working
  • Fix or feature added
  • Entry to release notes added
  • Pre-commit CI shows no errors
  • Unit tests passing
  • For documentation changes: The Read the Docs preview builds and looks as expected

@lengau lengau marked this pull request as ready for review October 4, 2024 18:20
@lengau lengau changed the title Add specific tests for PurePosixPath and PureWindowsPath [WIP]Add specific tests for PurePosixPath and PureWindowsPath Oct 4, 2024
@lengau lengau marked this pull request as draft October 4, 2024 18:25
@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 4, 2024

Thanks - I see the failing tests... is_reserved has been overwritten for fake pathlib.Path (for Python >= 3.12), but obviously not for Windows/PosixPath. I haven't checked that recursion under Windows with the joinpath test yet, that seems worse...

I will have a look over the weekend, unless you are going to fix it yourself :)

@mrbean-bremen
Copy link
Member

@lengau: I added a commit with a fix for the reserved names to this PR (didn't want to copy the tests, or push the fix before the tests).

I wrote a spin-off issue for the joinpath problem under Windows and commented out the failing test for Windows.
I added a workaround for the recursion, so that will not happen anymore, but the test still fails - this turned out to be a bit more complicated.

Please check if that works for you.

@mrbean-bremen mrbean-bremen changed the title [WIP]Add specific tests for PurePosixPath and PureWindowsPath Add specific tests for PurePosixPath and PureWindowsPath Oct 7, 2024
- had been only patched for pathlib.Path
- avoid possible recursion in PurePath.joinpath
- fixes pytest-dev#1067
@mrbean-bremen
Copy link
Member

Ok, turned out the joinpath problem was easy to fix, I just made a mistake last time I tried it.
I have added an extra commit with the fix.

I'm planning to make a new release with these fixes (and with the official 3.13 support that I also added, as Python 3.13 was released today) soon - please check if this works for you.

@lengau
Copy link
Contributor Author

lengau commented Oct 8, 2024

@mrbean-bremen thanks for doing this! I was meaning to get to it this weekend but ended up being busy with other things. These fixes work for me on my Linux machine.

@mrbean-bremen
Copy link
Member

I was meaning to get to it this weekend but ended up being busy with other things

Sounds familiar...

These fixes work for me on my Linux machine

Ok, I'm going to merge this and make a new release tonight - thanks!

@mrbean-bremen mrbean-bremen merged commit 68c0a42 into pytest-dev:main Oct 8, 2024
67 checks passed
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.

2 participants