Fun with mocks, or: Mock-mock-unmock-unmock oops

In Python we often use the mock library to do dependency injection for tests. Recently I came across a really confounding edge case that can be easily avoided if you know about it.

Mock intro redux

(Skip this section if you're already familiar with mock.)

Let's say we have a function that calls another function, and we want to replace the callee during test runs.

>>> def foo():
...     return "I am the original foo"
... 
>>> def bar():
...     return foo()
... 
>>> print bar()
I am the original foo

If we want to replace the foo that's seen by bar during our test runs, the mock library is great for this, because it generally does a good job of cleaning up after itself. After all, if you mock out foo in one test, that shouldn't affect any other usage of foo later in the test run - they may be depending on the real foo.

You can do this via a context manager or decorator. Let's see the context manager:

>>> import mock
>>> with mock.patch('__main__.foo', return_value='mocky!'):
...     print bar()
... 
mocky!
>>> print bar()
I am the original foo

When the context manager exits, the original foo is back. Great!

Another way to use a patch object is to call its start and stop methods explicitly. For example, you could use start() in a unittest setUp method, and stop in a tearDown method. I'm not writing a unit test here, but let's see those in action:

>>> patcher = mock.patch('__main__.foo', return_value='mock is started')
>>> patcher.start()
<MagicMock name='foo' id='139968131869776'>
>>> bar()
'mock is started'
>>> patcher.stop()
>>> bar()
'I am the original foo'

If you try to call stop() a second time, you'll get a RuntimeError, which seems sensible.

Nested mocks

You can even nest calls to mock.patch and it'll work fine, even if patching the same object - even though you're mocking the same thing, these are separate mock.patch instances and they automatically get cleaned up as each context manager exits:

>>> with mock.patch('__main__.foo') as outer_mock:
...     outer_mock.return_value = 'I am the outer'
...     with mock.patch('__main__.foo') as inner_mock:
...         inner_mock.return_value = 'I am the inner'
...         print bar()
...     print bar()
... 
I am the inner
I am the outer
>>> print bar()
I am the original foo

Here's where it all goes to hell

Do NOT create a patch instance once and use it multiple times. This means:

  • Do NOT combine explicit start / stop calls with using the same patch instance as a context manager or decorator between those calls.

  • Do NOT call start multiple times on the same patch instance without a stop call in between.

  • Do NOT use the same patch instance as a nested context manager.

Why? Let's try it.

Avoid combining start() with same mock as context manager

This looks like it should work, right?

patcher = mock.patch('__main__.foo', return_value='whee')
patcher.start()
print bar()
with patcher:
    print bar()
patcher.stop()
print bar()

Here's what happens:

>>> patcher = mock.patch('__main__.foo', return_value='whee')
>>> print bar()
I am the original foo
>>> patcher.start()
<MagicMock name='foo' id='139816682462800'>
>>> print bar()
whee
>>> with patcher:   # you just broke patcher.stop()
...     print bar()
... 
whee
>>> # All is fine until we try to clean up.
>>> patcher.stop()
Traceback (most recent call last):
...
RuntimeError: stop called on unstarted patcher
>>> print bar()
whee
>>> foo
<MagicMock name='foo' id='139816682462800'>

The RuntimeError is raised as if the patch has been stopped and things have been restored to their pre-patch state, but they have not. foo is forever replaced with the mock and you can't get it back.

Is this a bug, a feature, or a simple case of Don't Do That?

For now, I'm treating it as the latter.

Avoid multiple start() calls

The same thing happens if you call start() more than once:

>>> def foo():
...     return "orig"
... 
>>> def bar():
...     return foo()
... 
>>> import mock
>>> patcher = mock.patch('__main__.foo', return_value='patched')
>>> patcher.start()
<MagicMock name='foo' id='140124223901072'>
>>> patcher.start()
<MagicMock name='foo' id='140124222729232'>
>>> bar()
'patched'
>>> patcher.stop()
>>> patcher.stop()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/mock.py", line 1404, in stop
    return self.__exit__()
  File "/usr/local/lib/python2.7/dist-packages/mock.py", line 1376, in __exit__
    raise RuntimeError('stop called on unstarted patcher')
RuntimeError: stop called on unstarted patcher
>>> bar()
'patched'

Avoid nesting the same patch instance

The same thing happens if you use the same instance as a context manager, nested:

>>> def foo():
...     return "I am the original foo"
...
>>>
>>> def bar():
...     return foo()
...
>>> patch = mock.patch('__main__.foo', return_value='patched')
>>>
>>> with patch as p1:
...     p1.return_value = 'oh boy'
...     with patch as p2:
...         p2.return_value = 'oh no'
...         print bar()
...     print bar(), "leaving now"
...
oh no
oh boy leaving now
Traceback (most recent call last):
  File "<stdin>", line 6, in <module>
  File "/Users/paul/src/handshake/.venv/lib/python2.7/site-packages/mock.py", line 1376, in __exit__
    raise RuntimeError('stop called on unstarted patcher')
RuntimeError: stop called on unstarted patcher
>>> print bar(), "got to the end"
oh boy got to the end

Avoid implicit mocks. Keep setup simple.

I found all this out the hard way, at work. The way we got into this situation is not, of course, as straightforward as the simple demonstrations above.

We had a test case base class doing the start / stop calls in setUp() and tearDown(), and a elsewhere, a testing utility function that bootstrapped some test data and -- nested somewhere down in its call stack -- it used the same patch instance as a context manager.

Neither of these things would announce the presence of mocks to any programmer who simply inherited from the base class and called the utility function.

Nothing about the API of the base class or the utility function suggests that they are unsafe to use together.

As for the patch itself, the reason the same instance was re-used multiple times was because we had a bunch of common patches defined in a module, so people could conveniently do:

from patches import patch_thing_that_hits_the_network


class MyTests(TestCase):
    def test_foo():
        with patch_thing_that_hits_the_network:
            self.assertEqual(foo(),  "expected result")

We thought this encouraged using the mocks correctly and saved people the trouble of writing out the mocks correctly, remembering to use autospec, etc. Little did we know.

And most of the time, things seem just fine - most of the code exercised by tests works happily with the mock in place. The symptom only became visible when a test was added - in a different test module - that depended on a particular failure condition of the original (unmocked) function.

This test passed when run in isolation, but failed if run after the test module that implicitly double-patched the function under test and left behind this evil immortal mock.

As you might expect, tracking down the culprit was rather time consuming. It involved running pairs of test suites to find a combination that failed, then whittling down the apparently unrelated test suite to one particular test case, then zeroing in on the line that called the utility function with the implicit mocking buried deep within. At that point we could see that the function was mocked (unexpectedly) when the test failed, and was not mocked when the test passed in isolation. That led to the discovery of this bizarre behavior.

I'm tempted to conclude that hiding use of mocks deep within setup or utility code is an antipattern. This follows from the well-known principle that test setup should be as minimal as possible. It should be easy for a test author (or reader) to understand the context in which a test runs.

Tags