pythongh-119600: mock: do not access attributes of original when new_…

…callable is set (python#119601) In order to patch flask.g e.g. as in python#84982, that proxies getattr must not be invoked. For that, mock must not try to read from the original object. In some cases that is unavoidable, e.g. when doing autospec. However, patch("flask.g", new_callable=MagicMock) should be entirely safe.
miss-islington · Jun 11, 2024 · 422c4fc · 422c4fc
1 parent 6efe346
commit 422c4fc
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 5 deletions.
diff --git a/Lib/test/test_unittest/testmock/support.py b/Lib/test/test_unittest/testmock/support.py
@@ -14,3 +14,14 @@ def wibble(self): pass
 
 class X(object):
     pass
+
+# A standin for weurkzeug.local.LocalProxy - issue 119600
+def _inaccessible(*args, **kwargs):
+    raise AttributeError
+
+
+class OpaqueProxy:
+    __getattribute__ = _inaccessible
+
+
+g = OpaqueProxy()
diff --git a/Lib/test/test_unittest/testmock/testpatch.py b/Lib/test/test_unittest/testmock/testpatch.py
@@ -2045,6 +2045,13 @@ def test(): pass
         with self.assertRaises(TypeError):
             test()
 
+    def test_patch_proxy_object(self):
+        @patch("test.test_unittest.testmock.support.g", new_callable=MagicMock())
+        def test(_):
+            pass
+
+        test()
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
@@ -1508,13 +1508,12 @@ def __enter__(self):
                 if isinstance(original, type):
                     # If we're patching out a class and there is a spec
                     inherit = True
-            if spec is None and _is_async_obj(original):
-                Klass = AsyncMock
-            else:
-                Klass = MagicMock
-            _kwargs = {}
+
+            # Determine the Klass to use
             if new_callable is not None:
                 Klass = new_callable
+            elif spec is None and _is_async_obj(original):
+                Klass = AsyncMock
             elif spec is not None or spec_set is not None:
                 this_spec = spec
                 if spec_set is not None:
@@ -1527,7 +1526,12 @@ def __enter__(self):
                     Klass = AsyncMock
                 elif not_callable:
                     Klass = NonCallableMagicMock
+                else:
+                    Klass = MagicMock
+            else:
+                Klass = MagicMock
 
+            _kwargs = {}
             if spec is not None:
                 _kwargs['spec'] = spec
             if spec_set is not None:

diff --git a/Misc/NEWS.d/next/Library/2024-06-10-14-00-40.gh-issue-119600.jJMf4C.rst b/Misc/NEWS.d/next/Library/2024-06-10-14-00-40.gh-issue-119600.jJMf4C.rst
@@ -0,0 +1,2 @@
+Fix :func:`unittest.mock.patch` to not read attributes of the target when
+``new_callable`` is set. Patch by Robert Collins.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Fix :func:`unittest.mock.patch` to not read attributes of the target when
		``new_callable`` is set. Patch by Robert Collins.