Merge "Adds mock autospec fixture"queens-em3.2.0

5 files changed, 391 insertions, 0 deletions

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.