Repository: jmcgeheeiv/pyfakefs
Pull request #665 (2022-02-03 19:33:21)

Open binary no encoding
On open() binary mode should not allow an `encoding` argument. See #664.

If encoding is passed with binary mode, raise: `ValueError(binary mode doesn't take an encoding argument)`

Add unit test helper to check the open() constructor call. Test for both binary and text mode with encoding, where binary fails and text passes.

yaythomas: When I did my local due diligence to pre-check that the CI checks will all pass, I got a mypy error:

```
pyfakefs/fake_filesystem_unittest.py:750: error: Module has no attribute "_HAS_FCOPYFILE"  [attr-defined]
Found 1 error in 1 file (checked 23 source files)
```

I'm pretty sure this is not related to the changeset under consideration.

It _might_ have something to do with issue #542.


Patch:
diff --git a/CHANGES.md b/CHANGES.md
index ed1d9c8..e59b9c0 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -7,7 +7,9 @@ The released versions correspond to PyPi releases.
 * correctly handle file system space for files opened in write mode
   (see [#660](../../issues/660))
 * correctly handle reading/writing pipes via file
-  (see [#661](../../issues/661)) 
+  (see [#661](../../issues/661))
+* disallow `encoding` argument on binary `open()`
+  (see [#664](../../issues/664))
 
 ## [Version 4.5.4](https://pypi.python.org/pypi/pyfakefs/4.5.4) (2022-01-12)
 Minor bugfix release.
diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py
index 2e2d129..29bd1ba 100644
--- a/pyfakefs/fake_filesystem.py
+++ b/pyfakefs/fake_filesystem.py
@@ -5571,6 +5571,10 @@ class FakeFileOpen:
             ValueError: for an invalid mode or mode combination
         """
         binary = 'b' in mode
+
+        if binary and encoding:
+            raise ValueError("binary mode doesn't take an encoding argument")
+
         newline, open_modes = self._handle_file_mode(mode, newline, open_modes)
 
         file_object, file_path, filedes, real_path = self._handle_file_arg(


Test patch:
diff --git a/pyfakefs/tests/fake_filesystem_vs_real_test.py b/pyfakefs/tests/fake_filesystem_vs_real_test.py
index cd55006..3be1076 100644
--- a/pyfakefs/tests/fake_filesystem_vs_real_test.py
+++ b/pyfakefs/tests/fake_filesystem_vs_real_test.py
@@ -399,6 +399,50 @@ class FakeFilesystemVsRealTest(TestCase):
             self.fail('Behaviors do not match for %s:\n    %s' %
                       (path, '\n    '.join(differences)))
 
+    def assertFileHandleOpenBehaviorsMatch(self, *args, **kwargs):
+        """Compare open() function invocation between real and fake.
+
+        Runs open(*args, **kwargs) on both real and fake.
+
+        Args:
+            *args: args to pass through to open()
+            **kwargs: kwargs to pass through to open().
+
+        Returns:
+            None.
+
+        Raises:
+            AssertionError if underlying open() behavior differs from fake.
+        """
+        real_err = None
+        fake_err = None
+        try:
+            with open(*args, **kwargs):
+                pass
+        except Exception as e:  # pylint: disable-msg=W0703
+            real_err = e
+
+        try:
+            with self.fake_open(*args, **kwargs):
+                pass
+        except Exception as e:  # pylint: disable-msg=W0703
+            fake_err = e
+
+        # default equal in case one is None and other is not.
+        is_exception_equal = (real_err == fake_err)
+        if real_err and fake_err:
+            # exception __eq__ doesn't evaluate equal ever, thus manual check.
+            is_exception_equal = (type(real_err) is type(fake_err) and
+                                  real_err.args == fake_err.args)
+
+        if not is_exception_equal:
+            msg = (
+                "Behaviors don't match on open with args %s & kwargs %s.\n" %
+                (args, kwargs))
+            real_err_msg = 'Real open results in: %s\n' % repr(real_err)
+            fake_err_msg = 'Fake open results in: %s\n' % repr(fake_err)
+            self.fail(msg + real_err_msg + fake_err_msg)
+
     # Helpers for checks which are not straight method calls.
     @staticmethod
     def _access_real(path):
@@ -635,6 +679,18 @@ class FakeFilesystemVsRealTest(TestCase):
         self.assertFileHandleBehaviorsMatch('write', 'wb', 'other contents')
         self.assertFileHandleBehaviorsMatch('append', 'ab', 'other contents')
 
+        # binary cannot have encoding
+        self.assertFileHandleOpenBehaviorsMatch('read', 'rb', encoding='enc')
+        self.assertFileHandleOpenBehaviorsMatch(
+            'write', mode='wb', encoding='enc')
+        self.assertFileHandleOpenBehaviorsMatch('append', 'ab', encoding='enc')
+
+        # text can have encoding
+        self.assertFileHandleOpenBehaviorsMatch('read', 'r', encoding='utf-8')
+        self.assertFileHandleOpenBehaviorsMatch('write', 'w', encoding='utf-8')
+        self.assertFileHandleOpenBehaviorsMatch(
+            'append', 'a', encoding='utf-8')
+
 
 def main(_):
     unittest.main()