Five unit testing tips #4: Don’t mock your way into accidental complexity

I’ve all but stopped using mock objects in my tests. The reason is that mocking have had a detrimental effect on the design of my systems. I’ve often ended up having the mocks trick me into adding a needless layer of indirection that does nothing except delegate to the next layer, just to satisfy the mocks.

For a while, I was wondering whether I was the only one with this problem, but then I saw this tutorial on JBehave, which so perfectly illustrates the problem I saw myself doing. Now, I don’t mean to pick on JBehave, or the author of this tutorial. Furthermore, this is not a particularly extreme example of the problem. Rather, the point of this posting is to show that even expert developers run into this trap.

Here is the test from the JBehave tutorial:

@Test
<span style="color: #000000; font-weight: bold;">public</span> <span style="color: #000066; font-weight: bold;">void</span> shouldProvideNotesToBePlayedInOrder<span style="color: #009900;">(</span><span style="color: #009900;">)</span> <span style="color: #009900;">{</span>
    TabParser parser <span style="color: #339933;">=</span> mock<span style="color: #009900;">(</span>TabParser.<span style="color: #000000; font-weight: bold;">class</span><span style="color: #009900;">)</span>;
    <span style="color: #003399;">String</span> asciiTab <span style="color: #339933;">=</span> <span style="color: #0000ff;">"e|--------A3-B6-"</span>;
 
    List<span style="color: #339933;"><</span>Note<span style="color: #339933;">></span> notes <span style="color: #339933;">=</span> asList<span style="color: #009900;">(</span>A<span style="color: #009900;">(</span><span style="color: #cc66cc;">3</span><span style="color: #009900;">)</span>, B<span style="color: #009900;">(</span><span style="color: #cc66cc;">6</span><span style="color: #009900;">)</span><span style="color: #009900;">)</span>;
    stub<span style="color: #009900;">(</span>parser.<span style="color: #006633;">parse</span><span style="color: #009900;">(</span>asciiTab<span style="color: #009900;">)</span><span style="color: #009900;">)</span>.<span style="color: #006633;">toReturn</span><span style="color: #009900;">(</span>notes<span style="color: #009900;">)</span>;
 
    Tab tab <span style="color: #339933;">=</span> <span style="color: #000000; font-weight: bold;">new</span> Tab<span style="color: #009900;">(</span>asciiTab, parser<span style="color: #009900;">)</span>;
 
    List<span style="color: #339933;"><</span>Note<span style="color: #339933;">></span> actualNotes <span style="color: #339933;">=</span> tab.<span style="color: #006633;">notesToBePlayed</span><span style="color: #009900;">(</span><span style="color: #009900;">)</span>;
 
    ensureThat<span style="color: #009900;">(</span>notes, equalTo<span style="color: #009900;">(</span>actualNotes<span style="color: #009900;">)</span><span style="color: #009900;">)</span>;
<span style="color: #009900;">}</span>

In other words, the tab’s notes to be played should be equal to the parsed notes.

Here’s the implementation:

<span style="color: #000000; font-weight: bold;">public</span> <span style="color: #000000; font-weight: bold;">class</span> Tab <span style="color: #009900;">{</span>
 
    <span style="color: #000000; font-weight: bold;">private</span> <span style="color: #000000; font-weight: bold;">final</span> List<span style="color: #339933;"><</span>Note<span style="color: #339933;">></span> notes;
 
    <span style="color: #000000; font-weight: bold;">public</span> Tab<span style="color: #009900;">(</span><span style="color: #003399;">String</span> asciiTab,
               TabParser parser<span style="color: #009900;">)</span> <span style="color: #009900;">{</span>
        notes <span style="color: #339933;">=</span> parser.<span style="color: #006633;">parse</span><span style="color: #009900;">(</span>asciiTab<span style="color: #009900;">)</span>;
    <span style="color: #009900;">}</span>
 
    <span style="color: #000000; font-weight: bold;">public</span> List<span style="color: #339933;"><</span>Note<span style="color: #339933;">></span> notesToBePlayed<span style="color: #009900;">(</span><span style="color: #009900;">)</span> <span style="color: #009900;">{</span>
        <span style="color: #000000; font-weight: bold;">return</span> notes;
    <span style="color: #009900;">}</span>
<span style="color: #009900;">}</span>

Whoops! We just introduced a layer in the design that does nothing. As a matter of fact, removing the Tab class from the whole example leaves us with a design that’s easier to understand.

Mocks can be great when you really, really need them. And I’m extremely impressed by the power of mockito. But used inappropriately, mocking can trick you into creating accidental complexity and extra layers of code that does nothing.