Changed patch behavior to rely on module names instead of imported names

- adapted tests and documentation

CHANGES.md

@@ -7,6 +7,8 @@ This version of pyfakefs does not support Python 3.3. Python 3.3 users shall
 keep using pyfakefs 3.4.3, or upgrade to a newer Python version.
 
 #### New Features
+  * a module imported as another name (`import os as _os`) is now correctly 
+    patched without the need of additional parameters
   * automatically patch `Path` if imported like `from pathlib import Path` 
     ([#440](../../issues/440))
   * added side_effect option to fake files ([#433](../../pull/433))

docs/intro.rst

@@ -30,9 +30,9 @@ Limitations
 pyfakefs will not work with Python libraries that use C libraries to access the
 file system, because it cannot patch the underlying C libraries' file access functions.
 
-pyfakefs works out of the box only if the faked file system modules are loaded directly,
-e.g. ``import pathlib`` works, but ``from pathlib import Path`` does not. See
-:ref:`customizing_patcher` for ways to work around this.
+Depending on the kind of import statements used, pyfakefs may not patch the
+file system automatically. See :ref:`customizing_patcher` for more
+information and ways to work around this.
 
 pyfakefs is only tested with CPython and newest PyPy versions, other Python implementations
 will probably not work.

docs/usage.rst

@@ -89,9 +89,31 @@ deprecated and will not be described in detail.
 
 Customizing Patcher and TestCase
 --------------------------------
+Pyfakefs automatically patches file system related modules
+
+- if they are imported directly, e.g:
+
+.. code:: python
+
+  import os
+  import pathlib.Path
+
+- if they are imported under another name:
+
+.. code:: python
+
+  import os as my_os
+
+- if there are imported using one of these two specially handled statements:
+
+.. code:: python
+
+  from os import path
+  from pathlib import Path
+
+There are other cases where automatic patching does not work.
 Both ``fake_filesystem_unittest.Patcher`` and ``fake_filesystem_unittest.TestCase``
-provide a few additional arguments for fine-tuning. These are only needed if
-patching does not work for some module.
+provide a few additional arguments to handle such cases.
 In case of ``fake_filesystem_unittest.TestCase``, these arguments can either
 be set in the TestCase instance initialization, or in the call of
 ``setUpPyfakefs()``.
@@ -100,32 +122,16 @@ be set in the TestCase instance initialization, or in the call of
   use ``Patcher`` directly instead of the ``fs`` fixture. Alternatively,
   you can add your own fixture with the needed parameters.
 
-  An example for both approches can be found in
+  An example for both approaches can be found in
   `pytest_fixture_test.py <https://github.com/jmcgeheeiv/pyfakefs/blob/master/pyfakefs/tests/pytest/pytest_fixture_test.py>`__
   with the example fixture in `conftest.py <https://github.com/jmcgeheeiv/pyfakefs/blob/master/pyfakefs/tests/pytest/conftest.py>`__.
 
+
 modules_to_reload
 ~~~~~~~~~~~~~~~~~
 This allows to pass a list of modules that shall be reloaded, thus allowing
-to patch modules not imported directly.
-
-Pyfakefs automatically patches modules only if they are imported directly, e.g:
-
-.. code:: python
-
-  import os
-  import pathlib.Path
-
-The following import of ``os`` will not be patched by ``pyfakefs``, however:
-
-.. code:: python
-
-  import os as my_os
+to patch modules not patched automatically.
 
-.. note:: There are two exceptions to that: importing ``os.path`` like
-  ``from os import path`` will work, because it is handled by ``pyfakefs``.
-  The same is true for ``pathlib.Path`` if imported like ``from pathlib import
-  Path``.
 
 If adding the module containing these imports to ``modules_to_reload``, they
 will be correctly patched.
@@ -174,7 +180,8 @@ use_dynamic_patch
 ~~~~~~~~~~~~~~~~~
 If ``True`` (the default), dynamic patching after setup is used (for example
 for modules loaded locally inside of functions).
-Can be switched off if it causes unwanted side effects.
+Can be switched off if it causes unwanted side effects. This parameter may
+be removed in the future if no use case turns up for it.
 
 Using convenience methods
 -------------------------

pyfakefs/fake_filesystem_unittest.py

@@ -77,6 +77,7 @@
 else:
     import builtins
 
+PATH_MODULE = 'ntpath' if sys.platform == 'win32' else 'posixpath'
 
 def load_doctests(loader, tests, ignore, module,
                   additional_skip_names=None):  # pylint: disable=unused-argument
@@ -313,14 +314,18 @@ def __init__(self, additional_skip_names=None,
             'shutil': fake_filesystem_shutil.FakeShutilModule,
             'io': fake_filesystem.FakeIoModule,
         }
+
+        # class modules maps class names against a list of modules they can
+        # be contained in - this allows for alternative modules like
+        # `pathlib` and `pathlib2`
         self._class_modules = {}
         if pathlib:
             self._fake_module_classes[
                 'pathlib'] = fake_pathlib.FakePathlibModule
             self._fake_module_classes[
                 'Path'] = fake_pathlib.FakePathlibPathModule
             mod_name = 'pathlib2' if pathlib2 is not None else 'pathlib'
-            self._class_modules['Path'] = mod_name
+            self._class_modules['Path'] = [mod_name]
         if use_scandir:
             self._fake_module_classes[
                 'scandir'] = fake_scandir.FakeScanDirModule
@@ -329,20 +334,12 @@ def __init__(self, additional_skip_names=None,
             for name, fake_module in modules_to_patch.items():
                 if '.' in name:
                     module_name, name = name.split('.')
-                    self._class_modules[name] = module_name
+                    self._class_modules.setdefault(name, []).append(
+                        module_name)
                 self._fake_module_classes[name] = fake_module
 
-        self._modules = {}
-        for name in self._fake_module_classes:
-            self._modules[name] = set()
-        self._modules['path'] = set()
-
-        self._find_modules()
-
-        assert None not in vars(self).values(), \
-            "_findModules() missed the initialization of an instance variable"
-
         # Attributes set by _refresh()
+        self._modules = {}
         self._stubs = None
         self.fs = None
         self.fake_open = None
@@ -369,22 +366,26 @@ def _find_modules(self):
         Later, `setUp()` will stub these with the fake file system
         modules.
         """
+        module_names = list(self._fake_module_classes.keys()) + [PATH_MODULE]
         for name, module in set(sys.modules.items()):
-            if (module in self.SKIPMODULES or
-                    (not inspect.ismodule(module)) or
-                    name.split('.')[0] in self._skipNames):
+            try:
+                if (module in self.SKIPMODULES or
+                        not inspect.ismodule(module) or
+                        module.__name__.split('.')[0] in self._skipNames):
+                    continue
+            except AttributeError:
+                # workaround for some py (part of pytest) versions
+                # where py.error has no __name__ attribute
+                # see https://github.com/pytest-dev/py/issues/73
                 continue
-            for mod_name in self._modules:
-                mod = module.__dict__.get(mod_name)
-                if (mod is not None and
-                        (inspect.ismodule(mod) or
-                         inspect.isclass(mod) and
-                         mod.__module__ == self._class_modules.get(mod_name))):
-                    # special handling for path: check for correct name
-                    if (mod_name == 'path' and
-                            mod.__name__ not in ('ntpath', 'posixpath')):
-                        continue
-                    self._modules[mod_name].add((module, mod_name))
+            modules = {name: mod for name, mod in module.__dict__.items()
+                       if inspect.ismodule(mod) and
+                       mod.__name__ in module_names
+                       or inspect.isclass(mod) and
+                       mod.__module__ in self._class_modules.get(name, [])}
+            for name, mod in modules.items():
+                self._modules.setdefault(name, set()).add((module,
+                                                           mod.__name__))
 
     def _refresh(self):
         """Renew the fake file system and set the _isStale flag to `False`."""
@@ -395,7 +396,7 @@ def _refresh(self):
         self.fs = fake_filesystem.FakeFilesystem()
         for name in self._fake_module_classes:
             self.fake_modules[name] = self._fake_module_classes[name](self.fs)
-        self.fake_modules['path'] = self.fake_modules['os'].path
+        self.fake_modules[PATH_MODULE] = self.fake_modules['os'].path
         self.fake_open = fake_filesystem.FakeFileOpen(self.fs)
 
         self._isStale = False
@@ -417,7 +418,7 @@ def setUp(self, doctester=None):
         self._stubs.smart_set(builtins, 'open', self.fake_open)
         for name in self._modules:
             for module, attr in self._modules[name]:
-                self._stubs.smart_set(module, attr, self.fake_modules[name])
+                self._stubs.smart_set(module, name, self.fake_modules[attr])
 
         self._dyn_patcher = DynamicPatcher(self)
         sys.meta_path.insert(0, self._dyn_patcher)
@@ -441,7 +442,6 @@ def replace_globs(self, globs_):
         for name in self._fake_module_classes:
             if name in globs:
                 globs[name] = self._fake_module_classes[name](self.fs)
-            globs['path'] = globs['os'].path
         return globs
 
     def tearDown(self, doctester=None):
@@ -463,9 +463,6 @@ def __init__(self, patcher):
         self._patcher = patcher
         self.sysmodules = {}
         self.modules = self._patcher.fake_modules
-        if 'path' in self.modules:
-            self.modules['os.path'] = self.modules['path']
-            del self.modules['path']
 
         # remove all modules that have to be patched from `sys.modules`,
         # otherwise the find_... methods will not be called

pyfakefs/tests/fake_filesystem_unittest_test.py

@@ -53,15 +53,15 @@ def setUp(self):
 class TestPyfakefsUnittest(TestPyfakefsUnittestBase):  # pylint: disable=R0904
     """Test the `pyfakefs.fake_filesystem_unittest.TestCase` base class."""
 
-    @unittest.skipIf(sys.version_info > (2, ),
+    @unittest.skipIf(sys.version_info > (2,),
                      "file() was removed in Python 3")
     def test_file(self):
         """Fake `file()` function is bound"""
         self.assertFalse(os.path.exists('/fake_file.txt'))
         with file('/fake_file.txt', 'w') as f:  # noqa: F821 is only run on Py2
             f.write("This test file was created using the file() function.\n")
         self.assertTrue(self.fs.exists('/fake_file.txt'))
-        with file('/fake_file.txt') as f:       # noqa: F821 is only run on Py2
+        with file('/fake_file.txt') as f:  # noqa: F821 is only run on Py2
             content = f.read()
         self.assertEqual(content, 'This test file was created using the '
                                   'file() function.\n')
@@ -137,34 +137,28 @@ def test_fakepathlib(self):
             self.assertTrue(self.fs.exists('/fake_file.txt'))
 
 
-class TestImportAsOtherNameInit(fake_filesystem_unittest.TestCase):
-    def __init__(self, methodName='RunTest'):
-        modules_to_load = [pyfakefs.tests.import_as_example]
-        super(TestImportAsOtherNameInit, self).__init__(
-            methodName, modules_to_reload=modules_to_load)
-
-    def setUp(self):
-        self.setUpPyfakefs()
-
-    def test_file_exists(self):
+class TestPatchingImports(TestPyfakefsUnittestBase):
+    def test_import_as_other_name(self):
         file_path = '/foo/bar/baz'
         self.fs.create_file(file_path)
         self.assertTrue(self.fs.exists(file_path))
         self.assertTrue(
-            pyfakefs.tests.import_as_example.check_if_exists(file_path))
-
-
-class TestImportAsOtherNameSetup(fake_filesystem_unittest.TestCase):
-    def setUp(self):
-        self.setUpPyfakefs(
-            modules_to_reload=[pyfakefs.tests.import_as_example])
+            pyfakefs.tests.import_as_example.check_if_exists1(file_path))
 
-    def test_file_exists(self):
+    def test_import_path_from_os(self):
+        """Make sure `from os import path` patches `path`."""
         file_path = '/foo/bar/baz'
         self.fs.create_file(file_path)
         self.assertTrue(self.fs.exists(file_path))
         self.assertTrue(
-            pyfakefs.tests.import_as_example.check_if_exists(file_path))
+            pyfakefs.tests.import_as_example.check_if_exists2(file_path))
+
+    if pathlib:
+        def test_import_path_from_pathlib(self):
+            file_path = '/foo/bar'
+            self.fs.create_dir(file_path)
+            self.assertTrue(
+                pyfakefs.tests.import_as_example.check_if_exists3(file_path))
 
 
 class TestAttributesWithFakeModuleNames(TestPyfakefsUnittestBase):
@@ -193,35 +187,25 @@ class TestPathNotPatchedIfNotOsPath(TestPyfakefsUnittestBase):
        An own path module (in this case an alias to math) can be imported
        and used.
     """
+
     def test_own_path_module(self):
         self.assertEqual(2, path.floor(2.5))
 
 
-if pathlib:
-    class PatchPathlibPathTest(TestPyfakefsUnittestBase):
-        """Shows that pathlib.Path is correctly patched."""
-        def test_path_exists(self):
-            file_path = '/foo/bar'
-            self.fs.create_dir(file_path)
-            self.assertTrue(
-                pyfakefs.tests.import_as_example.check_if_path_exists(
-                    file_path))
-
-
-class PatchAsOtherNameTest(TestPyfakefsUnittestBase):
-    """Patches a module imported under another name using `modules_to_patch`.
-    This is an alternative to reloading the module.
+class ReloadModuleTest(fake_filesystem_unittest.TestCase):
+    """Make sure that reloading a module allows patching of classes not
+    patched automatically.
     """
-    def __init__(self, methodName='RunTest'):
-        modules_to_patch = {'my_os': fake_filesystem.FakeOsModule}
-        super(PatchAsOtherNameTest, self).__init__(
-            methodName, modules_to_patch=modules_to_patch)
+    def setUp(self):
+        """Set up the fake file system"""
+        self.setUpPyfakefs(
+            modules_to_reload=[pyfakefs.tests.import_as_example])
 
     def test_path_exists(self):
         file_path = '/foo/bar'
         self.fs.create_dir(file_path)
         self.assertTrue(
-            pyfakefs.tests.import_as_example.check_if_exists(file_path))
+            pyfakefs.tests.import_as_example.check_if_exists4(file_path))
 
 
 class TestCopyOrAddRealFile(TestPyfakefsUnittestBase):
@@ -353,6 +337,7 @@ def test_set_up_pyfakefs(self):
 
 class TestShutilWithZipfile(fake_filesystem_unittest.TestCase):
     """Regression test for #427."""
+
     def setUp(self):
         self.setUpPyfakefs()
         self.fs.create_file('foo/bar')

pyfakefs/tests/import_as_example.py

@@ -14,6 +14,7 @@
 Example module that is used for testing modules that import file system modules
 to be patched under another name.
 """
+from os import path
 import os as my_os
 
 try:
@@ -25,10 +26,22 @@
         Path = None
 
 
-def check_if_exists(filepath):
+def check_if_exists1(filepath):
+    # test patching module imported under other name
     return my_os.path.exists(filepath)
 
 
+def check_if_exists2(filepath):
+    # tests patching path imported from os
+    return path.exists(filepath)
+
+
 if Path:
-    def check_if_path_exists(filepath):
+    def check_if_exists3(filepath):
+        # tests patching Path imported from pathlib
         return Path(filepath).exists()
+
+
+def check_if_exists4(filepath, exists=my_os.path.exists):
+    # this is a similar case as in the tempfile implementation under Posix
+    return exists(filepath)