We talk a lot about the sort of wheels one shouldn’t reinvent. Loads of bad code stumbles down that path. Today, Mary sends us some code from their home-grown unit testing framework.
Mary doesn’t have much to say about whatever case of Not Invented Here Syndrome brought things to this point. It’s especially notable that this is Python, which comes, out of the box, with a perfectly serviceable
unittest module built in. Apparently not serviceable enough for their team, however, as Burt, the Lead Developer, wrote his own.
His was Object Oriented. Each test case received a
TestStepOutcome object as a parameter, and was expected to return that object. This meant you didn’t have to use those pesky, readable, and easily comprehensible
assert… methods. Instead, you just did your test steps and called something like:
Now, no one particularly liked the calling convention of
setPassed(False), so after some complaints, Burt added a
setFailed method. Developers started using it, and everyone’s tests passed. Everyone was happy.
At least, everyone was happy up until Mary wrote a test she expected to see fail. There was aa production bug, and she could replicate it, step by step, at the Python REPL. So that she could “catch” the bug and “prove” it was dead, she wanted a unit test.
The unit test passed.
The bug was still there, and she continued to be able to replicate it manually.
outcome.setFailed("OH FFS THIS SHOULD FAIL"), but the test passed.
outcome.setPassed(False), however… worked just like it was supposed to.
Mary checked the implementation of the
TestStepOutcome class, and found this:
class TestStepOutcome(object): def setPassed(self, flag=True): self.result = flag def setFailed(self, flag=True): self.result = flag
Yes, in Burt’s reluctance to have a
setFailed message, he just copy/pasted the
setPassed, thinking, “They basically do the same thing.” No one checked his work or reviewed the code. They all just started using
setFailed, saw their tests pass, which is what they wanted to see, and moved on about their lives.
Fixing Burt’s bug was no small task- changing the
setFailed behavior broke a few hundred unit tests, proving that every change breaks someone’s workflow.
can centralize your organization’s software applications and components to provide uniform access to developers and servers.