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

@@ -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)
 
 

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