Mock object are making your tests stupid

Mock objects were created because when you're writing unit tests (especially in a statically typed language), it can be a pain to create a subclass/implementations to use for your tests. You need a mock whenever the class you are testing depends on another class. You want to guarantee that changes to the other class don't break your tests for this class, because that would defeat one of the purposes of unit testing: isolation of the failure to a particular class/method. So you want an object that, whenever getTemperatureOutside is called, returns 32 degrees.

Sometimes you also want to make sure that the method you are testing causes some side effect. e.g., if the temperature outside is less than 32 degrees, it should call deployIceScraperNarwhals(). So, you want a method that records that deployIceScraperNarwhals() was called (but doesn't actually deploy the Narwhals) and something that checks that it actually was called, failing the test if it wasn't. Thus, the idea of verifying method calls was inextricably bound to the idea of providing fake methods to isolate behavior and every genius-pants writing a mock object framework decided that the verification was the most important part, to be emphasized throughout the documentation.

Then you came along, read the mock object documentation and proceed to go and write unit tests like this (mock object syntax is made up):

class UnderTest {
 int calculate(Foo foo,Bar bar) {
    int a = foo.getA()
    int b = bar.getB()
    return a + b
 }
}

class UnderTestTest {
  void testCalculate() {
     Foo foo = createMock(Foo).getA().andReturn(3)
     Bar bar = createMock(Bar).getB().andReturn(2)
     assertEquals(5,instance.calculate(foo,bar))
  }
}

Honestly, I'm looking for the stupid thing you did here, and I can't find it. The test passes, you didn't have to write MockFoo and MockBar classes, and all is right with the world. Until you decide to make an optimization:

class UnderTest {
 int calculate(Foo foo,Bar bar) {
    if(foo.getA() == bar.getB()) {
      return foo.getA() << 1 // a+b = a*2 = a left shifted 1 bit     
    }     
    int a = foo.getA()     
    int b = bar.getB()
     return a + b
 }
}

Ignoring the fact that this probably hurts performance instead of helping, lets rerun the test and see what we get (error message made up because I'm lazy): "Unexpected call to getA." That's right, you only told it to expect one call to getA, not two. You didn't change the result, you didn't remove an important side effect, in short, you did nothing that should break a test. Everything is still working perfectly. But it broke the test.

This is the part where I get to tell you you're an idiot. Instead of realizing that what you really want is a stub object, you decided that you must have written the test wrong so you rewrite it to expect between 1 and 2 calls to getA(). Because these mock object things can be tricky, you proceed to write all of your unit tests from then on by examining the source of the method you are testing, creating a mock for each dependency, adding a method call for each method call in the source, and basically rewriting the method in mock form. You idiot. You just wrote a test that confirms that the method was coded a particular way, not that the method behaves correctly.

Here's an example:

class OtherUnderTest {
 /**
  * @return a Map with a result entry, which is a message to the user.
  */
 Map deployNarhwals(Foo foo,Bar bar,Narwhals narwhals) {
    if( underTest.calculate(foo,bar) < 32 ) {
       narwhals.deploy()
       return [result: narwhals.getStatus()+' narwhals deployed']
    }
    return [result:'ok']
 }
}

class OtherUnderTestTest {
  void testDeployNarwhals() {
     Foo foo = createMock(Foo).getA().andReturn(3)
     Bar bar = createMock(Bar).getB().andReturn(2)
     Narwhals narwhals = createMock(Narwhals).getStatus().andReturn('Deadly')
     narwhals.deploy()
     Map result = instance.deployNarwhals(foo,bar,narwhals)
     assertEquals([result:'Deadly narwhals deployed'],result)
  }
}

You did so many things wrong here I'm going to have to make a list:

• You mocked the parameters, but didn't stub the internal dependency. See that instance of UnderTest in there? You're not testing it's calculate method, you're testing deployNarwhals, so you need to replace it with an instance that returns something like 31 so that you'll always be testing the condition (which should be a part of your API documentation) "if the temperature outside is less than 32 degrees, the narwhals will be deployed." Stupid.


• You verify that the side effect occurs, but you also couple your test to your implementation.What if I decide that the user doesn't need to know the Narwhal's status, I change the wording of the message, or make an extra call to getStatus()? Broken test. But the test shouldn't break just because your marketing department decided that 'released' was a more green term than 'deployed', or you cleaned up your code and change the number or order of non-side-effect-causing method calls.


• You check for equality on a MapDo you really care if there are extra keys in the Map? Probably not. Instead, check that all the key value pairs you expect are there.

This is how you should have written your test:

class OtherUnderTestTest {
  void testDeployNarwhals() {
     boolean deployed = false
     Narwhals narwhals = [
        getStatus: { return "Deadly" },
        deployNarwhals: {
          deployed = true
        }
     ] as Narwhals
     instance.setUnderTest([calculate:{ return 31 }] as UnderTest)
     Map result = instance.deployNarwhals(new Foo(),new Bar(),narwhals)
     assertTrue(deployed)
     assertNotNull(result.result)
  }
}

You could use a mock object framework for the Narwhals object, but you should configure it to only require the call to deployNarwhals, and not to fail when other methods are called. The above test will not fail unless the API contract is broken. I only assert that the expected side effect occurred, and that a non-null message was returned.

The problem is that you morons seem to think that more assertions == better test. That's simply not true. A good test covers as many cases as possible, but only asserts what is defined in the API contract. If you start asserting things that are not part of the API, like getA() is called, you make the test brittle. Even worse, someone could come along and see your assertion and think that getA() being called is part of the API and code accordingly! Now you've introduced a bug somewhere else in the code that your tests wont catch and could become a complete mess if it isn't caught quickly. The reality is you should assert only what you have to to catch a bug. Sometimes tests don't even need assertions, or it's not worth the effort to write them. It's enough that they show that the code under test ran without error, which is no small matter. Knowing that retrieveNarwhals(null,null,null) doesn't throw an exception can be very important.

Now, get back to work and try to stop writing brain dead tests. You thick ninny.

EDIT: Here's a good alternative: Mockito.