diff --git a/doc/source/user/index.rst b/doc/source/user/index.rst index 719dfb5..99a1b40 100644 --- a/doc/source/user/index.rst +++ b/doc/source/user/index.rst @@ -8,6 +8,7 @@ Using oslotest features debugging testing + mock-autospec cross-testing resources history \ No newline at end of file diff --git a/doc/source/user/mock-autospec.rst b/doc/source/user/mock-autospec.rst new file mode 100644 index 0000000..bde081e --- /dev/null +++ b/doc/source/user/mock-autospec.rst @@ -0,0 +1,96 @@ +============= +Mock autospec +============= + +In typical unit tests, almost all of the dependencies are mocked or patched +(``mock.patch``), without any guarantee that the mocked methods actually exist, +or if their signatures are respected. Because of this, actual issues can be +easily overlooked and missed, as the unit tests are wrongfully passing. + +The ``mock.Mock`` class accepts a spec as an argument, which only solves half +the problem: it only checks if an attribute exists, based on the given spec. It +does not guarantee that the given attribute is actually a method, or if its +signature is respected. The Mock class does not accept an autospec argument [1]. + +``mock.patch``, ``mock.patch.object``, ``mock.patch.multiple`` accept an +autospec argument, but because of a bug [2], it cannot be used properly. + +oslotest offers a solution for problems mentioned above. + +1. https://github.com/testing-cabal/mock/issues/393 +2. https://github.com/testing-cabal/mock/issues/396 + + +Patching the ``mock`` module +============================ + +The ``oslotest.mock_fixture`` module contains 2 components: + +- patch_mock_module +- MockAutospecFixture + +Both components need to be used in order to fix various issues within ``mock`` +regarding autospec. + +patch_mock_module +----------------- + +At the moment, ``mock.patch``, ``mock.patch.object``, ``mock.patch.multiple`` +accepts the ``autospec`` argument, but it does not correctly consume the +self / cls argument of methods or class methods. + +``patch_mock_module`` addresses this issue. In order to make sure that the +original version of ``mock.patch`` is not used by the unit tests, this +function has to be called as early as possible within the test module, or +the base test module. E.g.:: + + nova/test.py + ... + from oslotest import mock_fixture + + mock_fixture.patch_mock_module() + +Additionally, this function will set the ``autospec`` argument's value +to ``True``, unless otherwise specified or these arguments are passed in: +``new_callable, create, spec``. + +MockAutospecFixture +------------------- + +``mock.Mock`` and ``mock.MagicMock`` classes do not accept any ``autospec`` +argument. This fixture will replace the ``mock.Mock`` and ``mock.MagicMock`` +classes with subclasses which accepts the said argument. + +The fixture can be used in the test ``setUp`` method. E.g.:: + + nova/test.py + ... + from oslotest import mock_fixture + + class TestCase(testtools.TestCase): + def setUp(self): + super(TestCase, self).setUp() + self.useFixture(mock_fixture.MockAutospecFixture()) + + +Mock autospec usage +=================== + +Consider the following class as an example:: + + class Foo(object): + def bar(self, a, b, c, d=None): + pass + +Using the setup described above, the following unit tests will now pass +correctly:: + + class FooTestCase(TestCase): + def test_mock_bar(self): + mock_foo = mock.Mock(autospec=Foo) + self.assertRaises(TypeError, mock_foo.bar, invalid='argument') + + @mock.patch.object(Foo, 'bar', autospec=True) + def test_patch_bar(self, mock_bar): + foo = Foo() + self.assertRaises(TypeError, foo.bar, invalid='argument') diff --git a/oslotest/mock_fixture.py b/oslotest/mock_fixture.py new file mode 100644 index 0000000..6596d01 --- /dev/null +++ b/oslotest/mock_fixture.py @@ -0,0 +1,185 @@ + +# Copyright 2017 Cloudbase Solutions Srl +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import functools + +import fixtures +import mock + + +def _lazy_autospec_method(mocked_method, original_method, eat_self): + if mocked_method._mock_check_sig.__dict__.get('autospeced'): + return + + _lazy_autospec = mock.create_autospec(original_method) + if eat_self: + # consume self argument. + _lazy_autospec = functools.partial(_lazy_autospec, None) + + def _autospeced(*args, **kwargs): + _lazy_autospec(*args, **kwargs) + + # _mock_check_sig is called by the mock's __call__ method. + # which means that if a method is not called, _autospeced is not + # called. + _autospeced.__dict__['autospeced'] = True + mocked_method._mock_check_sig = _autospeced + + +class _AutospecMockMixin(object): + """Mock object that lazily autospecs the given spec's methods.""" + + def __init__(self, *args, **kwargs): + super(_AutospecMockMixin, self).__init__(*args, **kwargs) + self.__dict__['_autospec'] = kwargs.get('autospec') + _mock_methods = self.__dict__['_mock_methods'] + if _mock_methods: + # this will allow us to be able to set _mock_check_sig if + # the spec_set argument has been given. + _mock_methods.append('_mock_check_sig') + + def __getattr__(self, name): + attr = super(_AutospecMockMixin, self).__getattr__(name) + + original_spec = self.__dict__['_autospec'] + if not original_spec: + return attr + + if not hasattr(original_spec, name): + raise AttributeError(name) + + # check if the original attribute is callable, and the mock was not + # autospeced already. + original_attr = getattr(original_spec, name) + if callable(original_attr): + # lazily autospec callable attribute. + eat_self = mock.mock._must_skip(original_spec, name, + isinstance(original_spec, type)) + + _lazy_autospec_method(attr, original_attr, eat_self) + + return attr + + +class _AutospecMock(_AutospecMockMixin, mock.Mock): + pass + + +class _AutospecMagicMock(_AutospecMockMixin, mock.MagicMock): + pass + + +class MockAutospecFixture(fixtures.Fixture): + """A fixture to add / fix the autospec feature into the mock library. + + The current version of the mock library has a few unaddressed issues, which + can lead to erroneous unit tests, and can hide actual issues. This fixture + is to be removed once these issues have been addressed in the mock library. + + Issue addressed by the fixture: + + * mocked method's signature checking: + - https://github.com/testing-cabal/mock/issues/393 + - mock can only accept a spec object / class, and it makes sure that + that attribute exists, but it does not check whether the given + attribute is callable, or if its signature is respected in any way. + - adds autospec argument. If the autospec argument is given, the + mocked method's signature is also checked. + """ + + def setUp(self): + super(MockAutospecFixture, self).setUp() + + # patch both external and internal usage of Mock / MagicMock. + self.useFixture(fixtures.MonkeyPatch('mock.Mock', _AutospecMock)) + self.useFixture(fixtures.MonkeyPatch('mock.mock.Mock', _AutospecMock)) + self.useFixture(fixtures.MonkeyPatch('mock.mock.MagicMock', + _AutospecMagicMock)) + self.useFixture(fixtures.MonkeyPatch('mock.mock.MagicMock', + _AutospecMagicMock)) + + +class _patch(mock.mock._patch): + """Patch class with working autospec functionality. + + Currently, mock.patch functionality doesn't handle the autospec parameter + properly (the self argument is not consumed, causing assertions to fail). + Until the issue is addressed in the mock library, this should be used + instead. + https://github.com/testing-cabal/mock/issues/396 + """ + + def __init__(self, *args, **kwargs): + super(_patch, self).__init__(*args, **kwargs) + + # By default, autospec is None. We will consider it as True. + autospec = True if self.autospec is None else self.autospec + + # in some cases, autospec cannot be set to True. + skip_autospec = (getattr(self, attr) for attr in + ['new_callable', 'create', 'spec']) + # NOTE(claudiub): The "new" argument is always mock.DEFAULT, unless + # explicitly set otherwise. + if self.new is not mock.DEFAULT or any(skip_autospec): + # cannot autospec if new, new_callable, or create arguments given. + autospec = False + elif self.attribute: + target = getattr(self.getter(), self.attribute, None) + if isinstance(target, mock.Mock): + # NOTE(claudiub): shouldn't autospec already mocked targets. + # this can cause some issues. There are quite a few tests + # which patch mocked methods. + autospec = False + + # NOTE(claudiub): reset the self.autospec property, so we can handle + # the autospec scenario ourselves. + self._autospec = autospec + self.autospec = None + + def __enter__(self): + if self._autospec: + target = self.getter() + original_attr = getattr(target, self.attribute) + eat_self = mock.mock._must_skip(target, self.attribute, + isinstance(target, type)) + + new = super(_patch, self).__enter__() + _lazy_autospec_method(new, original_attr, eat_self) + return new + else: + return super(_patch, self).__enter__() + + +def _safe_attribute_error_wrapper(func): + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except AttributeError: + pass + + return wrapper + + +def patch_mock_module(): + """Replaces the mock.patch class.""" + mock.mock._patch = _patch + + # NOTE(claudiub): mock cannot autospec partial functions properly, + # especially those created by LazyLoader objects (scheduler client), + # as it will try to copy the partial function's __name__ (which they do + # not have). + mock.mock._copy_func_details = _safe_attribute_error_wrapper( + mock.mock._copy_func_details) diff --git a/oslotest/tests/unit/test_mock_fixture.py b/oslotest/tests/unit/test_mock_fixture.py new file mode 100644 index 0000000..a574e08 --- /dev/null +++ b/oslotest/tests/unit/test_mock_fixture.py @@ -0,0 +1,104 @@ +# Copyright 2017 Cloudbase Solutions Srl +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock +import testtools + +from oslotest import mock_fixture + +# NOTE(claudiub): this needs to be called before any mock.patch calls are +# being done, and especially before the test class loads. This fixes +# the mock.patch autospec issue: +# https://github.com/testing-cabal/mock/issues/396 +# TODO(claudiub): Remove this once the fix has been merged and released. +mock_fixture.patch_mock_module() + + +class Foo(object): + def bar(self, a, b, c, d=None): + pass + + @classmethod + def classic_bar(cls, a, b, c, d=None): + pass + + @staticmethod + def static_bar(a, b, c, d=None): + pass + + +class MockSanityTestCase(testtools.TestCase): + + def setUp(self): + super(MockSanityTestCase, self).setUp() + self.useFixture(mock_fixture.MockAutospecFixture()) + + def _check_autospeced_foo(self, foo): + for method_name in ['bar', 'classic_bar', 'static_bar']: + mock_method = getattr(foo, method_name) + + # check that the methods are callable with correct args. + mock_method(mock.sentinel.a, mock.sentinel.b, mock.sentinel.c) + mock_method(mock.sentinel.a, mock.sentinel.b, mock.sentinel.c, + d=mock.sentinel.d) + mock_method.assert_has_calls([ + mock.call(mock.sentinel.a, mock.sentinel.b, mock.sentinel.c), + mock.call(mock.sentinel.a, mock.sentinel.b, mock.sentinel.c, + d=mock.sentinel.d)]) + + # assert that TypeError is raised if the method signature is not + # respected. + self.assertRaises(TypeError, mock_method) + self.assertRaises(TypeError, mock_method, mock.sentinel.a) + self.assertRaises(TypeError, mock_method, a=mock.sentinel.a) + self.assertRaises(TypeError, mock_method, mock.sentinel.a, + mock.sentinel.b, mock.sentinel.c, + e=mock.sentinel.e) + + # assert that AttributeError is raised if the method does not exist. + self.assertRaises(AttributeError, getattr, foo, 'lish') + + def test_mock_autospec_all_members(self): + for spec in [Foo, Foo()]: + foo = mock.Mock(autospec=spec) + self._check_autospeced_foo(foo) + + @mock.patch.object(Foo, 'static_bar') + @mock.patch.object(Foo, 'classic_bar') + @mock.patch.object(Foo, 'bar') + def test_patch_autospec_class(self, mock_meth, mock_cmeth, mock_smeth): + foo = Foo() + self._check_autospeced_foo(foo) + + @mock.patch.object(Foo, 'static_bar', autospec=False) + @mock.patch.object(Foo, 'classic_bar', autospec=False) + @mock.patch.object(Foo, 'bar', autospec=False) + def test_patch_autospec_class_false(self, mock_meth, mock_cmeth, + mock_smeth): + foo = Foo() + # we're checking that method signature is not enforced. + foo.bar() + mock_meth.assert_called_once_with() + foo.classic_bar() + mock_cmeth.assert_called_once_with() + foo.static_bar() + mock_smeth.assert_called_once_with() + + @mock.patch.object(Foo, 'lish', create=True) + def test_patch_create(self, mock_lish): + foo = Foo() + + foo.lish() + mock_lish.assert_called_once_with() diff --git a/releasenotes/notes/mock-autospec-fix-9042c30dbb74032f.yaml b/releasenotes/notes/mock-autospec-fix-9042c30dbb74032f.yaml new file mode 100644 index 0000000..40019ab --- /dev/null +++ b/releasenotes/notes/mock-autospec-fix-9042c30dbb74032f.yaml @@ -0,0 +1,5 @@ +--- +other: + - | + Oslotest now contains the mock_fixture module, which offers fixes to the + autospec functionality in the mock library.