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

Module scoped pytest fixture raises error #684

Closed
iSplasher opened this issue Jul 5, 2022 · 10 comments · Fixed by #685
Closed

Module scoped pytest fixture raises error #684

iSplasher opened this issue Jul 5, 2022 · 10 comments · Fixed by #685

Comments

@iSplasher
Copy link

Describe the bug
Hello, when I try to create this

@pytest.fixture(scope="module")
def fs_module():
    patcher = Patcher()
    patcher.setUp()
    yield patcher.fs
    patcher.tearDown()

I get this error when the function yields

p = WindowsPath('D:/TEMP/pytest-of-User/pytest-9')

    def create_cleanup_lock(p: Path) -> Path:
        """Create a lock to prevent premature folder cleanup."""
        lock_path = get_lock_path(p)
        try:
>           fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644)
E           FileNotFoundError: [Errno 2] No such file or directory: 'D:\\TEMP\\pytest-of-User\\pytest-9\\.lock'

C:\dev\code\env\lib\site-packages\_pytest\pathlib.py:234: FileNotFoundError

Your environment
Please run the following and paste the output.

- Windows-10-10.0.22000-SP0
- Python 3.10.5 (tags/v3.10.5:f377153, Jun  6 2022, 16:14:13) [MSC v.1929 64 bit (AMD64)]
- pyfakefs 4.5.6
- pytest 7.1.2
@mrbean-bremen
Copy link
Member

Can you please add a minimal reproducible example, or at least a bit more context? Is p a global variable (which could be a problem, because it is not patched)?

@mrbean-bremen
Copy link
Member

I assume this happens only with the module-scoped fixture, not with the function-scoped fs fixture, is this correct?

@iSplasher
Copy link
Author

iSplasher commented Jul 5, 2022

I apologize for the lack of info. I just managed to create a repro:

# conftest.py

import pytest
from pyfakefs.fake_filesystem_unittest import Patcher

@pytest.fixture(autouse=True)
def test_internaldb( tmpdir ):
    yield



@pytest.fixture(scope="module")
def fs_module():
    patcher = Patcher()
    patcher.setUp()
    yield patcher.fs
    patcher.tearDown()
# test_fail.py

def test_fail(fs_module):
    assert True

Creating a clean environment using the latest pytest and pyfakefs with these two files produces the error.

Seems like the tmpdir fixture in the auto-used fixture is the culprit.

@mrbean-bremen
Copy link
Member

Thank you for the example! At a first this looks like a clash between the 2 fixtures - at some point the temp file is probably created in the fake filesystem, but accessed in the real fs. It may be that the fixtures just won't work together nicely, or that pyfakefs does not properly patch something used by tmpdir. I will have a closer look at this tonight.

Keep in mind that you won't really use the temp directory here, as it will live in the fake file system. Maybe you don't need the tmpdir fixture here at all, but can use real paths (that will live in the fake fs) instead.

@iSplasher
Copy link
Author

Thanks for looking into this!

Keep in mind that you won't really use the temp directory here, as it will live in the fake file system. Maybe you don't need the tmpdir fixture here at all, but can use real paths (that will live in the fake fs) instead.

You are correct, however, I'm using the fixture with tmpdir in a lot of other test functions which don't depend on a fake fs. A workaround is to remove the autouse parameter and manually specify it in those test functions, but if it could work automatically that would be fantastic!

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Jul 6, 2022

This is a regression in pyfakefs, though I'm not sure yet how to fix it. The change has been introduced in pyfakefs 3.5.5.
You can disable this change if you add the following lines in your conftest.py:

import _pytest

Patcher.SKIPMODULES.remove(_pytest.pathlib)

I suspect though that this will cause other problems, as the change has been introduced for compatibility with pytest 7.0. If that is the case, you can temporary downgrade to pytest < 7.0 (additionally to that change). A fix will probably take some time, so you can use this as a workaround for the time being.

Some background info (mostly for myself, you may ignore this):
The change had been introduced for #666 (yes, I knew that an issue with that number would bite us one day 😁), as a compatibility fix for pytest 7.0 that was needed due to an implementation change in the internal pathlib module in pytest. This module had previously used the fake fs, and the creation of temp files as used in tmpdir worked fine in the fake fs. Due to an implementation detail in pytest 7 that internal module cannot longer be patched by pyfakefs (at least I haven't found a way to do this). Now that it is not faked, it calls a function in Python's pathlib.mkdir to create the root temp directory, which is still patched, so the directory is created in the fake fs. Then it creates a lock file in that directory using the unfaked os.open function, which fails because the directory does not exist in the real fs. What could fix this is to ensure that the pathlib calls from pytest's pathlib are not patched - there is some infrastructure for this in the main branch needed for the changed pathlib implementation in Python 3.11, though I'm not sure yet if it can be used here.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Jul 10, 2022
- fixes pytest-dev#666 without the need to skip _pytest.pathlib patching
- add module and session scoped fs fixtures
- fixes pytest-dev#684
mrbean-bremen added a commit that referenced this issue Jul 11, 2022
- fixes #666 without the need to skip _pytest.pathlib patching
- add module and session scoped fs fixtures
- fixes #684
@mrbean-bremen
Copy link
Member

There turned out to be an easy fix for #666 that I overlooked at the time - should be fixed in master now. I'll probably make a new release in a few days if nothing else comes up.

github-actions bot pushed a commit that referenced this issue Jul 11, 2022
…thout the need to skip _pytest.pathlib patching - add module and session scoped fs fixtures - fixes #684
@mrbean-bremen
Copy link
Member

Reopen as there still can be an error logged on test shutdown (which does not affect the tests though).

@mrbean-bremen
Copy link
Member

Removing regression label, as the regression is fixed.

@mrbean-bremen
Copy link
Member

I investigated some more, and it turned out to be a problem with combining the usage of tmpdir, the module_fs fixture, and tests that emulate another file system. If emulating another file system, the file system is reset, which doesn't play nice with a tmpdir created in the fake file system before (the reset of the fake filesystem removes the created root dir for the temp files, which pytest tries to cleanup on shutdown). I can't think of an easy workaround here, but this should not be a problem in real world tests, so I'm closing this now. The real problem has been fixed before. If it will happen in real tests, I may have another look.

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 a pull request may close this issue.

2 participants