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

Tests failing with pyfakefs and django after first test #932

Closed
mrbean-bremen opened this issue Jan 11, 2024 · 8 comments · Fixed by #933
Closed

Tests failing with pyfakefs and django after first test #932

mrbean-bremen opened this issue Jan 11, 2024 · 8 comments · Fixed by #933
Labels

Comments

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Jan 11, 2024

This came up in a stackoverflow question by @konnerthg 2 days ago. If running some django tests using pytest-django and pyfakefs, all tests after the first fail because the mock is not working anymore. A reproducer can be found here.

The test in question:

@pytest.mark.parametrize("counter", range(2))
def test_fs_and_mocker(
    client,
    fs,
    counter,
):
    with unittest.mock.patch("mysite.views.foo") as mocker:
        client.get("/foo/")
        mocker.assert_called()

failed on the second run because foo was no longer properly patched.

A similar issue with django tests came up yesterday in a private communication with @sgarcialaguna: subsequent tests where failing after one test run with pyfakefs under some circumstances. Can be reproduced both under Linux and Windows.

The root cause seems to be that modules that are loaded dynamically during the test are removed from sys.modules after the test to ensure that they are reloaded unpatched as soon as needed. This does not properly work in django, as some of these modules are not re-imported as assumed - there has to be some statically cached information that prevents that.

@mrbean-bremen
Copy link
Member Author

I tried to fix this by reloading the dynamically patched modules instead of removing them (which is the obvious fix), but this failed for Python < 3.10 (e.g. a couple of tests failed). Also, I'm missing a proper test for this.

@konnerthg
Copy link

@mrbean-bremen thank you for looking into this. would the above work as a temporary quick-fix for python >= 3.10? if so, could you describe how to do it? I don't understand what you mean by "reloading the dynamically patched modules" and if this is feasible without hacking too deep into the pyfakefs implementation

@mrbean-bremen
Copy link
Member Author

@konnerthg - I had another look yesterday and have found a slightly changed solution that seems to work with all versions, I just need to write an adequate test for this. I may check in something tonight for you to test if you want (you will have to install pyfakefs from the main branch), and I hope to finish the fix over the weekend.

@mrbean-bremen
Copy link
Member Author

@konnerthg and @sgarcialaguna: I didn't come up with a reliable test for this - may add some later, if I can think of something - so I merged as is. Please test with pyfakefs main branch. If it works for you, I will make a patch release.

@konnerthg
Copy link

@mrbean-bremen I ran both the minimal reproducer (linked in the first comment) as well as our large project. the error no longer occurrs and everything workes as expected. thank you for the fix. looking forward to the release

@mrbean-bremen
Copy link
Member Author

@konnerthg - thanks, good to hear that! Unfortunately, the experience @sgarcialaguna had was mixed - it started to work for one case, but not for another (depending on the number of tests running simultaneoulsy), so I will try to understand that first before doing a patch release.

@mrbean-bremen
Copy link
Member Author

I have meanwhile added back the possibility to disable dynamic patching - it works around the problem in the failing case reported by @sgarcialaguna. There is also a regression due to the current fix, and I'm going to handle further changes in that issue. Closing this one as fixed or at least worked around.

@mrbean-bremen
Copy link
Member Author

Reopening this - I had another look, and this is so far only a probem with django, while reloading all dynamically loaded modules after the test has unwanted side effects in other cases.
I think I'm going to replace the generic change by a more specific workaround for django, and in this case it needs to be retested with the known issues.

@mrbean-bremen mrbean-bremen reopened this Jan 23, 2024
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Feb 25, 2024
- reload only the django view modules instead of all modules
- change the module cleanup mode default to always delete modules
  (avoiding problems with reloading other modules)
- closes pytest-dev#932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants