-
Notifications
You must be signed in to change notification settings - Fork 93
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
fix breaking pathlib change in 3.10 beta1 #594
fix breaking pathlib change in 3.10 beta1 #594
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this looks good to me apart from that minor change! Can you also please add an entry in the release notes?
If you want, you could also add 3.10-dev
to the test matrix in the workflow.
I had actually planned to test Python 3.10-dev with pyfakefs after I have seen some changes that break for example pytest, so this is a nice contribution. |
Changelog updated, and fixed the edit looks like we may need to update the python actions to v2 as well. I will update that here in just a sec! |
Just pushed a commit that restores the |
# same comment as above for these two | ||
realpath = staticmethod(os.path.realpath) | ||
getcwd = staticmethod(os.getcwd) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and the above section I am 99% sure I am doing something wrong/bad here -- but the gist of this is it looks like we needed to add some methods to the FakeAccessor object -- I am assuming(?) its not a great idea to just attach os
methods here because we would want to have the accessor using the patched methods? But this does seem to get more of the tests passing so figured id commit it to try to be as helpful as I can... (even if it isn't much :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it accordingly. Regarding getcwd
- if using os
inside the function, as I do it now, it will be patched, so it will behave correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed realpath
as it doesn't seem to be really needed, but maybe I'm wrong here.
Last push for the day because I am def out of my element here :) I left some comments in the files changed section about things im not super sure about and/or I think are probably wrong. This does get tox tests passing locally but some of it feels fairly wrong. Hopefully this can at least save ya some time on figuring stuff out! |
@carlmontanari - unfortunately, I don't have much time today, but I had a quick look. What I can see is that there are other tests failing because of other changes in Python 3.10, so I don't expect you to fix it all. There are also a few tests failing in 3.9 now, but I haven't really looked into this yet. Hopefully will have some time tomorrow. I can make commits into your PR to fix some of the problems if you don't mind (not today though), or help you to fix it, |
@mrbean-bremen Sorry I didn't get a chance to look today either, and sorry for my barrage of not working commits :). If you dont get to it first I will try to spend some more time looking at this this weekend but I think I am in over my head! Whatever is easiest for you w/ respect to the PR I'm totally good w/! I'm glad to have at least helped a little bit! |
@carlmontanari -- thanks! There is no need for you to spend more time on this, I can take it from here. As I wrote, your contribution has already been helpful enough. I'll work on your branch then if you don't mind and will see if I can finish it soon -- maybe at the weekend. |
1c20274
to
089da72
Compare
@carlmontanari - I got it to work now, I'll let you have a look and check if something is missing from your POV before I'm going to merge it. Didn't have to wait until the weekend, as today was a holiday here... |
You rock! A quick check and looks like all my tests are passing on 3.10 locally. Will try to get a branch updated to remove my skips to let it run on 3.10 on ubuntu latest too (locally am macOS). edit all tests in ci on ubuntu-latest for one of my projects work as expected in 3.10, so think we are lookin good! |
- fix link_to for Python 3.10 - revert some of the probably unneeded changes
- handle dummy encoding "locale" introduced in Python 3.10
- it calls chmod() in Python 3.10, so this has to handled
- some dependencies are not available for the beta version - comment out docker tests - setup currently fails, has to be handled separately
049e2fd
to
86bb5f7
Compare
…I - adapt fake pathlib, fix pathlib.Path methods link_to, getcwd, lchmod - handle dummy encoding "locale" introduced in Python 3.10 - do not test extra dependencies with Python 3.10 (some are not available)
Hello!
Sorry for the PR outa the blue w/out an issue to discuss -- had some time and figured I'd take a crack at it. I added 3.10 beta1 to my ci for some projects using pyfakefs and my builds are failing. Did a bit of sleuthing and it seems there were a few changes to Pathlib for this release. The main issue/thread seems to be here -- and associated commit. This commit also appears to be related.
I think (hope?!) that this fix is backwards compatible. Tests in the docker container all seem happy, and I've tested this locally on my Mac w/ 3.10 beta1 and the tests that are/were failing in CI pass like normal. For reference, here is my failed tests (the
build_posix (ubuntu-latest, 3.10.0-beta.1)
job).The fix itself is basically to ignore that
init
argument (the pr/issue on Cpython seems to say this is mostly legacy and (obviously) going away anyway). Then allowing the wrapped stat method (well I guess everything... which is perhaps not ideal?) to accept kwargs as well. Since_init
method is no longer called, I removed that and just assigned that_accessor
to the_fake_accessor
directly after the_from_parts
calls.Hope that all makes sense -- have been happily using pyfakefs (thanks for the project btw!) a while but the guts of it are probably a bit over my head :)
Carl