bpo-32092: Consume self when autospeccing unbound instance methods

Currently, when patching methods with autospec, their self
arguments are not consumed, causing call asserts to fail (they
expect an instance reference as the first argument).

Example:

```
from unittest import mock

class Something(object):
    def foo(self, a):
        pass

patcher = mock.patch.object(Something, 'foo', autospec=True)
patcher.start()

s = Something()

s.foo("foo")

s.foo.assert_called_once_with("foo")
```

The assertion above will fail, expecting an instance as a first
argument. This issue **only** occurs in this scenario, and **not**
in any of the following scenarios:
- `autospec=False`
- if the autospecced method is a `classmethod` or a `staticmethod`
- if an instance is patched instead of a class
- if `mock.create_autospec(Something)` is used instead
- if `mock.Mock(autospec=Something)` is used instead

This issue affects all patch variants.

self is now consumed by the generated wrapper and excluded from call
recording and signature checking, in both the sync and async paths.

wraps and side_effect still get `self` bound for the call if needed,
since they need to proxy through to a potentially unbound callable (e.g.:
`wraps=getattr(cls, name)`.

Adds test coverage across instance methods, classmethods,
staticmethods, sync and async, for both wraps and side_effect,
including a recursive-wraps regression test.

Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

Author: claudiubelu

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)