bpo-30587: Adds signature checking for mock autospec object method calls

Mock can accept an spec object / class as argument, making sure
that accessing attributes that do not exist in the spec will cause an
AttributeError to be raised, but there is no guarantee that the spec's
methods signatures are respected in any way. This creates the possibility
to have faulty code with passing unittests and assertions.

Example:

from unittest import mock

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

m = mock.Mock(spec=Something)
m.foo()

Adds the autospec argument to Mock, and its mock_add_spec method.

Passes the spec's attribute with the same name to the child mock (spec-ing
the child), if the mock's autospec is True.

Sets _mock_check_sig if the given spec is callable.

Adds unit tests to validate the fact that the autospec method signatures are
respected.

Author: claudiubelu

Lib/test/test_unittest/testmock/testasync.py

@@ -450,7 +450,7 @@ async def test_add_return_value(self):
         async def addition(self, var): pass
 
         mock = AsyncMock(addition, return_value=10)
-        output = await mock(5)
+        output = await mock(self, 5)
 
         self.assertEqual(output, 10)
 

Lib/test/test_unittest/testmock/testmock.py

@@ -668,6 +668,47 @@ def test_only_allowed_methods_exist(self):
                     getattr, mock, 'something_else'
                 )
 
+    def _check_autospeced_something(self, something):
+        for method_name in ['meth', 'cmeth', 'smeth']:
+            mock_method = getattr(something, method_name)
+
+            # 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)
+
+        # assert that AttributeError is raised if the method does not exist.
+        self.assertRaises(AttributeError, getattr, something, 'foolish')
+
+
+    def test_mock_autospec_all_members(self):
+        for spec in [Something, Something()]:
+            mock_something = Mock(autospec=spec)
+            self._check_autospeced_something(mock_something)
+
+
+    def test_mock_spec_function(self):
+        def foo(lish):
+            pass
+
+        mock_foo = Mock(spec=foo)
+
+        mock_foo(sentinel.lish)
+        mock_foo.assert_called_once_with(sentinel.lish)
+        self.assertRaises(TypeError, mock_foo)
+        self.assertRaises(TypeError, mock_foo, sentinel.foo, sentinel.lish)
+        self.assertRaises(TypeError, mock_foo, foo=sentinel.foo)
+
 
     def test_from_spec(self):
         class Something(object):
@@ -1974,6 +2015,13 @@ def test_mock_add_spec_magic_methods(self):
             self.assertRaises(TypeError, lambda: mock['foo'])
 
 
+    def test_mock_add_spec_autospec_all_members(self):
+        for spec in [Something, Something()]:
+            mock_something = Mock()
+            mock_something.mock_add_spec(spec, autospec=True)
+            self._check_autospeced_something(mock_something)
+
+
     def test_adding_child_mock(self):
         for Klass in (NonCallableMock, Mock, MagicMock, NonCallableMagicMock,
                       AsyncMock):

Lib/unittest/mock.py

@@ -137,6 +137,8 @@ def checksig(self, /, *args, **kwargs):
     type(mock)._mock_check_sig = checksig
     type(mock).__signature__ = sig
 
+    return sig
+
 
 def _copy_func_details(func, funcopy):
     # we explicitly don't copy func.__dict__ into this copy as it would
@@ -465,7 +467,8 @@ def __new__(
     def __init__(
             self, spec=None, wraps=None, name=None, spec_set=None,
             parent=None, _spec_state=None, _new_name='', _new_parent=None,
-            _spec_as_instance=False, _eat_self=None, unsafe=False, **kwargs
+            _spec_as_instance=False, _eat_self=None, unsafe=False,
+            autospec=None, **kwargs
         ):
         if _new_parent is None:
             _new_parent = parent
@@ -476,10 +479,15 @@ def __init__(
         __dict__['_mock_new_name'] = _new_name
         __dict__['_mock_new_parent'] = _new_parent
         __dict__['_mock_sealed'] = False
+        __dict__['_autospec'] = autospec
 
         if spec_set is not None:
             spec = spec_set
             spec_set = True
+        if autospec is not None:
+            # autospec is even stricter than spec_set.
+            spec = autospec
+            autospec = True
         if _eat_self is None:
             _eat_self = parent is not None
 
@@ -522,12 +530,18 @@ def attach_mock(self, mock, attribute):
         setattr(self, attribute, mock)
 
 
-    def mock_add_spec(self, spec, spec_set=False):
+    def mock_add_spec(self, spec, spec_set=False, autospec=None):
         """Add a spec to a mock. `spec` can either be an object or a
         list of strings. Only attributes on the `spec` can be fetched as
         attributes from the mock.
 
-        If `spec_set` is True then only attributes on the spec can be set."""
+        If `spec_set` is True then only attributes on the spec can be set.
+        If `autospec` is True then only attributes on the spec can be accessed
+        and set, and if a method in the `spec` is called, it's signature is
+        checked.
+        """
+        if autospec is not None:
+            self.__dict__['_autospec'] = autospec
         self._mock_add_spec(spec, spec_set)
 
 
@@ -545,9 +559,9 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False,
                 _spec_class = spec
             else:
                 _spec_class = type(spec)
-            res = _get_signature_object(spec,
-                                        _spec_as_instance, _eat_self)
-            _spec_signature = res and res[1]
+
+            _spec_signature = _check_signature(spec, self, _eat_self,
+                                               _spec_as_instance)
 
             spec_list = dir(spec)
 
@@ -714,9 +728,20 @@ def __getattr__(self, name):
                     # execution?
                     wraps = getattr(self._mock_wraps, name)
 
+                kwargs = {}
+                if self.__dict__.get('_autospec') is not None:
+                    # get the mock's spec attribute with the same name and
+                    # pass it to the child.
+                    spec_class = self.__dict__.get('_spec_class')
+                    spec = getattr(spec_class, name, None)
+                    is_type = isinstance(spec_class, type)
+                    eat_self = _must_skip(spec_class, name, is_type)
+                    kwargs['_eat_self'] = eat_self
+                    kwargs['autospec'] = spec
+
                 result = self._get_child_mock(
                     parent=self, name=name, wraps=wraps, _new_name=name,
-                    _new_parent=self
+                    _new_parent=self, **kwargs
                 )
                 self._mock_children[name]  = result
 

Misc/NEWS.d/next/Library/2017-11-20-06-36-00.bpo-30587.1SsAy9.rst

@@ -0,0 +1,3 @@
+mock: Added autospec argument to the constructors and mock_add_spec. Passing
+the autospec argument, will also check the method signatures of the mocked
+methods.