Use positional instead of keyword argument for patchfs

- avoids confudion with pytest fixture
- see pytest-dev#566

CHANGES.md

@@ -3,15 +3,23 @@ The released versions correspond to PyPi releases.
 
 ## Version 4.3.0 (as yet unreleased)
 
+
+### Changes
+  * The `patchfs` decorator now expects a positional argument instead of the
+    keyword arguments `fs`. This avoids confusion with the pytest `fs`
+    fixture and conforms to the behavior of `mock.patch`. You may have to
+    adapt the argument order if you use the `patchfs` and `mock.patch`
+    decorators together (see  [#566](../../issues/566)) 
+
 ## [Version 4.2.1](https://pypi.python.org/pypi/pyfakefs/4.2.1)
 
-This is a bugfix releases that fixes a regression issue.
+This is a bugfix release that fixes a regression issue.
 
 ### Fixes
   * remove dependency of pyfakefs on `pytest` (regression, 
     see [#565](../../issues/565)) 
 
-## [Version 4.2.0](https://pypi.python.org/pypi/pyfakefs/4.2.0)
+## [Version 4.2.0](https://pydpi.python.org/pypi/pyfakefs/4.2.0)
 
 #### New Features
   * add support for the `buffering` parameter in `open` 

docs/usage.rst

@@ -92,29 +92,38 @@ single function, you can write:
    from pyfakefs.fake_filesystem_unittest import patchfs
 
    @patchfs
-   def test_something(fs):
-       # access the fake_filesystem object via fs
-       fs.create_file('/foo/bar', contents='test')
+   def test_something(fake_fs):
+       # access the fake_filesystem object via fake_fs
+       fake_fs.create_file('/foo/bar', contents='test')
 
-Note the argument name ``fs``, which is mandatory. As this is a keyword
-argument, it must go after all positional arguments, such as arguments from
-``mock.patch``, regardless of the decorator order:
+Note the ``fake_fs`` is a positional argument and the argument name does not
+matter. If there are additional ``mock.patch`` decorators that also
+create positional arguments, the argument order is the same as the decorator
+order, as shown here:
 
 .. code:: python
 
-   @mock.patch('foo.bar')
    @patchfs
-   def test_something(mocked_bar, fs):
+   @mock.patch('foo.bar')
+   def test_something(fake_fs, mocked_bar):
        ...
 
-   @patchfs
    @mock.patch('foo.bar')
-   def test_something(mocked_bar, fs):
+   @patchfs
+   def test_something(mocked_bar, fake_fs):
        ...
 
-Don't confuse this with ``pytest`` tests, where ``fs`` is the fixture name
-(with the same functionality). If you use ``pytest``, you don't need this
-decorator.
+.. note::
+  Avoid writing the ``patchfs`` decorator *between* ``mock.patch`` operators,
+  as the order will not be what you expect. Due to implementation details,
+  all arguments created by ``mock.patch`` decorators are always expected to
+  be contiguous, regardless of other decorators positioned between them.
+
+.. caution::
+  In previous versions, the keyword argument `fs` has been used instead,
+  which had to be positioned *after* all positional arguments regardless of
+  the decorator order. If you upgrade from a version before pyfakefs 4.2,
+  you may have to adapt the argument order.
 
 You can also use this to make a single unit test use the fake fs:
 
@@ -218,7 +227,7 @@ the decorator:
   from pyfakefs.fake_filesystem_unittest import patchfs
 
   @patchfs(allow_root_user=False)
-  def test_something(fs):
+  def test_something(fake_fs):
       ...
 
 
@@ -399,7 +408,7 @@ has now been been integrated into pyfakefs):
 
   # test code using patchfs decorator
   @patchfs(modules_to_patch={'django.core.files.locks': FakeLocks})
-  def test_django_stuff(fs):
+  def test_django_stuff(fake_fs):
       ...
 
 additional_skip_names

pyfakefs/fake_filesystem_unittest.py

@@ -87,8 +87,8 @@ def patchfs(_func=None, *,
     Usage::
 
         @patchfs
-        def test_my_function(fs):
-            fs.create_file('foo')
+        def test_my_function(fake_fs):
+            fake_fs.create_file('foo')
 
         @patchfs(allow_root_user=False)
         def test_with_patcher_args(fs):
@@ -105,17 +105,20 @@ def wrapped(*args, **kwargs):
                     allow_root_user=allow_root_user,
                     use_known_patches=use_known_patches,
                     patch_open_code=patch_open_code) as p:
-                kwargs['fs'] = p.fs
+                args = list(args)
+                args.append(p.fs)
                 return f(*args, **kwargs)
 
         return wrapped
 
     if _func:
         if not callable(_func):
             raise TypeError(
-                "Decorator argument not a function.\n"
+                "Decorator argument is not a function.\n"
                 "Did you mean `@patchfs(additional_skip_names=...)`?"
             )
+        if hasattr(_func, 'patchings'):
+            _func.nr_patches = len(_func.patchings)
         return wrap_patchfs(_func)
 
     return wrap_patchfs

pyfakefs/tests/fake_filesystem_unittest_test.py

@@ -29,7 +29,7 @@
 import warnings
 from distutils.dir_util import copy_tree, remove_tree
 from pathlib import Path
-from unittest import TestCase
+from unittest import TestCase, mock
 
 import pyfakefs.tests.import_as_example
 from pyfakefs import fake_filesystem_unittest, fake_filesystem
@@ -50,13 +50,35 @@ def test_context_manager(self):
             self.assertEqual('test', contents)
 
     @patchfs
-    def test_context_decorator(self, fs):
-        fs.create_file('/foo/bar', contents='test')
+    def test_context_decorator(self, fake_fs):
+        fake_fs.create_file('/foo/bar', contents='test')
         with open('/foo/bar') as f:
             contents = f.read()
         self.assertEqual('test', contents)
 
 
+class TestPatchfsArgumentOrder(TestCase):
+    @patchfs
+    @mock.patch('os.system')
+    def test_argument_order1(self, fake_fs, patched_system):
+        fake_fs.create_file('/foo/bar', contents='test')
+        with open('/foo/bar') as f:
+            contents = f.read()
+        self.assertEqual('test', contents)
+        os.system("foo")
+        patched_system.assert_called_with("foo")
+
+    @mock.patch('os.system')
+    @patchfs
+    def test_argument_order2(self, patched_system, fake_fs):
+        fake_fs.create_file('/foo/bar', contents='test')
+        with open('/foo/bar') as f:
+            contents = f.read()
+        self.assertEqual('test', contents)
+        os.system("foo")
+        patched_system.assert_called_with("foo")
+
+
 class TestPyfakefsUnittestBase(fake_filesystem_unittest.TestCase):
     def setUp(self):
         """Set up the fake file system"""
@@ -468,17 +490,17 @@ class PatchModuleTestUsingDecorator(unittest.TestCase):
 
     @patchfs
     @unittest.expectedFailure
-    def test_system_stat_failing(self, fs):
+    def test_system_stat_failing(self, fake_fs):
         file_path = '/foo/bar'
-        fs.create_file(file_path, contents=b'test')
+        fake_fs.create_file(file_path, contents=b'test')
         self.assertEqual(
             4, pyfakefs.tests.import_as_example.system_stat(file_path).st_size)
 
     @patchfs(modules_to_patch={
         'pyfakefs.tests.import_as_example': FakeExampleModule})
-    def test_system_stat(self, fs):
+    def test_system_stat(self, fake_fs):
         file_path = '/foo/bar'
-        fs.create_file(file_path, contents=b'test')
+        fake_fs.create_file(file_path, contents=b'test')
         self.assertEqual(
             4, pyfakefs.tests.import_as_example.system_stat(file_path).st_size)