diff --git a/Doc/library/unittest.mock-examples.rst b/Doc/library/unittest.mock-examples.rst index b8aa04b9ab246d1..65f6b44bddd1073 100644 --- a/Doc/library/unittest.mock-examples.rst +++ b/Doc/library/unittest.mock-examples.rst @@ -743,23 +743,19 @@ Mocking unbound methods ~~~~~~~~~~~~~~~~~~~~~~~ Sometimes a test needs to patch an *unbound method*, which means patching the -method on the class rather than on the instance. In order to make assertions -about which objects were calling this particular method, you need to pass -``self`` as the first argument. The issue is that you can't patch with a mock for -this, because if you replace an unbound method with a mock it doesn't become -a bound method when fetched from the instance, and so it doesn't get ``self`` -passed in. The workaround is to patch the unbound method with a real function -instead. The :func:`patch` decorator makes it so simple to patch out methods -with a mock that having to create a real function becomes a nuisance. +method on the class rather than on the instance. If you replace an unbound +method with a plain mock, it doesn't become a bound method when fetched from +an instance, so it never gets ``self`` passed in, and you also lose any +checking of the arguments against the original method's signature. If you pass ``autospec=True`` to patch then it does the patching with a *real* function object. This function object has the same signature as the one it is replacing, but delegates to a mock under the hood. You still get your mock auto-created in exactly the same way as before. What it means though, is that if you use it to patch out an unbound method on a class the mocked -function will be turned into a bound method if it is fetched from an instance. -It will have ``self`` passed in as the first argument, which is exactly what -was needed: +function will be turned into a bound method if it is fetched from an instance, +just like the method it replaces. ``self`` is consumed automatically, so you +don't need to account for it in call assertions: >>> class Foo: ... def foo(self): @@ -771,7 +767,7 @@ was needed: ... foo.foo() ... 'foo' - >>> mock_foo.assert_called_once_with(foo) + >>> mock_foo.assert_called_once_with() If we don't use ``autospec=True`` then the unbound method is patched out with a Mock instance instead, and isn't called with ``self``. diff --git a/Lib/test/test_unittest/testmock/testasync.py b/Lib/test/test_unittest/testmock/testasync.py index f96aef7325ff4c1..14832844468e111 100644 --- a/Lib/test/test_unittest/testmock/testasync.py +++ b/Lib/test/test_unittest/testmock/testasync.py @@ -9,9 +9,10 @@ support.requires_working_socket(module=True) from asyncio import run +from test.test_unittest.testmock.testmock import SomethingAsync from unittest import IsolatedAsyncioTestCase from unittest.mock import (ANY, call, AsyncMock, patch, MagicMock, Mock, - create_autospec, sentinel, _CallList, seal) + create_autospec, sentinel, _CallList, seal, DEFAULT) def tearDownModule(): @@ -298,6 +299,148 @@ async def test_async(): run(test_async()) + # gh-76273 / bpo-32092 + def _check_autospeced_something(self, something): + self._check_autospeced_something_method(something.meth) + self._check_autospeced_something_method(something.cmeth) + self._check_autospeced_something_method(something.smeth) + + def _check_autospeced_something_method(self, mock_method): + # check that the methods are callable with correct args. + asyncio.run(mock_method(sentinel.a, sentinel.b, sentinel.c)) + asyncio.run(mock_method(sentinel.a, sentinel.b, sentinel.c, + d=sentinel.d)) + mock_method.assert_has_calls([ + call(sentinel.a, sentinel.b, sentinel.c), + call(sentinel.a, sentinel.b, sentinel.c, d=sentinel.d)]) + + # assert that TypeError is raised if the method signature is not + # respected. + self._assert_call_raises_typeerror(mock_method) + self._assert_call_raises_typeerror(mock_method, sentinel.a) + self._assert_call_raises_typeerror(mock_method, a=sentinel.a) + self._assert_call_raises_typeerror( + mock_method, sentinel.a, sentinel.b, sentinel.c, e=sentinel.e) + + def _assert_call_raises_typeerror(self, mock_method, *args, **kwargs): + # classmethod / staticmethod autospecs check the signature + # synchronously at call time (they go through _check_signature, + # not _set_async_signature); plain instance methods only check it + # once the returned coroutine is awaited, since the check runs + # inside the coroutine body. Accept either. + try: + awaitable = mock_method(*args, **kwargs) + except TypeError: + return + + self.assertRaises(TypeError, asyncio.run, awaitable) + + def test_patch_autospec_obj(self): + something = SomethingAsync() + with patch.multiple(something, meth=DEFAULT, cmeth=DEFAULT, + smeth=DEFAULT, autospec=True): + self._check_autospeced_something(something) + + def test_patch_autospec_obj_wraps(self): + something = SomethingAsync() + with ( + patch.object(something, "meth", autospec=True, wraps=something.meth), + patch.object(something, "cmeth", autospec=True, wraps=something.cmeth), + patch.object(something, "smeth", autospec=True, wraps=something.smeth), + ): + self._check_autospeced_something(something) + + @patch.object(SomethingAsync, 'smeth', autospec=True) + @patch.object(SomethingAsync, 'cmeth', autospec=True) + @patch.object(SomethingAsync, 'meth', autospec=True) + def test_patch_autospec_class(self, mock_meth, mock_cmeth, mock_smeth): + something = SomethingAsync() + self._check_autospeced_something(something) + + @patch.object(SomethingAsync, 'smeth', autospec=True, + wraps=SomethingAsync.smeth) + @patch.object(SomethingAsync, 'cmeth', autospec=True, + wraps=SomethingAsync.cmeth) + @patch.object(SomethingAsync, 'meth', autospec=True, + wraps=SomethingAsync.meth) + def test_patch_autospec_class_wraps(self, mock_meth, mock_cmeth, + mock_smeth): + something = SomethingAsync() + self._check_autospeced_something(something) + + def test_patch_autospec_obj_side_effect(self): + for method in ["meth", "cmeth", "smeth"]: + seen = [] + def side_effect(a, b, c, d=None): + seen.append((a, b, c, d)) + + async def test_async(): + something = SomethingAsync() + with patch.object(something, method, + autospec=True) as mock_method: + mock_method.side_effect = side_effect + + await getattr(something, method)( + sentinel.a, sentinel.b, sentinel.c) + + self.assertEqual( + seen, [(sentinel.a, sentinel.b, sentinel.c, None)]) + mock_method.assert_called_once_with( + sentinel.a, sentinel.b, sentinel.c) + mock_method.assert_awaited_once_with( + sentinel.a, sentinel.b, sentinel.c) + + run(test_async()) + + def test_patch_autospec_class_side_effect(self): + for method in ["cmeth", "smeth"]: + seen = [] + async def side_effect(a, b, c, d=None): + seen.append((a, b, c, d)) + + async def test_async(): + with patch.object(SomethingAsync, method, + autospec=True) as mock_method: + mock_method.side_effect = side_effect + something = SomethingAsync() + + await getattr(something, method)( + sentinel.a, sentinel.b, sentinel.c) + + expected = (sentinel.a, sentinel.b, sentinel.c, None) + self.assertEqual(seen, [expected]) + mock_method.assert_called_once_with( + sentinel.a, sentinel.b, sentinel.c) + mock_method.assert_awaited_once_with( + sentinel.a, sentinel.b, sentinel.c) + + run(test_async()) + + # `meth` is a plain instance method patched via the class; it will + # goes through the self-consuming funcopy path: `side_effect` gets + # `self` rebound onto it, just like `wraps` does. This doesn't apply + # to classmethods. + seen = [] + async def self_side_effect(self, a, b, c, d=None): + seen.append((self, a, b, c, d)) + + async def test_async(): + with patch.object(SomethingAsync, "meth", autospec=True) as mock_method: + mock_method.side_effect = self_side_effect + something = SomethingAsync() + + await something.meth(sentinel.a, sentinel.b, sentinel.c) + + expected = (something, sentinel.a, sentinel.b, sentinel.c, None) + self.assertEqual(seen, [expected]) + mock_method.assert_called_once_with( + sentinel.a, sentinel.b, sentinel.c) + mock_method.assert_awaited_once_with( + sentinel.a, sentinel.b, sentinel.c) + + run(test_async()) + + class AsyncSpecTest(unittest.TestCase): def test_spec_normal_methods_on_class(self): diff --git a/Lib/test/test_unittest/testmock/testmock.py b/Lib/test/test_unittest/testmock/testmock.py index 764585ec5d54688..b2609422596e2ce 100644 --- a/Lib/test/test_unittest/testmock/testmock.py +++ b/Lib/test/test_unittest/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 SomethingAsync(object): + async def meth(self, a, b, c, d=None): pass + + @classmethod + async def cmeth(cls, a, b, c, d=None): pass + + @staticmethod + async def smeth(a, b, c, d=None): pass + + class SomethingElse(object): def __init__(self): self._instance = None @@ -2196,9 +2206,9 @@ def test_attach_mock_patch_autospec_signature(self): manager.attach_mock(mocked, 'attach_meth') obj = Something() obj.meth(1, 2, 3, d=4) - manager.assert_has_calls([call.attach_meth(mock.ANY, 1, 2, 3, d=4)]) - obj.meth.assert_has_calls([call(mock.ANY, 1, 2, 3, d=4)]) - mocked.assert_has_calls([call(mock.ANY, 1, 2, 3, d=4)]) + manager.assert_has_calls([call.attach_meth(1, 2, 3, d=4)]) + obj.meth.assert_has_calls([call(1, 2, 3, d=4)]) + mocked.assert_has_calls([call(1, 2, 3, d=4)]) with mock.patch(f'{__name__}.something', autospec=True) as mocked: manager = Mock() diff --git a/Lib/test/test_unittest/testmock/testpatch.py b/Lib/test/test_unittest/testmock/testpatch.py index bd85fdcfc472a61..0d92025f9d25efc 100644 --- a/Lib/test/test_unittest/testmock/testpatch.py +++ b/Lib/test/test_unittest/testmock/testpatch.py @@ -10,6 +10,7 @@ import test from test.test_unittest.testmock import support from test.test_unittest.testmock.support import SomeClass, is_instance +from test.test_unittest.testmock.testmock import Something from test.support.import_helper import DirsOnSysPath from test.test_importlib.util import uncache @@ -1075,6 +1076,155 @@ def class_method(cls, a, b=10, *, c): pass self.assertRaises(TypeError, method, 1) self.assertRaises(TypeError, method, 1, 2, 3, c=4) + # gh-76273 / bpo-32092 + def _check_autospeced_something(self, something): + self._check_autospeced_something_meth(something.meth) + self._check_autospeced_something_meth(something.cmeth) + self._check_autospeced_something_meth(something.smeth) + + + def _check_autospeced_something_meth(self, mock_method): + # check that the methods are callable with correct args. + mock_method(sentinel.a, sentinel.b, sentinel.c) + mock_method(sentinel.a, sentinel.b, sentinel.c, d=sentinel.d) + mock_method.assert_has_calls([ + call(sentinel.a, sentinel.b, sentinel.c), + call(sentinel.a, sentinel.b, sentinel.c, d=sentinel.d)]) + + # assert that TypeError is raised if the method signature is not + # respected. + self.assertRaises(TypeError, mock_method) + self.assertRaises(TypeError, mock_method, sentinel.a) + self.assertRaises(TypeError, mock_method, a=sentinel.a) + self.assertRaises(TypeError, mock_method, sentinel.a, sentinel.b, + sentinel.c, e=sentinel.e) + + + def test_patch_autospec_obj(self): + something = Something() + with patch.multiple(something, meth=DEFAULT, cmeth=DEFAULT, + smeth=DEFAULT, autospec=True): + self._check_autospeced_something(something) + + + def test_patch_autospec_obj_wraps(self): + something = Something() + with ( + patch.object(something, "meth", autospec=True, wraps=something.meth), + patch.object(something, "cmeth", autospec=True, wraps=something.cmeth), + patch.object(something, "smeth", autospec=True, wraps=something.smeth), + ): + self._check_autospeced_something(something) + + + @patch.object(Something, 'smeth', autospec=True) + @patch.object(Something, 'cmeth', autospec=True) + @patch.object(Something, 'meth', autospec=True) + def test_patch_autospec_class(self, mock_meth, mock_cmeth, mock_smeth): + # patching a single instance method with autospec should not require + # `self` to be passed to assertion methods (bpo-32092) + something = Something() + self._check_autospeced_something(something) + + + @patch.object(Something, 'smeth', autospec=True, wraps=Something.smeth) + @patch.object(Something, 'cmeth', autospec=True, wraps=Something.cmeth) + @patch.object(Something, 'meth', autospec=True, wraps=Something.meth) + def test_patch_autospec_class_wraps(self, mock_meth, mock_cmeth, + mock_smeth): + something = Something() + self._check_autospeced_something(something) + + + def test_patch_autospec_obj_side_effect(self): + for method in ["meth", "cmeth", "smeth"]: + seen = [] + def side_effect(a, b, c, d=None): + seen.append((a, b, c, d)) + + something = Something() + with patch.object(something, method, autospec=True) as mock_method: + mock_method.side_effect = side_effect + + getattr(something, method)(sentinel.a, sentinel.b, sentinel.c) + + self.assertEqual(seen, [(sentinel.a, sentinel.b, sentinel.c, None)]) + mock_method.assert_called_once_with(sentinel.a, sentinel.b, + sentinel.c) + + + def test_patch_autospec_class_side_effect(self): + for method in ["cmeth", "smeth"]: + seen = [] + def side_effect(a, b, c, d=None): + seen.append((a, b, c, d)) + + with patch.object(Something, method, autospec=True) as mock_method: + mock_method.side_effect = side_effect + something = Something() + + getattr(something, method)(sentinel.a, sentinel.b, sentinel.c) + + expected = (sentinel.a, sentinel.b, sentinel.c, None) + self.assertEqual(seen, [expected]) + mock_method.assert_called_once_with(sentinel.a, sentinel.b, + sentinel.c) + + # `meth` is a plain instance method patched via the class; it will + # goes through the self-consuming funcopy path: `side_effect` gets + # `self` rebound onto it, just like `wraps` does. This doesn't apply + # to classmethods. + seen = [] + def self_side_effect(self, a, b, c, d=None): + seen.append((self, a, b, c, d)) + + with patch.object(Something, "meth", autospec=True) as mock_method: + mock_method.side_effect = self_side_effect + something = Something() + + something.meth(sentinel.a, sentinel.b, sentinel.c) + + expected = (something, sentinel.a, sentinel.b, sentinel.c, None) + self.assertEqual(seen, [expected]) + mock_method.assert_called_once_with(sentinel.a, sentinel.b, sentinel.c) + + + def test_autospec_wraps_getattr_idiom(self): + # mirrors test_hmac.py's watch_method() helper exactly. + class Foo: + def f(self, a, b): + return ('real', a, b) + + def watch(cls, name): + wraps = getattr(cls, name) + return patch.object(cls, name, autospec=True, wraps=wraps) + + with watch(Foo, 'f') as method: + foo = Foo() + self.assertEqual(foo.f(1, 2), ('real', 1, 2)) + method.assert_called_once_with(1, 2) + + + def test_autospec_wraps_recursive_call(self): + # a wrapped method that calls itself (through the mock) should not + # corrupt call recording or the `self` rebound onto `wraps`. + class Foo: + def f(self, n): pass + + def unbound_f(self, n): + if n <= 0: + return 0 + return n + self.f(n - 1) + + with patch.object(Foo, 'f', autospec=True, wraps=unbound_f) as method: + foo = Foo() + self.assertEqual(foo.f(3), 6) + self.assertEqual(method.call_count, 4) + self.assertEqual( + method.call_args_list, + [call(3), call(2), call(1), call(0)], + ) + def test_autospec_with_new(self): patcher = patch('%s.function' % __name__, new=3, autospec=True) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 49011faaa51eaed..032a93c55fe0043 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -181,12 +181,12 @@ def _instance_callable(obj): return False -def _set_signature(mock, original, instance=False): +def _set_signature(mock, original, instance=False, eat_self=None): # creates a function with signature (*args, **kwargs) that delegates to a # mock. It still does signature checking by calling a lambda with the same # signature as the original. - skipfirst = isinstance(original, type) + skipfirst = isinstance(original, type) if eat_self is None else eat_self result = _get_signature_object(original, instance, skipfirst) if result is None: return mock @@ -198,31 +198,84 @@ def checksig(*args, **kwargs): name = original.__name__ if not name.isidentifier(): name = 'funcopy' - context = {'_checksig_': checksig, 'mock': mock} + + def _call_mock_wrapped(*args, **kwargs): + # if skipfirst, `self` is consumed here so that the recorded / checked + # call args doesn't include it (see bpo-32092). + # If set, `side_effect` and `wraps` still need `self` in order to be + # called correctly, e.g. if `wraps` is an unbound method (as in + # `wraps=getattr(cls, name)`) or if `side_effect` is meant to stand + # in for the real (bound) method. + + if skipfirst: + slf, *args = args + args = tuple(args) + + checksig(*args, **kwargs) + mock._increment_mock_call(*args, **kwargs) + + effect = mock.side_effect + wraps = mock._mock_wraps + if skipfirst: + if effect is not None and _callable(effect) and not _is_exception(effect): + effect = partial(effect, slf) + if wraps is not None: + wraps = partial(wraps, slf) + + return mock._resolve_effect(args, kwargs, effect, wraps) + + context = {'mock': mock, '_call_mock_wrapped': _call_mock_wrapped} src = """def %s(*args, **kwargs): - _checksig_(*args, **kwargs) - return mock(*args, **kwargs)""" % name + return _call_mock_wrapped(*args, **kwargs)""" % name exec (src, context) funcopy = context[name] _setup_func(funcopy, mock, sig) return funcopy -def _set_async_signature(mock, original, instance=False, is_async_mock=False): +def _set_async_signature(mock, original, instance=False, is_async_mock=False, + eat_self=None): # creates an async function with signature (*args, **kwargs) that delegates to a # mock. It still does signature checking by calling a lambda with the same # signature as the original. - skipfirst = isinstance(original, type) + skipfirst = isinstance(original, type) if eat_self is None else eat_self func, sig = _get_signature_object(original, instance, skipfirst) def checksig(*args, **kwargs): sig.bind(*args, **kwargs) _copy_func_details(func, checksig) name = original.__name__ - context = {'_checksig_': checksig, 'mock': mock} + if not name.isidentifier(): + name = 'funcopy' + + async def _call_mock_wrapped(*args, **kwargs): + # if skipfirst, `self` is consumed here so that the recorded / checked + # call args doesn't include it (see bpo-32092). + # If set, `side_effect` and `wraps` still need `self` in order to be + # called correctly, e.g. if `wraps` is an unbound method (as in + # `wraps=getattr(cls, name)`) or if `side_effect` is meant to stand + # in for the real (bound) method. + if skipfirst: + slf, *args = args + args = tuple(args) + + checksig(*args, **kwargs) + mock._increment_mock_call(*args, **kwargs) + mock._record_await(args, kwargs) + + effect = mock.side_effect + wraps = mock._mock_wraps + if skipfirst: + if effect is not None and _callable(effect) and not _is_exception(effect): + effect = partial(effect, slf) + if wraps is not None: + wraps = partial(wraps, slf) + + return await mock._resolve_async_effect(args, kwargs, effect, wraps) + + context = {'mock': mock, '_call_mock_wrapped': _call_mock_wrapped} src = """async def %s(*args, **kwargs): - _checksig_(*args, **kwargs) - return await mock(*args, **kwargs)""" % name + return await _call_mock_wrapped(*args, **kwargs)""" % name exec (src, context) funcopy = context[name] _setup_func(funcopy, mock, sig) @@ -1232,8 +1285,13 @@ def _increment_mock_call(self, /, *args, **kwargs): def _execute_mock_call(self, /, *args, **kwargs): # separate from _increment_mock_call so that awaited functions are # executed separately from their call, also AsyncMock overrides this method + return self._resolve_effect(args, kwargs, self.side_effect, self._mock_wraps) - effect = self.side_effect + def _resolve_effect(self, args, kwargs, effect, wraps): + # `effect` and `wraps` are passed in explicitly rather than read + # from self, so that autospecced methods can rebind a consumed + # `self` onto them for a single call, without mutating shared mock + # state (see bpo-32092). if effect is not None: if _is_exception(effect): raise effect @@ -1253,8 +1311,8 @@ def _execute_mock_call(self, /, *args, **kwargs): if self._mock_delegate and self._mock_delegate.return_value is not DEFAULT: return self.return_value - if self._mock_wraps is not None: - return self._mock_wraps(*args, **kwargs) + if wraps is not None: + return wraps(*args, **kwargs) return self.return_value @@ -1605,8 +1663,11 @@ def __enter__(self): f'{target_name!r} as it has already been mocked out. ' f'[target={self.target!r}, attr={autospec!r}]') + is_class = isinstance(self.target, type) + eat_self = _must_skip(self.target, self.attribute, is_class) new = create_autospec(autospec, spec_set=spec_set, - _name=self.attribute, **kwargs) + _name=self.attribute, _eat_self=eat_self, + **kwargs) elif kwargs: # can't set keyword args when we aren't creating the mock # XXXX If new is a Mock we could call new.configure_mock(**kwargs) @@ -2313,12 +2374,22 @@ async def _execute_mock_call(self, /, *args, **kwargs): # This is nearly just like super(), except for special handling # of coroutines + self._record_await(args, kwargs) + return await self._resolve_async_effect( + args, kwargs, self.side_effect, self._mock_wraps + ) + + def _record_await(self, args, kwargs): _call = _Call((args, kwargs), two=True) self.await_count += 1 self.await_args = _call self.await_args_list.append(_call) - effect = self.side_effect + async def _resolve_async_effect(self, args, kwargs, effect, wraps): + # `effect` and `wraps` are passed in explicitly rather than read + # from self, so that autospecced async methods can rebind a + # consumed `self` onto them for a single call, without mutating + # shared mock state (see bpo-32092). if effect is not None: if _is_exception(effect): raise effect @@ -2342,10 +2413,10 @@ async def _execute_mock_call(self, /, *args, **kwargs): if self._mock_return_value is not DEFAULT: return self.return_value - if self._mock_wraps is not None: - if iscoroutinefunction(self._mock_wraps): - return await self._mock_wraps(*args, **kwargs) - return self._mock_wraps(*args, **kwargs) + if wraps is not None: + if iscoroutinefunction(wraps): + return await wraps(*args, **kwargs) + return wraps(*args, **kwargs) return self.return_value @@ -2737,7 +2808,7 @@ def call_list(self): def create_autospec(spec, spec_set=False, instance=False, _parent=None, - _name=None, *, unsafe=False, **kwargs): + _name=None, *, unsafe=False, _eat_self=None, **kwargs): """Create a mock object using another object as a spec. Attributes on the mock will use the corresponding attribute on the `spec` object as their spec. @@ -2814,7 +2885,7 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None, Klass = NonCallableMagicMock mock = Klass(parent=_parent, _new_parent=_parent, _new_name=_new_name, - name=_name, **_kwargs) + name=_name, _eat_self=_eat_self, **_kwargs) if is_dataclass_spec: mock._mock_extend_spec_methods(dataclass_spec_list) @@ -2822,9 +2893,9 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None, # should only happen at the top level because we don't # recurse for functions if is_async_func: - mock = _set_async_signature(mock, spec) + mock = _set_async_signature(mock, spec, eat_self=_eat_self) else: - mock = _set_signature(mock, spec) + mock = _set_signature(mock, spec, eat_self=_eat_self) else: _check_signature(spec, mock, is_type, instance) diff --git a/Misc/NEWS.d/next/Library/2017-11-20-08-06-32.bpo-32092.fjNh4q.rst b/Misc/NEWS.d/next/Library/2017-11-20-08-06-32.bpo-32092.fjNh4q.rst new file mode 100644 index 000000000000000..adcadd2bb3c2e2e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-11-20-08-06-32.bpo-32092.fjNh4q.rst @@ -0,0 +1,2 @@ +mock: :func:`unittest.mock.patch` no longer leaking ``self`` into the recorded +call when autospeccing a plain instance method patched via its class.