Ensure that mock.patch.stopall is called last

Fix the way we register mock.patch.stopall as a cleanup so that it is
called last to ensure that we do not then later try to stop any
individual mocks that will have already been stopped.

Change-Id: I50235eb1c8fae3e690fce63b27d18cafdefd8c4c
Partial-Bug: #1365678

2 files changed, 38 insertions, 1 deletions

diff --git a/oslotest/base.py b/oslotest/base.py
index a9b3c14..213b28c 100644
--- a/oslotest/base.py
+++ b/oslotest/base.py
@@ -70,6 +70,15 @@ class BaseTestCase(testtools.TestCase):
 
     """
 
+    def __init__(self, *args, **kwds):
+        super(BaseTestCase, self).__init__(*args, **kwds)
+        # Ensure that the mock.patch.stopall cleanup is registered
+        # before any setUp() methods have a chance to register other
+        # things to be cleaned up, so it is called last. This allows
+        # tests to register their own cleanups with a mock.stop method
+        # so those mocks are not included in the stopall set.
+        self.addCleanup(mock.patch.stopall)
+
     def setUp(self):
         super(BaseTestCase, self).setUp()
         self._set_timeout()
@@ -77,7 +86,6 @@ class BaseTestCase(testtools.TestCase):
         self._fake_logs()
         self.useFixture(fixtures.NestedTempfile())
         self.useFixture(fixtures.TempHomeDir())
-        self.addCleanup(mock.patch.stopall)
 
     def _set_timeout(self):
         test_timeout = os.environ.get('OS_TEST_TIMEOUT', 0)
diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
index b918586..0816041 100644
--- a/tests/unit/test_base.py
+++ b/tests/unit/test_base.py
@@ -19,9 +19,11 @@ from six.moves import mock
 import testtools
 
 from oslotest import base
+from oslotest import mockpatch
 
 
 class TestBaseTestCase(testtools.TestCase):
+
     class FakeTestCase(base.BaseTestCase):
         def test_fake_test(self):
             pass
@@ -92,3 +94,30 @@ class TestBaseTestCase(testtools.TestCase):
 
         # check that mock patches are cleaned up
         self.assertEqual(obj.value, obj.backup)
+
+
+class TestManualMock(base.BaseTestCase):
+
+    def setUp(self):
+        # Create a cleanup to undo a patch() call *before* calling the
+        # base class version of setup().
+        patcher = mock.patch('os.environ.keys')
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        super(TestManualMock, self).setUp()
+        self.useFixture(mockpatch.Patch('fixtures.Timeout'))
+        self.unstopped = mock.patch('os.environ.put')
+
+    def tearDown(self):
+        super(TestManualMock, self).tearDown()
+        self.assertRaises(
+            RuntimeError,
+            self.unstopped.stop,
+        )
+
+    def test_mock_patch_manually(self):
+        # Verify that if a test instance creates its own mock and
+        # calls start/stop itself we don't get an error.
+        patcher = mock.patch('os.environ.get')
+        patcher.start()
+        self.addCleanup(patcher.stop)