From 0bde35899a10875891f0592961c01e9fbb854738 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Fri, 1 Dec 2017 03:24:14 +0200 Subject: [PATCH] Adds mock autospec fixture 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 (see below). Because of this, actual issues can easily be 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. Some unit tests may pass the autospec argument, but mock doesn't support it at the moment. mock.patch, mock.patch.object, mock.patch.multiple accept an autospec argument, but because of a bug, it cannot be used properly. Adds a fixture which replaces mock.Mock and mock.MagicMock with subclass which accepts the autospec argument, and on call, it will check the signature of the called method / function. Adds a function which replaces mock.mock._patch with a subclass, which treats the autospec argument properly (consumes the self / cls argument), and sets autospec=True by default, unless otherwise specified. WARNING: this function is not a fixture, and in order to benefit from it, it will have to be called as EARLY as possible, before any test classes are loaded, otherwise the original mock.mock._patch is used instead. Needed-By: I3636833962c905faa0f144c7fdc4833037324d31 Needed-By: I4484e63c97bd1cdde3d88855eabe7545784f365e Closes-Bug: #1735588 Change-Id: I0e4a55fbf4c1d175726ca22b664e240849a99856 --- doc/source/user/index.rst | 1 + doc/source/user/mock-autospec.rst | 96 +++++++++ oslotest/mock_fixture.py | 185 ++++++++++++++++++ oslotest/tests/unit/test_mock_fixture.py | 104 ++++++++++ .../mock-autospec-fix-9042c30dbb74032f.yaml | 5 + 5 files changed, 391 insertions(+) create mode 100644 doc/source/user/mock-autospec.rst create mode 100644 oslotest/mock_fixture.py create mode 100644 oslotest/tests/unit/test_mock_fixture.py create mode 100644 releasenotes/notes/mock-autospec-fix-9042c30dbb74032f.yaml 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.