Implement dangling mock detector for unittests
The Issue: Dangling mock objects in global modules (mocked members of imported modules that never get restored) have been causing various transient failures in the unit test suite. The main issues posed by dangling mock objects include: - Such object references propagate across the entire test suite. Any caller may be hit by a non-functional or worse crippled module member because some other (potentially totally unrelated) test case failed to restore it. - Dangling mock references shared across different test modules may lead to unexpected results/behavior in multi-threaded environments. One example could be a test case failing because a mock got called multiple times from unrelated modules. Such issues are likely to exhibit transient random behavior depending on the runtime environment making them difficult to debug. This contribution is aiming to provide a simple transparent detection layer that would help us prevent (most of) such issues. Solution Strategy: We extend the 'testtools.TestCase' class currently used as the base class of Trove unit tests with functions that attempts to detect leaked mock objects in imported modules. The new 'trove_testtools.TestCase' implementation basically retrieves all loaded modules and scans their members for mock objects before and after each test case. An outline of the procedure follows: 1. Override the setUp() method and add a cleanup call (executed after the tearDown()) to collect mock references before and after a test case. 2. Subtract the two sets after each test case and mark it as failed if there are any new mock references left over. Code Impact: The existing test classes replace 'testtools.TestCase' base class with 'trove_testtools.TestCase'. Documentation Impact: Added a short document on recommended mocking strategies in unit tests to the Trove Developer Documentation. Known Limitations: The current implementation has a configurable recursion depth which is the number of nested levels to examine when searching for mocks. Higher setting will potentially uncover more dangling objects, at the cost of increased scanning time. We set it to 2 by default to get better coverage. This setting will increase test runtime. Recommended Mocking Patterns: Mock Guide: https://docs.python.org/3/library/unittest.mock.html - Mocking a class or object shared across multiple test cases. Use the patcher pattern in conjunction with the setUp() and tearDown() methods [ see section 26.4.3.5. of Mock Guide ]. def setUp(self): super(CouchbaseBackupTests, self).setUp() self.exe_timeout_patch = patch.object(utils, 'execute_with_timeout') def test_case(self): # This line can be moved to the setUp() method if the mock object # is not needed. mock_object = self.exe_timeout_patch.start() def tearDown(self): super(CouchbaseBackupTests, self).tearDown() self.exe_timeout_patch.stop() Note also: patch.stopall() Stop all active patches. Only stops patches started with start. - Mocking a class or object for a single entire test case. Use the decorator pattern. @patch.object(utils, 'execute_with_timeout') @patch.object(os, 'popen') def test_case(self, popen_mock, execute_with_timeout_mock): pass @patch.multiple(utils, execute_with_timeout=DEFAULT, generate_random_password=MagicMock(return_value=1)) def test_case(self, generate_random_password, execute_with_timeout): pass - Mocking a class or object for a smaller scope within one test case. Use the context manager pattern. def test_case(self): # Some code using real implementation of 'generate_random_password'. with patch.object(utils, 'generate_random_password') as pwd_mock: # Using the mocked implementation of 'generate_random_password'. # Again code using the actual implementation of the method. def test_case(self): with patch.multiple(utils, execute_with_timeout_mock=DEFAULT, generate_random_password=MagicMock( return_value=1)) as mocks: password_mock = mocks['generate_random_password'] execute_mock = mocks['execute_with_timeout_mock'] Change-Id: Ia487fada249aa903410a1a3fb3f717d6e0d581d2 Closes-Bug: 1447833
This commit is contained in:
parent
ad008daeba
commit
4899f49401
115
doc/source/dev/testing.rst
Normal file
115
doc/source/dev/testing.rst
Normal file
@ -0,0 +1,115 @@
|
||||
.. _testing:
|
||||
|
||||
=========================
|
||||
Notes on Trove Unit Tests
|
||||
=========================
|
||||
|
||||
Mock Object Library
|
||||
-------------------
|
||||
|
||||
Trove unit tests make a frequent use of the Python Mock library.
|
||||
This library lets the caller replace (*"mock"*) parts of the system under test with
|
||||
mock objects and make assertions about how they have been used. [1]_
|
||||
|
||||
The Problem of Dangling Mocks
|
||||
-----------------------------
|
||||
|
||||
Often one needs to mock global functions in shared system modules.
|
||||
The caller must restore the original state of the module
|
||||
after it is no longer required.
|
||||
|
||||
Dangling mock objects in global modules (mocked members of imported
|
||||
modules that never get restored) have been causing various transient
|
||||
failures in the unit test suite.
|
||||
|
||||
The main issues posed by dangling mock objects include::
|
||||
|
||||
- Such object references propagate across the entire test suite. Any
|
||||
caller may be hit by a non-functional - or worse - crippled module member
|
||||
because some other (potentially totally unrelated) test case failed to
|
||||
restore it.
|
||||
|
||||
- Dangling mock references shared across different test modules may
|
||||
lead to unexpected results/behavior in multi-threaded environments. One
|
||||
example could be a test case failing because a mock got called multiple
|
||||
times from unrelated modules.
|
||||
|
||||
Such issues are likely to exhibit transient random behavior depending
|
||||
on the runtime environment, making them difficult to debug.
|
||||
|
||||
There are several possible strategies available for dealing with dangling
|
||||
mock objects (see the section on recommended patterns).
|
||||
Further information is available in [1]_.
|
||||
|
||||
Dangling Mock Detector
|
||||
----------------------
|
||||
|
||||
All Trove unit tests should extend 'trove_testtools.TestCase'.
|
||||
It is a subclass of 'testtools.TestCase' which automatically checks for
|
||||
dangling mock objects after each test.
|
||||
It does that by recording mock instances in loaded modules before and after
|
||||
a test case. It marks the test as failed and reports the leaked reference if it
|
||||
finds any.
|
||||
|
||||
Recommended Mocking Patterns
|
||||
----------------------------
|
||||
|
||||
- Mocking a class or object shared across multiple test cases.
|
||||
Use the patcher pattern in conjunction with the setUp() and tearDown()
|
||||
methods [ see section 26.4.3.5. of [1]_ ].
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
def setUp(self):
|
||||
super(CouchbaseBackupTests, self).setUp()
|
||||
self.exe_timeout_patch = patch.object(utils, 'execute_with_timeout')
|
||||
|
||||
def test_case(self):
|
||||
# This line can be moved to the setUp() method if the mock object
|
||||
# is not needed.
|
||||
mock_object = self.exe_timeout_patch.start()
|
||||
|
||||
def tearDown(self):
|
||||
super(CouchbaseBackupTests, self).tearDown()
|
||||
self.exe_timeout_patch.stop()
|
||||
|
||||
Note also: patch.stopall()
|
||||
This method stops all active patches that were started with start.
|
||||
|
||||
- Mocking a class or object for a single entire test case.
|
||||
Use the decorator pattern.
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
@patch.object(utils, 'execute_with_timeout')
|
||||
@patch.object(os, 'popen')
|
||||
def test_case(self, popen_mock, execute_with_timeout_mock):
|
||||
pass
|
||||
|
||||
@patch.multiple(utils, execute_with_timeout=DEFAULT,
|
||||
generate_random_password=MagicMock(return_value=1))
|
||||
def test_case(self, generate_random_password, execute_with_timeout):
|
||||
pass
|
||||
|
||||
- Mocking a class or object for a smaller scope within one test case.
|
||||
Use the context manager pattern.
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
def test_case(self):
|
||||
# Some code using real implementation of 'generate_random_password'.
|
||||
with patch.object(utils, 'generate_random_password') as pwd_mock:
|
||||
# Using the mocked implementation of 'generate_random_password'.
|
||||
# Again code using the actual implementation of the method.
|
||||
|
||||
def test_case(self):
|
||||
with patch.multiple(utils, execute_with_timeout_mock=DEFAULT,
|
||||
generate_random_password=MagicMock(
|
||||
return_value=1)) as mocks:
|
||||
password_mock = mocks['generate_random_password']
|
||||
execute_mock = mocks['execute_with_timeout_mock']
|
||||
|
||||
References
|
||||
----------
|
||||
|
||||
.. [1] Mock Guide: https://docs.python.org/3/library/unittest.mock.html
|
@ -44,6 +44,7 @@ functionality, the following resources are provided.
|
||||
:maxdepth: 1
|
||||
|
||||
dev/design
|
||||
dev/testing
|
||||
dev/install
|
||||
dev/manual_install.rst
|
||||
dev/building_guest_images.rst
|
||||
|
@ -11,19 +11,17 @@
|
||||
# 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 testtools
|
||||
import mock
|
||||
from mock import ANY, DEFAULT, patch
|
||||
|
||||
from trove.guestagent.common.operating_system import FileMode
|
||||
import trove.guestagent.strategies.backup.base as backupBase
|
||||
import trove.guestagent.strategies.restore.base as restoreBase
|
||||
from trove.guestagent.strategies.restore.mysql_impl import MySQLRestoreMixin
|
||||
|
||||
from trove.guestagent.strategies.backup import mysql_impl
|
||||
from trove.common import utils
|
||||
from testtools.testcase import ExpectedException
|
||||
from trove.common import exception
|
||||
from trove.common import utils
|
||||
from trove.guestagent.common.operating_system import FileMode
|
||||
from trove.guestagent.strategies.backup import base as backupBase
|
||||
from trove.guestagent.strategies.backup import mysql_impl
|
||||
from trove.guestagent.strategies.restore import base as restoreBase
|
||||
from trove.guestagent.strategies.restore.mysql_impl import MySQLRestoreMixin
|
||||
from trove.tests.unittests import trove_testtools
|
||||
|
||||
BACKUP_XTRA_CLS = ("trove.guestagent.strategies.backup."
|
||||
"mysql_impl.InnoBackupEx")
|
||||
@ -77,7 +75,7 @@ CBBACKUP_CMD = "tar cpPf - /tmp/backups"
|
||||
CBBACKUP_RESTORE = "sudo tar xpPf -"
|
||||
|
||||
|
||||
class GuestAgentBackupTest(testtools.TestCase):
|
||||
class GuestAgentBackupTest(trove_testtools.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(GuestAgentBackupTest, self).setUp()
|
||||
@ -315,40 +313,46 @@ class GuestAgentBackupTest(testtools.TestCase):
|
||||
remove.assert_called_once_with(ANY, force=True, as_root=True)
|
||||
|
||||
|
||||
class CouchbaseBackupTests(testtools.TestCase):
|
||||
class CouchbaseBackupTests(trove_testtools.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(CouchbaseBackupTests, self).setUp()
|
||||
|
||||
self.backup_runner = utils.import_class(
|
||||
BACKUP_CBBACKUP_CLS)
|
||||
self.exec_timeout_patch = patch.object(utils, 'execute_with_timeout')
|
||||
self.exec_timeout_patch.start()
|
||||
self.backup_runner = utils.import_class(BACKUP_CBBACKUP_CLS)
|
||||
self.backup_runner_patch = patch.multiple(
|
||||
self.backup_runner, _run=DEFAULT,
|
||||
_run_pre_backup=DEFAULT, _run_post_backup=DEFAULT)
|
||||
|
||||
def tearDown(self):
|
||||
super(CouchbaseBackupTests, self).tearDown()
|
||||
self.backup_runner_patch.stop()
|
||||
self.exec_timeout_patch.stop()
|
||||
|
||||
def test_backup_success(self):
|
||||
self.backup_runner.__exit__ = mock.Mock()
|
||||
self.backup_runner._run = mock.Mock()
|
||||
self.backup_runner._run_pre_backup = mock.Mock()
|
||||
self.backup_runner._run_post_backup = mock.Mock()
|
||||
utils.execute_with_timeout = mock.Mock(return_value=None)
|
||||
backup_runner_mocks = self.backup_runner_patch.start()
|
||||
with self.backup_runner(12345):
|
||||
pass
|
||||
self.assertTrue(self.backup_runner._run)
|
||||
self.assertTrue(self.backup_runner._run_pre_backup)
|
||||
self.assertTrue(self.backup_runner._run_post_backup)
|
||||
|
||||
backup_runner_mocks['_run_pre_backup'].assert_called_once_with()
|
||||
backup_runner_mocks['_run'].assert_called_once_with()
|
||||
backup_runner_mocks['_run_post_backup'].assert_called_once_with()
|
||||
|
||||
def test_backup_failed_due_to_run_backup(self):
|
||||
self.backup_runner._run = mock.Mock(
|
||||
side_effect=exception.ProcessExecutionError('test'))
|
||||
self.backup_runner._run_pre_backup = mock.Mock()
|
||||
self.backup_runner._run_post_backup = mock.Mock()
|
||||
utils.execute_with_timeout = mock.Mock(return_value=None)
|
||||
self.assertRaises(exception.ProcessExecutionError,
|
||||
self.backup_runner(12345).__enter__)
|
||||
backup_runner_mocks = self.backup_runner_patch.start()
|
||||
backup_runner_mocks['_run'].configure_mock(
|
||||
side_effect=exception.TroveError('test')
|
||||
)
|
||||
with ExpectedException(exception.TroveError, 'test'):
|
||||
with self.backup_runner(12345):
|
||||
pass
|
||||
|
||||
backup_runner_mocks['_run_pre_backup'].assert_called_once_with()
|
||||
backup_runner_mocks['_run'].assert_called_once_with()
|
||||
self.assertEqual(0, backup_runner_mocks['_run_post_backup'].call_count)
|
||||
|
||||
|
||||
class CouchbaseRestoreTests(testtools.TestCase):
|
||||
class CouchbaseRestoreTests(trove_testtools.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(CouchbaseRestoreTests, self).setUp()
|
||||
|
@ -12,19 +12,20 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import testtools
|
||||
from mock import Mock, MagicMock
|
||||
from mock import Mock
|
||||
from mock import patch
|
||||
from trove.guestagent import service
|
||||
from trove.tests.unittests import trove_testtools
|
||||
|
||||
|
||||
class ServiceTest(testtools.TestCase):
|
||||
class ServiceTest(trove_testtools.TestCase):
|
||||
def setUp(self):
|
||||
super(ServiceTest, self).setUp()
|
||||
|
||||
def tearDown(self):
|
||||
super(ServiceTest, self).tearDown()
|
||||
|
||||
def test_app_factory(self):
|
||||
service.API._instance_router = MagicMock()
|
||||
@patch.object(service.API, '_instance_router')
|
||||
def test_app_factory(self, instance_router_mock):
|
||||
service.app_factory(Mock)
|
||||
self.assertEqual(1, service.API._instance_router.call_count)
|
||||
self.assertEqual(1, instance_router_mock.call_count)
|
||||
|
96
trove/tests/unittests/trove_testtools.py
Normal file
96
trove/tests/unittests/trove_testtools.py
Normal file
@ -0,0 +1,96 @@
|
||||
# Copyright 2015 Tesora Inc.
|
||||
# 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 inspect
|
||||
import mock
|
||||
import sys
|
||||
import testtools
|
||||
|
||||
|
||||
class TestCase(testtools.TestCase):
|
||||
"""Base class of Trove unit tests.
|
||||
Integrates automatic dangling mock detection.
|
||||
"""
|
||||
|
||||
_NEWLINE = '\n'
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
# Number of nested levels to examine when searching for mocks.
|
||||
# Higher setting will potentially uncover more dangling objects,
|
||||
# at the cost of increased scanning time.
|
||||
cls._max_recursion_depth = 2
|
||||
cls._fail_fast = False # Skip remaining tests after the first failure.
|
||||
cls._only_unique = True # Report only unique dangling mock references.
|
||||
|
||||
cls._dangling_mocks = set()
|
||||
|
||||
def setUp(self):
|
||||
if self.__class__._fail_fast and self.__class__._dangling_mocks:
|
||||
self.skipTest("This test suite already has dangling mock "
|
||||
"references from a previous test case.")
|
||||
|
||||
super(TestCase, self).setUp()
|
||||
self.addCleanup(self._assert_modules_unmocked)
|
||||
self._mocks_before = self._find_mock_refs()
|
||||
|
||||
def _assert_modules_unmocked(self):
|
||||
"""Check that all members of loaded modules are currently unmocked.
|
||||
Consider only new mocks created since the last setUp() call.
|
||||
"""
|
||||
mocks_after = self._find_mock_refs()
|
||||
new_mocks = mocks_after.difference(self._mocks_before)
|
||||
if self.__class__._only_unique:
|
||||
# Remove mock references that have already been reported once in
|
||||
# this test suite (probably defined in setUp()).
|
||||
new_mocks.difference_update(self.__class__._dangling_mocks)
|
||||
|
||||
self.__class__._dangling_mocks.update(new_mocks)
|
||||
|
||||
if new_mocks:
|
||||
messages = ["Member '%s' needs to be unmocked." % item[0]
|
||||
for item in new_mocks]
|
||||
self.fail(self._NEWLINE + self._NEWLINE.join(messages))
|
||||
|
||||
def _find_mock_refs(self):
|
||||
discovered_mocks = set()
|
||||
for module_name, module in self._get_loaded_modules().items():
|
||||
self._find_mocks(module_name, module, discovered_mocks, 1)
|
||||
|
||||
return discovered_mocks
|
||||
|
||||
def _find_mocks(self, parent_name, parent, container, depth):
|
||||
"""Search for mock members in the parent object.
|
||||
Descend into class types.
|
||||
"""
|
||||
if depth <= self.__class__._max_recursion_depth:
|
||||
try:
|
||||
if isinstance(parent, mock.Mock):
|
||||
# Add just the parent if it's a mock itself.
|
||||
container.add((parent_name, parent))
|
||||
else:
|
||||
# Add all mocked members of the parent.
|
||||
for member_name, member in inspect.getmembers(parent):
|
||||
full_name = '%s.%s' % (parent_name, member_name)
|
||||
if isinstance(member, mock.Mock):
|
||||
container.add((full_name, member))
|
||||
elif inspect.isclass(member):
|
||||
self._find_mocks(
|
||||
full_name, member, container, depth + 1)
|
||||
except ImportError:
|
||||
pass # Module cannot be imported - ignore it.
|
||||
|
||||
def _get_loaded_modules(self):
|
||||
return {name: obj for name, obj in sys.modules.items() if obj}
|
Loading…
Reference in New Issue
Block a user