From 422c4fc855afd18bcc6415902ea1d85a50cb7ce1 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Tue, 11 Jun 2024 07:41:12 +0200 Subject: gh-119600: mock: do not access attributes of original when new_callable is set (#119601) In order to patch flask.g e.g. as in #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. --- Lib/test/test_unittest/testmock/support.py | 11 +++++++++++ Lib/test/test_unittest/testmock/testpatch.py | 7 +++++++ Lib/unittest/mock.py | 14 +++++++++----- .../Library/2024-06-10-14-00-40.gh-issue-119600.jJMf4C.rst | 2 ++ 4 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-06-10-14-00-40.gh-issue-119600.jJMf4C.rst diff --git a/Lib/test/test_unittest/testmock/support.py b/Lib/test/test_unittest/testmock/support.py index 49986d6..6c535b7 100644 --- a/Lib/test/test_unittest/testmock/support.py +++ b/Lib/test/test_unittest/testmock/support.py @@ -14,3 +14,14 @@ class SomeClass(object): 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 index be75fda..f26e74c 100644 --- a/Lib/test/test_unittest/testmock/testpatch.py +++ b/Lib/test/test_unittest/testmock/testpatch.py @@ -2045,6 +2045,13 @@ class PatchTest(unittest.TestCase): 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 index 3ef83e2..edabb45 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1508,13 +1508,12 @@ class _patch(object): 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 @@ class _patch(object): 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 new file mode 100644 index 0000000..04c9ca9 --- /dev/null +++ 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. -- cgit v0.12