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

Patcher.tearDown() issue with pathlib #518

Closed
sagi-z opened this issue Feb 15, 2020 · 10 comments
Closed

Patcher.tearDown() issue with pathlib #518

sagi-z opened this issue Feb 15, 2020 · 10 comments
Labels

Comments

@sagi-z
Copy link

sagi-z commented Feb 15, 2020

Describe the bug
If pathlib is imported between Patcher.setUp() and Patcher.tearDown() for the first time then
Patcher.tearDown() does not cleanup pathlib static accessors correctly.

How To Reproduce
Basically I'm using the 'fs' fixture plugin for pytest. I have tests which use pyfakefs and then some tests which don't use it. A simplified example of the problem decoupled from pytest is:

(bash) $ cat /tmp/pyfakefs_bug.py 
from pyfakefs.fake_filesystem_unittest import Patcher
import os
import shutil

# Use this directory as a showcase, remove it before testing
TMP_DIR = '/tmp/delmedir/somedir'
shutil.rmtree(TMP_DIR, ignore_errors=True)

# Patch setUp and teadDown
patcher = Patcher()
patcher.setUp()
import pathlib  # first imported while patched
dummy = pathlib.Path()
patcher.tearDown()

# We should not be patched here
p = pathlib.Path(TMP_DIR)
p.mkdir(parents=True, exist_ok=True)  # Problem because Path._accessor points to a class statically initialized during setUp
new_file = os.path.join(str(p), 'new-file')
file_obj = open(new_file, mode='w')

(bash) $ python /tmp/pyfakefs_bug.py
Traceback (most recent call last):
  File "/tmp/pyfakefs_bug.py", line 20, in <module>
    file_obj = open(new_file, mode='w')
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/delmedir/somedir/new-file'

Your enviroment
Please run the following and paste the output.

python -c "import platform; print(platform.platform())"
python -c "import sys; print('Python', sys.version)"
python -c "from pyfakefs.fake_filesystem import __version__; print('pyfakefs', __version__)"

Linux-4.15.0-76-generic-x86_64-with-debian-buster-sid
Python 3.7.3 (default, Nov 21 2019, 15:51:40)
[GCC 7.4.0]
pyfakefs 3.7.1

@mrbean-bremen
Copy link
Member

Good point. This is a general problem for modules loaded during patching, that is not specific to pathlib (and not related to the static accessor) - the loaded module retains the patched version. This gives the same behavior:

from pyfakefs.fake_filesystem_unittest import Patcher
import shutil

TMP_DIR = '/tmp/delmedir/somedir'
shutil.rmtree(TMP_DIR, ignore_errors=True)

with Patcher() as patcher:
    import os

# We should not be patched here
os.makedirs(TMP_DIR, exist_ok=True)
new_file = os.path.join(TMP_DIR, 'new-file')
file_obj = open(new_file, mode='w')

This never came up until now (the use case is not very common) - I will check how to resolve this, thanks for another bug report!

@mrbean-bremen
Copy link
Member

The obvious workaround is to import the module (pathlib or os) before patching.

@sagi-z
Copy link
Author

sagi-z commented Feb 16, 2020

Thanks for the quick reply, but I'm having troubles using this workaround with pytest.
I'm using pytest and I already have an import in my test for pathlib.
How can I workaround this using pytest with/without the fs fixture and the pytest_plugin.py?
I tried 2 approaches which didn't work for me for some reason (guessing this relates to how pytest loads modules for tests, but this is just a guess):

  1. Use import pathlib in my conftest.py, and actually using Path() globally. Didn't work.
  2. In addition to the above, also copy the pytest_plugin.py contents to my conftest.py and rename the fixture, using the new name. Didn't work.

Do you know what am I missing here?

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Feb 16, 2020

I have a bit trouble to reproduce your scenario. In the simple example you have shown the problem goes away if pathlib is imported outside of the test fixture (before setUp or after tearDown), at least for me.
As far as I understand, you use pytest for tests with and without the fs fixture, and the problem occurs if you import pathlib in a test and use it in another without fs. Maybe you can show an example where the workaround doesn't work?

The best would be to fix the problem, of course, but I had no good idea how to handle this yet, and due to time constraints this may take some time, so it would be nice to have a workaround that actually works for the time being...

@sagi-z
Copy link
Author

sagi-z commented Feb 17, 2020

Okay, so it turns out the problem is not directly pytest related. We have in the code a kind of global class with a pathlib.Path initialized once.
Here is some example code for which I need a pyfakefs solution as it won't always be tested with
pyfakefs:

from pyfakefs.fake_filesystem_unittest import Patcher
import os
import shutil

# Use this directory as a showcase, remove it before testing
TMP_DIR = '/tmp/delmedir/somedir'
shutil.rmtree(TMP_DIR, ignore_errors=True)

import pathlib  # not imported while patched


class PathResolver:
    def __init__(self):
        self.p = pathlib.Path(TMP_DIR)

    def get_cache_dir(self, sub_dir):
        ret = self.p.joinpath(sub_dir)
        ret.mkdir(parents=True, exist_ok=True)
        return ret


# Patch setUp and teadDown
with Patcher():
    path_resolver = PathResolver()

new_dir = path_resolver.get_cache_dir('my-plugin')
print(new_dir._accessor.mkdir)  # Note this is FakeFilesystem.makedir
new_file = os.path.join(str(new_dir), 'new-file')
file_obj = open(new_file, mode='w')

Without the Patcher this works fine.
Thanks.

@mrbean-bremen
Copy link
Member

Ok, thanks - I will have a closer look tonight.

@mrbean-bremen
Copy link
Member

Ok, I understand. I'm afraid that this will not work as is.

The problem is that PathResolver.p is assigned the fake path, and there is no possibility to change that automatically to the real path short of getting a new PathResolver object. Reloading only works with code that will be executed during reload, not function code.

What you try is to use an object created in the fake filesystem in the real file system. This will not work with any such object, be it a path, a file object, or a function pointer. You have to decide if the tests using the resolver run in the real or the fake file system.

For example, you could map a part of the real file system into the fake file system, if you are able to run the whole test there, though without knowing your concrete use case it is difficult to say.

@sagi-z
Copy link
Author

sagi-z commented Feb 18, 2020

I see.
I have worked around this by adding a reset method to actual PathResolver. I use it in a fixture wrapping your fixture. I would have preferred not to change the runtime code for this...
The only current reason some tests cannot use the pyfakefs are issues #516 and #517 .
Hoping those issues will be fixed soon.
Thank for this great library and for your efforts.

@mrbean-bremen
Copy link
Member

I will see what I can do with these issues. I already had a go, and they turned out to be a bit more tricky than expected. I don't have much free time at the moment, so it may take a bit...

@mrbean-bremen
Copy link
Member

After thinking a bit more about this, I do not consider this a bug - objects created in a fake environment cannot be used outside that environment due to side effects (e.g. fake objects assigned and processed). There is nothing we can do here - closing the issue.

Thanks anyway for the report that helped to understand the problem, and the other bug reports!

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

No branches or pull requests

2 participants