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

[wanted] Easy way to 'embed' certain directories in the virtual-file system #170

Closed
dwt opened this issue Mar 30, 2017 · 28 comments
Closed
Assignees

Comments

@dwt
Copy link

dwt commented Mar 30, 2017

Hi there,

while trying to test my flask application with pyfakefs, I had to do quite some magic to get it working.

My chief problems where, that a) jinja2 (in development mode at least) insists on always loading it's templates from disk and b) babel loads it's datafilee lazily from disk in time for request handling.

Both of these where impossible to workaround by using the additional_skip_modules, so I ended up doing this instead:

    def _restore_accss_to_crucial_files(self):
        from pyfakefs.fake_filesystem_unittest import REAL_OS, REAL_OPEN
        def copy_in_file(real_path):
            real_stat = REAL_OS.stat(real_path)
            with REAL_OPEN(real_path, 'rb') as real_file:
                real_contents = real_file.read()
            fake_file = self.fs.create_file(real_path, st_mode=real_stat.st_mode,
                                           contents=real_contents,
                                           create_missing_dirs=True)
            fake_file.st_ctime = real_stat.st_ctime
            fake_file.st_atime = real_stat.st_atime
            fake_file.st_mtime = real_stat.st_mtime
            fake_file.st_gid = real_stat.st_gid
            fake_file.st_uid = real_stat.st_uid
            return fake_file
        
        def copy_in_directory(directory_path):
            for name in REAL_OS.listdir(directory_path):
                path = REAL_OS.path.join(directory_path, name)
                copy_in_file(path)
            
        template_dir = REAL_OS.path.join(REAL_OS.path.dirname(__file__), 'templates')
        copy_in_directory(template_dir)
        
        import babel
        babel_dir = REAL_OS.path.dirname(babel.__file__)
        copy_in_file(REAL_OS.path.join(babel_dir, 'global.dat'))
        babel_data_dir = REAL_OS.path.join(babel_dir, 'locale-data')
        copy_in_directory(babel_data_dir)

This workaround works, but it has the unfortunate side effect of always copying megabytes of data around.

So this is what I'm asking about: Could you provide an easy way to allow access to certain parts of the file system in an easy way, by 'embedding' access to them into the fake filesystem? Bonus points if it's read only...

Some things I can imagine being very useful:

  • Allowing (read only) access to site_packages (of the current virtual env?)
  • Allow (read only) access to the source code of the current package to load ressources like templates for jinja2

What do you think?

@dwt
Copy link
Author

dwt commented Mar 30, 2017

Oh, not to forget that hardcoding all those paths is brittle as hell.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Mar 30, 2017

Hi @dwt ,
I think this is a valid request. A couple of points:

There is already TestCase.copyRealFile(), so no need to roll your own (it basically does the same as copy_in_file). Just in case you need this elsewhere...

I had a similar problem with django templates and some fixtures of my own, and my solution was in principle similar to yours - I added a method to my test class (derived from pyfakefs.TestCase) that takes a list of files and directories; these are read into memory before setting up pyfakefs, and later added to the fake fs. As there was not that much data, this was ok for me.

I think it makes sense to add a similar method that you can overwrite in your test case, with the difference that it only collects the file paths (recursively for directories) and adds them to the fake file system without reading the contents. The files would be created with an extra flag which is considered if accessing the file contents.

Regarding read-only access: in any case, I would not want to allow write access to the real file system from inside pyfakefs. I see 2 options here:

  • throw if trying to write into such a file
  • allow write access in memory as usually (reading the contents from disk first if needed)

Actually there could be a third option to allow both by adding separate methods for read-only and read-write access.

What do you think?

@dwt
Copy link
Author

dwt commented Mar 31, 2017

@mrbean-bremen Well, I have to admit that I stole copyRealFile because my TestCases are not subclasses of your TestCase, and copyRealFile wast therefore not easily accessible. So from an API standpoint, I would really like it if the endpoints are exposed on the FileSystem instead of on the TestCase.

