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

Release 4.5.2 broke our tests vs 4.5.1 #647

Closed
Safihre opened this issue Nov 8, 2021 · 4 comments
Closed

Release 4.5.2 broke our tests vs 4.5.1 #647

Safihre opened this issue Nov 8, 2021 · 4 comments

Comments

@Safihre
Copy link

Safihre commented Nov 8, 2021

We suddenly have various failures after our CI pipline started using 4.5.2 instead of 4.5.1.
I don't even know where to start to find the cause. It doesn't seem to be OS-dependent, since some Linux build fail, but not all. The error on Windows is different than on Linux and macOS succeeds.
No code was changed..

Yesterday: https://github.com/sabnzbd/sabnzbd/actions/runs/1431017533
Today: https://github.com/sabnzbd/sabnzbd/actions/runs/1434684518

(cc @jcfp)

@mrbean-bremen
Copy link
Member

Thank you for the report. I just found the problem - it is not actually a bug, but is due to a behavior change of os.listdir in pyfakefs. From the release notes:

os.listdir, os.scandir and pathlib.Path.listdir now return the directory list in a random order (see #638)

This has been requested as the order is not fixed in the real file system, but may lead to failed tests, if they relied on thi behavior. I just disabled this change in a pyfakefs branch and directed a build of your repo (e.g. a fork) to this branch. As you can see, the builds pass in this case.

I now realize that such a behavior change should not have been in a bug fix release, and probably should be a opt-in configuration option, at least for some time.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Nov 8, 2021
- the default is to not randomize
- can be changed by setting fs.shuffle_listdir_results to True
- see pytest-dev#647
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Nov 8, 2021
- the default is to not randomize
- can be changed by setting fs.shuffle_listdir_results to True
- see pytest-dev#647
github-actions bot pushed a commit that referenced this issue Nov 8, 2021
…randomize - can be changed by setting fs.shuffle_listdir_results to True - see #647
@Safihre
Copy link
Author

Safihre commented Nov 8, 2021

Thanks a lot for looking into this in detail, I was a bit lost where to start!
Of course we can sort our listdir results to fix our tests with the new code. But indeed, our assumption of sorted listdir is only used in the tests, not in the functions being tested.

@mrbean-bremen
Copy link
Member

Shall be fixed now in version 4.5.3, please check. It makes sense to sort the results in your tests anyway, as in some future version the unsorted output will be the default (as it matches the real fs).

@Safihre
Copy link
Author

Safihre commented Nov 9, 2021

Thank you, it works 👍 👍

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

No branches or pull requests

2 participants