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.

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

Jump to bottom

Fix some problems with pathlib PosixPath/WindowsPath behavior #1055

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Fix some problems with pathlib PosixPath/WindowsPath behavior #1055

merged 2 commits into from
Sep 17, 2024

Conversation

camillobruni
Copy link
Contributor

@camillobruni camillobruni commented Aug 29, 2024 •
edited by mrbean-bremen
Loading

Describe the changes

Adding tests to document subtle pyfakefs differences described in issue 1053

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

@camillobruni camillobruni mentioned this pull request Aug 29, 2024
Closed
@mrbean-bremen
Copy link
Member

@camillobruni - I have added a commit that shall fix most of the issues, but I still got one failing test (test_real_file_with_home) under Posix and Python < 3.12, which I have disabled at the moment.
Can you please check how this version works for you?

kurtmckee
kurtmckee suggested changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@kurtmckee kurtmckee left a comment

Choose a reason for hiding this comment

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

I haven't read all of this but wanted to offer feedback for what I did see.

pyfakefs/fake_filesystem.py Outdated Show resolved Hide resolved
@mrbean-bremen
Copy link
Member

I haven't read all of this but wanted to offer feedback for what I did see.

Another pair of eyes is always helpful, especially as I rarely get reviews here. Given enough eyeballs...

@camillobruni
Copy link
Contributor Author

Late reply back from vacation: thanks a lot for these fixes!

kurtmckee
kurtmckee reviewed Sep 13, 2024
View reviewed changes
pyfakefs/fake_filesystem.py Show resolved Hide resolved
pyfakefs/fake_filesystem.py Outdated Show resolved Hide resolved
pyfakefs/fake_pathlib.py Outdated Show resolved Hide resolved
@mrbean-bremen
Copy link
Member

@camillobruni - I added a workaround to get the failing test running, and I would merge this if you give your ok. Did you have a chance to test it?
Oh, and I forgot the release notes, will add these before merging, of course.

@mrbean-bremen mrbean-bremen changed the title [WIP]: Add tests documenting pyfakefs vs pathlib differences Fix some problems with pathlib PosixPath/WindowsPath behavior Sep 14, 2024
@camillobruni
Copy link
Contributor Author

Cool, double checked locally and I got the tests working with this 👍

@mrbean-bremen mrbean-bremen merged commit da7bd5d into pytest-dev:main Sep 17, 2024
67 checks passed
mrbean-bremen pushed a commit that referenced this pull request Sep 17, 2024
@camillobruni @mrbean-bremen
Add tests to reproduce #1055
a02d2b4
ossieoj3
ossieoj3 reviewed Oct 22, 2024
View reviewed changes
Copy link

@ossieoj3 ossieoj3 left a comment

Choose a reason for hiding this comment

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

great work guys

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

@ossieoj3 ossieoj3 ossieoj3 left review comments

@mrbean-bremen mrbean-bremen mrbean-bremen left review comments

@kurtmckee kurtmckee kurtmckee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@camillobruni @mrbean-bremen @kurtmckee @ossieoj3