Ideally I would imagine FakeFilesystem having methods like .expose_directory_for_reading(some_path) or .expose_file_for_reading(some_path), maybe even .read_only_expose(some_path) Not sure what the best name would be, but something in that general vicinity.

I'd say, API first, integration into TestCase later.

Regarding read only access: I would be completely happy to have read only access be the only possibility right now. Creating the files lazily in the fake filesystem on read access seems like a good way to handle this. When writing into such a file it is in the FakeFileSystem so the usual tools are there to test what happens. This would be enough I gather. Being able to get an exception when this happens (as it should be unexpected) would be a nice bonus, but not essential.

Read-Write access (to me) would be something I am happy to postpone for now - if it is more work to implement.

@dwt
Copy link
Author

dwt commented Mar 31, 2017

Oh, also something which would be really convenient is having methods to directly allow access to site-packages and the current project.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Mar 31, 2017

Ok, understood. As I'm only using the unittest approach, I usually see pyfakefs from that perspective (same goes for @jmcgeheeiv I guess, who wrote the unittest part). I will see if I can throw something together over the weekend. I'm thinking about adding something like AddRealFile() and AddRealDirectory() (this conforms to the Google coding guideline used throughout this API), and maybe AddRealPaths() for convenience.
If this works, I may think about adding access to some special locations for convenience.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 1, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see pytest-dev#170
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 1, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see pytest-dev#170
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 1, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see pytest-dev#170
@mrbean-bremen
Copy link
Member

@dwt - I put together a PR that implements the 3 methods mentioned above. Note that AddRealDirectory() enumerates all contained files recursively and adds them to the fake fs. This may slow down the test if you add a directory with a large number of contained files, but as it is far simpler than a lazy evaluation approach I would prefer it for now. Please check if this is what you need. I may add some more functionality later if needed.

@mrbean-bremen mrbean-bremen self-assigned this Apr 1, 2017
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 2, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see pytest-dev#170
@jmcgeheeiv jmcgeheeiv changed the title [wanted] Easy way to 'embedd' certain directories in the virtual-file system [wanted] Easy way to 'embed' certain directories in the virtual-file system Apr 3, 2017
@dwt
Copy link
Author

dwt commented Apr 6, 2017

Trying this out today, I've got a few problems that I seem to originate inside pyfakefs, but I'm not entirely sure yet.

I've tried this (your code is accessed via a pep8'tifing wrapper):

template_dir = os.path.join(os.path.dirname(__file__), 'templates')

import babel
babel_dir = os.path.dirname(babel.__file__)
babel_global_data = os.path.join(babel_dir, 'global.dat')
babel_data_dir = os.path.join(babel_dir, 'locale-data')

self.fs.add_real_paths([
    template_dir,
    babel_global_data,
    babel_data_dir,
])

But when I then try to access this directory, I get this:

(Pdb) os.listdir(template_dir)
*** IsADirectoryError: [Errno 21] Is a directory: '/Users/dwt/Code/Projekte/.../templates'
# path shortened

os.listdir'ing the directory above works fine - but not for templates. Any idea what is happening?

@mrbean-bremen
Copy link
Member

This is very strange - I just looked, and this exception is only raised in RemoveFile(), which certainly should not be called in os.listdir(). I will have a closer look later...

@mrbean-bremen
Copy link
Member

A second look didn't take me any further. Somehow RemoveFile() seems to be called, which is the fake fs implementation for os.remove(). This should not be possible just by calling os.listdir(), unless something has been patched / mocked incorrectly. Can you give me some stripped down source that reproduces the problem? Without more context, I'm at a loss here.
On another topic (as you mentioned pep8'tifying the code) - @jmcgeheeiv may do this with pyfakefs, too (we shortly discussed this yesterday), so I may rename the methods to conform to PEP 8 before merging them - but first I have to understand your problems.

@dwt
Copy link
Author

dwt commented Apr 7, 2017

Sure, but I'm out of town for the weekend, so I'm not sure when I'll get to it.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 11, 2017

@dwt - I just renamed the functions to be PEP 8 conform in the PR, so that you may use them directly.

@dwt
Copy link
Author

dwt commented Apr 12, 2017

:-)

