From 0a617700c68c60c097229de337e8c284b8ea2ac4 Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Sat, 12 Sep 2020 00:40:41 +0000 Subject: [PATCH 1/9] bpo-41768: Use getattr_static when adding mock spec Since commit 77b3b7701a34ecf6316469e05b79bb91de2addfa, class properties are being called when adding mock specs and this can introduce side effects for tests that are verifying state affected by code inside a @property. This replaces the getattr call with a inspect.getattr_static call to avoid executing class code as part of the mock speccing process. --- Lib/unittest/mock.py | 2 +- Lib/unittest/test/testmock/testmock.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index ecf84d224fce37..be206f26bc6e36 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -494,7 +494,7 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False, _spec_asyncs = [] for attr in dir(spec): - if iscoroutinefunction(getattr(spec, attr, None)): + if iscoroutinefunction(inspect.getattr_static(spec, attr, None)): _spec_asyncs.append(attr) if spec is not None and not _is_list(spec): diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py index fdba543b53511d..2edf046d47d57d 100644 --- a/Lib/unittest/test/testmock/testmock.py +++ b/Lib/unittest/test/testmock/testmock.py @@ -38,6 +38,16 @@ def cmeth(cls, a, b, c, d=None): pass def smeth(a, b, c, d=None): pass +class SomethingElse(object): + def __init__(self): + self._instance = None + + @property + def instance(self): + if not self._instance: + self._instance = 'object' + + class Typos(): autospect = None auto_spec = None @@ -2249,6 +2259,12 @@ class Foo(): f'{__name__}.Typos', autospect=True, set_spec=True, auto_spec=True): pass + def test_property_not_called_with_spec_mock(self): + obj = SomethingElse() + self.assertIsNone(obj._instance) + mock = Mock(spec=obj) + self.assertIsNone(obj._instance) + if __name__ == '__main__': unittest.main() From 943f8d3859555ba32fb6d93fbf72bac50897e01c Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 16 Sep 2020 16:53:06 +0000 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst diff --git a/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst b/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst new file mode 100644 index 00000000000000..d2b6d12e9fdaee --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst @@ -0,0 +1 @@ +unittest.mock speccing no longer calls class properties \ No newline at end of file From 4bdc642776d00901fb33cc910552951b17f40cdc Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Fri, 18 Sep 2020 17:42:13 -0700 Subject: [PATCH 3/9] Return object from SomethingElse.instance property The goal of the property is provide access to the object and create it lazily on first access. --- Lib/unittest/test/testmock/testmock.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py index 2edf046d47d57d..6f2e9b75fcebf4 100644 --- a/Lib/unittest/test/testmock/testmock.py +++ b/Lib/unittest/test/testmock/testmock.py @@ -46,7 +46,8 @@ def __init__(self): def instance(self): if not self._instance: self._instance = 'object' - + return self._instance + class Typos(): autospect = None From 2e05daa2d9dbaec82ac161740605364a57bfe957 Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Fri, 18 Sep 2020 17:42:59 -0700 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> --- Lib/unittest/test/testmock/testmock.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py index 6f2e9b75fcebf4..b785851a30cf35 100644 --- a/Lib/unittest/test/testmock/testmock.py +++ b/Lib/unittest/test/testmock/testmock.py @@ -2262,10 +2262,10 @@ class Foo(): def test_property_not_called_with_spec_mock(self): obj = SomethingElse() - self.assertIsNone(obj._instance) + self.assertIsNone(obj._instance, msg='before mock') mock = Mock(spec=obj) - self.assertIsNone(obj._instance) - + self.assertIsNone(obj._instance, msg='after mock') + self.assertEqual(mock.instance, 'instance') if __name__ == '__main__': unittest.main() From a916d4b1743d8287f37518b602705a96e1fc99af Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Sat, 19 Sep 2020 02:23:30 +0000 Subject: [PATCH 5/9] Clean up whitespace damage and adjust assert Fix accidental whitespace damage introduced from the web UI. Slight change from the originally suggested assert as mocks do not provide the implementation of the class, only the interface of it. Instead, we can assert the return value from the actual object after accessing the property. --- Lib/unittest/test/testmock/testmock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py index b785851a30cf35..5c5ec692da83a5 100644 --- a/Lib/unittest/test/testmock/testmock.py +++ b/Lib/unittest/test/testmock/testmock.py @@ -47,7 +47,7 @@ def instance(self): if not self._instance: self._instance = 'object' return self._instance - + class Typos(): autospect = None @@ -2265,7 +2265,7 @@ def test_property_not_called_with_spec_mock(self): self.assertIsNone(obj._instance, msg='before mock') mock = Mock(spec=obj) self.assertIsNone(obj._instance, msg='after mock') - self.assertEqual(mock.instance, 'instance') + self.assertEqual('object', obj.instance) if __name__ == '__main__': unittest.main() From d953168b5fcf1ff6ce1ffe8f58f441c4320a587e Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Tue, 1 Nov 2022 01:34:45 +0000 Subject: [PATCH 6/9] Add newline at the end of NEWS entry --- .../next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst b/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst index d2b6d12e9fdaee..eb96e92d314100 100644 --- a/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst +++ b/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst @@ -1 +1 @@ -unittest.mock speccing no longer calls class properties \ No newline at end of file +unittest.mock speccing no longer calls class properties From 698856c5edb1e4d17cbbd0dcbe1ebdfb769ad005 Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Wed, 3 May 2023 17:33:47 -0700 Subject: [PATCH 7/9] Move getattr_static call into existing loop --- Lib/unittest/mock.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 8f0c16c1c29a74..2a4899cf614b80 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -514,10 +514,6 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False, _spec_signature = None _spec_asyncs = [] - for attr in dir(spec): - if iscoroutinefunction(inspect.getattr_static(spec, attr, None)): - _spec_asyncs.append(attr) - if spec is not None and not _is_list(spec): if isinstance(spec, type): _spec_class = spec @@ -530,7 +526,7 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False, spec_list = dir(spec) for attr in spec_list: - if iscoroutinefunction(getattr(spec, attr, None)): + if iscoroutinefunction(inspect.getattr_static(spec, attr, None)): _spec_asyncs.append(attr) spec = spec_list From 9baa71f6b3b6a27cfc82b6ca3471282ca1881121 Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Thu, 4 May 2023 09:43:23 -0700 Subject: [PATCH 8/9] Update Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net> --- .../next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst b/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst index eb96e92d314100..bfd3a294d44efa 100644 --- a/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst +++ b/Misc/NEWS.d/next/Library/2020-09-16-16-53-06.bpo-41768.8_fWkC.rst @@ -1 +1,2 @@ -unittest.mock speccing no longer calls class properties +:mod:`unittest.mock` speccing no longer calls class properties. +Patch by Melanie Witt. From a20b360008ec9232e3b80043a4c2951ef3125802 Mon Sep 17 00:00:00 2001 From: melanie witt <melwittt@gmail.com> Date: Mon, 15 May 2023 21:39:57 -0700 Subject: [PATCH 9/9] Handle async methods decorated with @classmethod or @staticmethod --- Lib/test/test_unittest/testmock/testmock.py | 14 ++++++++++++++ Lib/unittest/mock.py | 8 +++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_unittest/testmock/testmock.py b/Lib/test/test_unittest/testmock/testmock.py index e305ae5c5397c3..c7895e73ad9a8c 100644 --- a/Lib/test/test_unittest/testmock/testmock.py +++ b/Lib/test/test_unittest/testmock/testmock.py @@ -2311,5 +2311,19 @@ def test_property_not_called_with_spec_mock(self): self.assertIsNone(obj._instance, msg='after mock') self.assertEqual('object', obj.instance) + def test_decorated_async_methods_with_spec_mock(self): + class Foo(): + @classmethod + async def class_method(cls): + pass + @staticmethod + async def static_method(): + pass + async def method(self): + pass + mock = Mock(spec=Foo) + for m in (mock.method, mock.class_method, mock.static_method): + self.assertIsInstance(m, AsyncMock) + if __name__ == '__main__': unittest.main() diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 2a4899cf614b80..22f81e55b567f2 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -526,7 +526,13 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False, spec_list = dir(spec) for attr in spec_list: - if iscoroutinefunction(inspect.getattr_static(spec, attr, None)): + static_attr = inspect.getattr_static(spec, attr, None) + unwrapped_attr = static_attr + try: + unwrapped_attr = inspect.unwrap(unwrapped_attr) + except ValueError: + pass + if iscoroutinefunction(unwrapped_attr): _spec_asyncs.append(attr) spec = spec_list