bpo-32092: Consume self when autospeccing unbound instance methods

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``.

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):

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()

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)