I've tried this reduction which reproduces the problem for me:

#!/usr/bin/env python

import os

template_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'redacted/web/templates'))

import babel
babel_dir = os.path.dirname(babel.__file__)
babel_global_data = os.path.join(babel_dir, 'global.dat')
babel_data_dir = os.path.join(babel_dir, 'locale-data')

print(os.listdir(template_dir))
print(os.listdir(babel_dir))

from pyfakefs.fake_filesystem_unittest import Patcher
stubber = Patcher()
stubber.setUp()

stubber.fs.add_real_paths([
    template_dir,
    babel_global_data,
    babel_data_dir,
])

print(os.listdir(babel_dir)) # works
print(os.listdir(template_dir)) # raises

Running it produces this output for me:

$ ./reduction.py                                        :(
['base.html.j2', 'batch_list.html.j2', 'error.html.j2', 'form.html.j2', 'layout.html.j2', 'login.html.j2']
['__init__.py', '__pycache__', '_compat.py', 'core.py', 'dates.py', 'global.dat', 'languages.py', 'lists.py', 'locale-data', 'localedata.py', 'localtime', 'messages', 'numbers.py', 'plural.py', 'support.py', 'units.py', 'util.py']
['global.dat', 'locale-data']
Traceback (most recent call last):
  File "./reduction.py", line 26, in <module>
    print(os.listdir(template_dir)) # raises
  File "/Users/dwt/Code/Projekte/Fnord/opensource/pyfakefs/pyfakefs/fake_filesystem.py", line 2856, in listdir
    return self.filesystem.ListDir(target_directory)
  File "/Users/dwt/Code/Projekte/Fnord/opensource/pyfakefs/pyfakefs/fake_filesystem.py", line 2093, in ListDir
    directory_contents = directory.contents
  File "/Users/dwt/Code/Projekte/Fnord/opensource/pyfakefs/pyfakefs/fake_filesystem.py", line 227, in contents
    if sys.version_info >= (3, 0) and isinstance(self.byte_contents, bytes):
  File "/Users/dwt/Code/Projekte/Fnord/opensource/pyfakefs/pyfakefs/fake_filesystem.py", line 220, in byte_contents
    with io.open(self.file_path, 'rb') as f:
IsADirectoryError: [Errno 21] Is a directory: '/Users/dwt/Code/Projekte/Fnord/redacted/web/templates'

Does that help reproducing the problem?

@mrbean-bremen
Copy link
Member

Thanks! I will have a look in the evening.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 12, 2017

Ok, I could reproduce something similar (though not the same problem) and could fix this by adding genericpath to skipnames in Patcher (genericpath is imported from ntpath and posixpath modules and implements os.path.exists and some other path functions, which have been stubbed out unwantedly without this change).
I added this in the PR, but you also may use:
stubber = Patcher(additional_skip_names=['genericpath'])
in your test with the version you tested. I'm not sure if this fixes all the problems, though - please check it out.

@dwt
Copy link
Author

dwt commented Apr 13, 2017

I've updated to the latest version of your pull request and indeed that seems to fix the problem. Great work!

@dwt
Copy link
Author

dwt commented Apr 13, 2017

On a related note: this pull request halves the execution time of my test suite. :-) Yay!

@mrbean-bremen
Copy link
Member

Nice to hear that :) I will see if I can make a unit test that reproduces the problem - I didn't catch it with the existing ones - and merge it back later if @jmcgeheeiv approves it.
I may also use the feature myself for django related tests.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 13, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see pytest-dev#170
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 14, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see pytest-dev#170
mrbean-bremen added a commit that referenced this issue Apr 14, 2017
- allow to add really existing files and directory trees to the fake
file system, with the contents read on demand
- see #170
@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 14, 2017

