One more threat of Mockito

Mockito is a great tool. It helps a lot. But nothing is perfect. Recently I’ve encountered the one scary thing and I want to share it.

Usually, with Mockito, the test structure looks like this:

"a thing" should {
  "do something" in new Scope {
    // setup data
    // setup mocks (returns/throws)
    thing.doSomething(input) must beCorrect
    got {
      // check calls to mocks
      noMoreCallsTo(mocks)
    }
  }
}

By default mock will return a default value (zero, null or empty collection). If the return type is Unit it won’t do anything.

Ok, where is the issue? I have a mutable structure (yes, yes, this is an issue too), some code modifies it’s state and stores to the database. I want to write a Unit test which will check, that the stored data is correct.

"manager" should {
  "change and store state" in new Scope {
    val data = new MutableContainer(...) // original state
    val dao = mock[Dao]
    val manager = new Manager(dao)
    manager.changeAndStore(data)
    got {
      dao.update(beMutableContainer(...)) // final expected state
      noMoreCallsTo(dao)
    }
  }
}

Eventually, I refactored the code inside changeAndStore method. Everything is green. By chance one of the E2E tests failed and I started to figure out how the unit test passed before.

At this point, I need to mention what I’ve actually changed. Previously the mutable class was totally mutable — all fields are mutable and contain references to mutable classes. I started to rewrite gradually from mutable structures to immutable (change it at once is too much job). So for a transition period, I have a mutable container which contains some immutable data. Simplified version is:

case class MutableContainer(var list: Seq[String])

object MutableMutator {
  def mutate(c: MutableContainer)(pf: PartialFunction[String, String])= {
    c.list = c.list.map(s => if (pf.isDefinedAt(s)) pf(s) else s)
  }
}

The MutableContainer class contain a mutable field with an immutable content. And the MutableMutator performs a mutation of this field with a new immutable object, PartialFunction is used to do it.

In the changeAndStore method I made a stupid mistake:

MutableMutator.mutate(c) {
  case s@"a" =>
    val newValue = s + "b"
    dao.update(c)
    newValue
}

As you can see, here I performed a call to a dao before the actual mutation in MutableContainer was performed. At the end, the MutableContainer will contain all necessary changes but it won’t be stored in a database. And I cannot catch with the default Mockito setup.

In other words, the problem is I’m checking expectations on mock after mutation was performed. On the call to mock Mockito just stores a reference to all arguments passed to the method, because an argument is a mutable class, it could be changed after a call and you will match this argument with another state.

Ok, what to do?

Besides avoiding of mutable data structures…

By its nature, dao don’t return anything (it throws an exception on errors) and we don’t need to rely on its return value (so it’s Unit). Which means that Mockito framework doesn’t force us to specify an expectation for a method before a call.

We may turn Mockito to JMock :)

To do this, we need to make extra setup to our mock:

val dao = mock[ContainerDao]
  .defaultAnswer(i => throw new Ex(s"Unexpected call: $i"))
doAnswer(_ => {}).when(dao).update(...) // final expected state

The first expression creates a mock which will throw an exception by default. The second expression specifies a concrete expectation. Now, an expectation will be checked at the same time when the method is calling. If an expectation matched not successfully, the defaultAnswer will be used and will throw an exception.

Conclusion

I don’t know enough about JMock to make a decision to switch from Mockito to JMock. But at least, it’s worth to think about use throwing default answer with Mockito. And don’t use mutable data structures :)

Originally posted on Medium.