assert_called_once: Threat or Menace
Tyler R., Software Engineer
- Feb 4, 2015
I remember the first time I laid eyes on the beast: Summer, 2012. The air conditioning at Yelp HQ hummed imperceptibly as I reviewed code for a colleague. This wasn’t my first rodeo, but I was new to Yelp and to working in a large Python codebase. I painstakingly scanned the code for lurking bugs, but couldn’t find any. “Ship it!” I declared electronically, freeing my colleague to deploy his changes.
It is chilling to think that on that day, I looked the beast squarely in the eyes and never realized it. Cunning camouflage allowed it to slip past me and into production.
Hunt the Wumpus
So what is this horrific beast? I will show you, and you still might not spot it! Imagine you are that starry-eyed engineer from the Summer of ‘12 and you’re reviewing production code that looks something like this:
This feature even comes with a unit test:
The main idea is that restarting actual servers whenever someone runs the test suite is not a great strategy. So instead of calling the real
restart_server(), we use
mock.patch() from the Python Mock library to swap in a test double. The fake
restart_server(), which we call
mock_restart, is a
MagicMock which remembers how it has been called and provides convenience methods like
assert_called_once_with to validate its experiences.
The code review includes output showing that this test passes. You can paste the code above into a file and run it with py.test if you don’t trust me:
Did you spot the beast?
It is Pitch Black
Here’s a clue: the test will happily continue to pass if you change the assertion to:
What’s going on here?
Remember that a mock’s job is to say, “You got it, boss” whenever anyone calls it. It will do real work, like raising an exception, when one of its convenience methods is called, like
assert_called_once_with. But it won’t do real work when you call a method that only resembles a convenience method, such as
You Are Likely to be Eaten by a Grue
More troubling, the test passes even if you remove the call to
restart_server() from the code-under-test entirely!
This should make your blood run cold. Our test looks reasonable, emits a green “1 test passed,” but doesn’t test anything!
Production code with no test coverage is pretty bad, but this test actually has negative value. Someone spent energy writing it. More engineers will spend energy reading it. Future maintainers, not realizing that they are bedding down with the beast, will add features based on this test and these new features will also be untested. Unwary newcomers will see the useful-looking but ultimately poisonous
assert_called_once() and spread it to other modules.
Kill The Beast
Now that we know the beast and how it ticks, how can we defeat it?
Fear of Commitment
Fellow Yelpers Cheng C. and Nicholas N. added a check to our pre-commit hooks that uses the compiler module to walk the AST looking for
assert_called_once and a few other disguises commonly used by the beast. If the beast’s signature is detected, the commit is rejected.
This band-aid was cheap to implement and provides a final, automated chance to slay the beast before it can sneak into the master branch. Blacklists are a crude instrument, however, so this hook is not a perfect defense.
You Can Go Your Own Way
After my colleague Keith M.’s first encounter with the beast (pointed out to him by battle-hardened Autumn 2014 me in a code review), he vowed to avoid the
Mock convenience methods entirely:
I’ve switched to an idiom of checking mock objects that should be a lot more resilient to the fact that they are pernicious liars.
So instead of:
assert mock_restart.call_count == 1
assert mock_restart.call_args == mock.call(“web1-sfo”)
The helper method saves a line, but calling things on mocks and expecting them to have side effects (like raising an error) is just asking to get bit by typos and misspellings. Meanwhile, it’s much harder for a mock to accidentally end up with very specific values occupying a random attribute.
Glasses for Your Subaru?
The authors of Mock are aware of the beast and have built a defense into the library – autospec:
If you use a class or instance as the spec for a mock then you can only access attributes on the mock that exist on the real class.
If we change our test to use the autospec defense:
Then the beast is exposed!
autospec is very effective… if you remember to use it. It’s not on by default due to “ caveats and limitations”, so the beast relies on the uninitiated and forgetful to avoid being trapped by
The best defense against the beast is not a blacklist or a workaround or a clever library feature. It is a change in how you write software.
When I see a test containing a bogus method like
assert_called_once, it tells me that the author never verified that the test could fail. A test that cannot fail is no test at all.
The easiest way to insure that your test can fail is to write the test first, watch it fail, then write production code until the test passes. This is Test-Driven Development (TDD) and in addition to being a relatively foolproof means of defeating the beast, it is the best way I know to write well-tested, well-factored, maintainable software.
TDD is what allowed me to outsmart the beast the first time. While adding a feature, I had copied from an existing, beast-infiltrated test. But because I had written the test first, I knew something was amiss when the test suite cheerfully passed. Suspicion and
ipdb soon laid bare the beast’s tricks, and now I pass my experiences on to you.
So go, my friends, into the great wilderness of software. May your test suite remain faithful, honest, and beast-free.
 You can see this hook at https://gist.github.com/mrtyler/995fcb4282a9d15de625.