A note about access to site-packages: there seems to be no reliable method, at least in a virtual environment. You have to use the path to the module instead (using yourmodule.__file__).

@dwt
Copy link
Author

dwt commented Apr 15, 2017

Yeah, iterating over all files in site packages is also quite slow, so doing it only for the files that are actually needed makes quite some sense (though it's tough to figure out).

@jakespracher
Copy link
Contributor

jakespracher commented Feb 16, 2023

I've been stuck trying to get existing django tests to pass after adding pyfakefs due to this general issue despite the additional helper methods in a07baf6

Couldn't parse a full solution from the thread, it seems like getting all the right static files copied over is not trivial.

Any tips? Would any of this be automatable for common libraries with filesystem dependencies behind an option or something to make this less painful? Bit of a roadblock for anyone looking to start using this library or testing in general with an existing django codebase.

If I can find time happy to look into opening a PR but at present I don't know how to solve this for myself.

@dwt
Copy link
Author

dwt commented Feb 16, 2023

@jakespracher: What I have done is look at the error messages / exceptions to find out which files are missing and then adding them or their directories to the fake fs as I find them. That seemed to work well enough?

Where exactly are you stuck, don't you get errors pointing to the missing files in Django land? (Sorry, I'm no Django user, so can't help you directly there)

@mrbean-bremen
Copy link
Member

I had been using django with pyfakefs at the time, and I did basically the same. I dimly remember that I had to add a termplates directory and the pytz directory with the timezone info, YMMV.
I don't think it is feasible to add some application-specific automatism, as the needs may vary widely depending on the use cases, the used packages and their versions.

@jakespracher
Copy link
Contributor

For some errors I had success. Currently stuck on django.template.exceptions.TemplateSyntaxError: 'static' is not a registered tag library. Must be one of: [blank] which is not super informative. As far as I can gather one of the dependency django apps has some unclear filesystem dependency.

@mrbean-bremen
Copy link
Member

This sounds as if it can't find some of your static files (e.g. from {% load static %} in a template), or from the library you mentioned. Though that won't help you, I guess...

@jakespracher
Copy link
Contributor

Thanks for all the input. I'll post what I figure out.

I don't think it is feasible to add some application-specific automatism, as the needs may vary widely depending on the use cases, the used packages and their versions.

Totally understand you would not want to be responsible. Maybe a separate package or wiki might make sense. Just something to make the "I added pyfakefs and it broke a million tests" problem a little less painful

@mrbean-bremen
Copy link
Member

Maybe a separate package or wiki might make sense.

I'm all for it if it helps. Adding some how-to to the documentation would certainly help - there is already a troubleshooting section where this could fit. We could add a section with hints for specific environments (like django), which could accumulate with time.

@jakespracher
Copy link
Contributor

jakespracher commented Feb 17, 2023

I ended up taking a pretty heavy handed approach to get unblocked:

@pytest.fixture
def fake_fs(fs):
    PROJECT_BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
    fs.add_real_paths(
        [
            PROJECT_BASE_DIR,
            os.path.dirname(django.__file__),
        ]
    )
    return fs

I can at least put together a PR to add this and other info from this thread to the troubleshooting section

@mrbean-bremen
Copy link
Member

I ended up taking a pretty heavy handed approach to get unblocked

This is ok - the contents are only accessed on demand, so this should normally not have a large performance impact.

jakespracher added a commit to jakespracher/pyfakefs that referenced this issue Feb 21, 2023
As discussed on pytest-dev#170, we expand the troubleshooting section to include some instructions for adding pyfakefs to a django project without breaking existing tests.
mrbean-bremen pushed a commit that referenced this issue Feb 21, 2023
As discussed on #170, we expand the troubleshooting section to include some instructions for adding pyfakefs to a django project without breaking existing tests.
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

3 participants