From 13f993dfb81cad9279d0037b8ab148f841ea103a Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Wed, 7 Jun 2017 04:34:52 -0700 Subject: [PATCH 1/2] 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/unittest/mock.py | 39 ++++++++++++--- Lib/unittest/test/testmock/testmock.py | 49 +++++++++++++++++++ .../2017-11-20-06-36-00.bpo-30587.1SsAy9.rst | 3 ++ 3 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-11-20-06-36-00.bpo-30587.1SsAy9.rst diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 9302dedae7fdad..76e148055057ff 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -103,6 +103,8 @@ def checksig(_mock_self, *args, **kwargs): _copy_func_details(func, checksig) type(mock)._mock_check_sig = checksig + return sig + def _copy_func_details(func, funcopy): # we explicitly don't copy func.__dict__ into this copy as it would @@ -373,7 +375,8 @@ def __new__(cls, *args, **kw): 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 @@ -384,10 +387,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 @@ -428,12 +436,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) @@ -447,9 +461,9 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False, _spec_class = spec else: _spec_class = _get_class(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 = dir(spec) @@ -594,9 +608,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/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py index b64c8663d21226..a8e3e6773277b7 100644 --- a/Lib/unittest/test/testmock/testmock.py +++ b/Lib/unittest/test/testmock/testmock.py @@ -510,6 +510,48 @@ def test_only_allowed_methods_exist(self): ) + 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): x = 3 @@ -1376,6 +1418,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: mock = Klass() 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. From 2e5cbc5e3d4ef5308b4fea487031b8e512af16f6 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 5 Feb 2023 21:59:52 +0400 Subject: [PATCH 2/2] Fix a merge error --- Lib/unittest/mock.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 31262b435c7440..cefd02c8cfd24a 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -529,7 +529,6 @@ def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False, _spec_class = type(spec) _spec_signature = _check_signature(spec, self, _eat_self, _spec_as_instance) - _spec_signature = res and res[1] spec_list = dir(spec)