From 4e560d49d4a8d08341dae6df5e123e1be1b3c779 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Thu, 16 May 2013 08:02:21 +0000 Subject: [PATCH] Mox should cleanup before stubs Tests that define both Mox and Stubs may not receive a proper cleanup by the `TestCase` base class. The reason is that the order in which the Mox and Stubs are unset may not necessarily match-up with the order in which the tests called them. For example: Suppose we have a test that Stubs a method in its `setUp` and then Mox's that same method in the test body: Stub: Original-Func -> Stubbed-Func Mox: Stubbed-Func -> Mocked-Func Now when `TestCase` ultimately does its `cleanUp`, it will UnStub before UnMocking: UnStub: Stubbed-Func -> Original-Func UnMox: Mocked-Func -> Stubbed-Func Notice that generic cleanup has mistakingly left `Stubbed-Func` in place. Given that Stubs and Mox can be performed in any order, the upshot here is that the generic cleanup can't always be relied on. The proposed solution has two parts: a best-practice change and a small code-change. The code-change is to reverse the current order to UnMox before UnStubbing. While in theory the order is arbitrary, in practice, we have lots of Stubs in `setUp` methods already, so UnMox'ing first makes sense (and it fixes a current bug to boot!) The best-practice change is to recommend against stubbing and mocking the same function as well as to always place stubs before mocks when relying on the generic cleanup. If a developers needs to deviate from this, cleanup of stubs created after mocks are their responsibility. Fixes bug 1180671 Change-Id: I42ab8b55026c0a133625a7cc81ed8b960e9d47d7 --- nova/test.py | 2 +- nova/tests/README.rst | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/nova/test.py b/nova/test.py index cc2466ff37bc..6dad0784abf6 100644 --- a/nova/test.py +++ b/nova/test.py @@ -173,9 +173,9 @@ class MoxStubout(fixtures.Fixture): # because it screws with our generators self.mox = mox.Mox() self.stubs = stubout.StubOutForTesting() - self.addCleanup(self.mox.UnsetStubs) self.addCleanup(self.stubs.UnsetAll) self.addCleanup(self.stubs.SmartUnsetAll) + self.addCleanup(self.mox.UnsetStubs) self.addCleanup(self.mox.VerifyAll) diff --git a/nova/tests/README.rst b/nova/tests/README.rst index 76b92258a207..9dd7e7e8b308 100644 --- a/nova/tests/README.rst +++ b/nova/tests/README.rst @@ -76,3 +76,20 @@ Example:: obj = cls() self.assertRaises(test.TestingException, obj.outer_method) + + +Stubbing and Mocking +-------------------- + +Whenever possible, tests SHOULD NOT stub and mock out the same function. + +If it's unavoidable, tests SHOULD define stubs before mocks since the +`TestCase` cleanup routine will un-mock before un-stubbing. Doing otherwise +results in a test that leaks stubbed functions, causing hard-to-debug +interference between tests [1]_. + +If a mock must take place before a stub, any stubs after the mock call MUST be +manually unset using `self.cleanUp` calls within the test. + + +.. [1] https://bugs.launchpad.net/nova/+bug/1180671