From c84677a1a84a5f28a6d47681fc238a5c470bb676 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Wed, 7 Jun 2017 04:34:52 -0700 Subject: [PATCH] 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. --- Lib/test/test_unittest/testmock/testasync.py | 2 +- Lib/test/test_unittest/testmock/testmock.py | 48 +++++++++++++++++++ Lib/unittest/mock.py | 39 ++++++++++++--- .../2017-11-20-06-36-00.bpo-30587.1SsAy9.rst | 3 ++ 4 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-11-20-06-36-00.bpo-30587.1SsAy9.rst diff --git a/Lib/test/test_unittest/testmock/testasync.py b/Lib/test/test_unittest/testmock/testasync.py index dc36ceeb6502e8..54d1ca53f36617 100644 --- a/Lib/test/test_unittest/testmock/testasync.py +++ b/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) diff --git a/Lib/test/test_unittest/testmock/testmock.py b/Lib/test/test_unittest/testmock/testmock.py index 764585ec5d5468..25b769f377aa81 100644 --- a/Lib/test/test_unittest/testmock/testmock.py +++ b/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): diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 1cee67fa5d7094..18f85ec874b184 100644 --- a/Lib/unittest/mock.py +++ b/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 diff --git a/Misc/NEWS.d/next/Library/2017-11-20-06-36-00.bpo-30587.1SsAy9.rst b/Misc/NEWS.d/next/Library/2017-11-20-06-36-00.bpo-30587.1SsAy9.rst new file mode 100644 index 00000000000000..496f26b9a8cd97 --- /dev/null +++ b/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.