Why do we not mock domain objects in unit tests?

Question

I give you 2 tests; the purpose of which is solely to confirm that when service.doSomething is called, emailService.sendEmail is called with the person's email as a parameter.

@Mock 
private EmailService emailService;

@InjectMocks
private Service service;

@Captor
private ArgumentCaptor<String> stringCaptor;

@Test
public void test_that_when_doSomething_is_called_sendEmail_is_called_NO_MOCKING() {

    final String email = "[email protected]";

    // There is only one way of building an Address and it requires all these fields
    final Address crowsNest = new Address("334", "Main Street", "Gloucester", "MA", "01930", "USA");
    // There is only one way of building a Phone and it requires all these fields
    final Phone phone = new Phone("1", "978-281-2965");
    // There is only one way of building a Vessel and it requires all these fields
    final Vessel andreaGail = new Vessel("Andrea Gail", "Fishing", 92000);
    // There is only one way of building a Person and it requires all these fields
    final Person captain = new Person("Billy", "Tyne", email, crowsNest, phone, andreaGail);

    service.doSomething(captain); // <-- This requires only the person's email to be initialised, it doesn't care about anything else

    verify(emailService, times(1)).sendEmail(stringCaptor.capture());

    assertThat(stringCaptor.getValue(), eq(email));   
}

@Test
public void test_that_when_doSomething_is_called_sendEmail_is_called_WITH_MOCKING() {

    final String email = "[email protected]";

    final Person captain = mock(Person.class);
    when(captain.getEmail()).thenReturn(email);

    service.doSomething(captain); // <-- This requires the person's email to be initialised, it doesn't care about anything else

    verify(emailService, times(1)).sendEmail(stringCaptor.capture());   

    assertThat(stringCaptor.getValue(), eq(email));   
}

Why is it that my team is telling me not to mock the domain objects required to run my tests, but not part of the actual test? I am told mocks are for the dependencies of the tested service only. In my opinion, the resulting test code is leaner, cleaner and easier to understand. There is nothing to distract from the purpose of the test which is to verify the call to emailService.sendEmail occurs. This is something that I have heard and accepted as gospel for a long time, over many jobs. But I still can not agree with.

Answer 1

I think I understand your team's position.

They are probably saying that you should reserve mocks for things that have hard-to-instantiate dependencies. That includes repositories that make calls to a database, and other services that can potentially have their own rats-nest of dependencies. It doesn't include domain objects that can be instantiated (even if filling out all the constructor arguments is a pain).

If you mock the domain objects then the test doesn't give you any code coverage of them. I know I'd rather get these domain objects covered by tests of services, controllers, repositories, etc. as much as possible and minimize tests written just to exercise their getters and setters directly. That lets tests of domain objects focus on any actual business logic.

That does mean that if the domain object has an error then tests of multiple components can fail. I think that's ok. I would still have tests of the domain objects (because it's easier to test those in isolation than to make sure all paths are covered in a test of a service), but I don't want to depend entirely on the domain object tests to accurately reflect how those objects are used in the service, it seems like too much to ask.

You have a point that the mocks allow you to make the objects without filling in all their data (and I'm sure the real code can get a lot worse than what is posted). It's a trade-off, but having code coverage that includes the actual domain objects as well as the service under test seems like a bigger win to me.

It seems to me like your team has chosen to err on the side of pragmatism vs purity. If everybody else has arrived at this consensus you need to respect that. Some things are worth making waves over. This isn't one of them.

Answer 2

It is a tradeoff, and you have designed your example nicely to be 'on the edge'. Generally, mocking should be done for a reason. Good reasons are:

• You can not easily make the depended-on-component (DOC) behave as intended for your tests.
• Does calling the DOC cause any non-derministic behaviour (date/time, randomness, network connections)?
• The test setup is overly complex and/or maintenance intensive (like, need for external files) (* see below)
• The original DOC brings portability problems for your test code.
• Does using the original DOC cause unnacceptably long build / execution times?
• Has the DOC stability (maturity) issues that make the tests unreliable, or, worse, is the DOC not even available yet?

For example, you (typically) don't mock standard library math functions like sin or cos, because they don't have any of the abovementioned problems.

Why is it recommendable to avoid mocking where unnecessary?

• For one thing, mocking increases test complexity.
• Secondly, mocking makes your tests dependent on the inner workings of your code, namely, on how the code interacts with the DOCs (like, in your case, that the captain's first name is obtained using getFirstName, although possibly another way might exist to get that information).
• And, as Nathan mentioned, it may be seen as a plus that - without mocking - DOCs are tested for free - although I would be careful here: There is a risk that your tests lose focus if you get tempted to also test the DOCs. The DOCs should have tests of their own.

Why is your scenario 'on the edge'?

One of the abovementioned good reasons for mocking is marked with (*): "The test setup is overly complex ...", and your example is constructed to have a test setup that is a bit complex. Complexity of the test setup is obviously not a hard criterion and developers will simply have to make a choice. If you want to look at it this way, you could say that either way has some risks when it comes to future maintenance scenarios.

Summarized, I would say that neither position (generally to mock or generally not to mock) is right. Instead, developers should understand the decision criteria and then apply them to the specific situation. And, when the scenario is in the grey zone such that the criteria don't lead to a clear decision, don't fight over it.

Answer 3

There are two mistakes here.

First, testing that when a service method is called, it delegates to another method. That is a bad specification. A service method should be specified in terms of the values it returns (for getters) or the values that could be subsequently got (for mutators) through that service interface. The service layer should be treated as a Facade. In general, few methods should be specified in terms of which methods they delegate to and when they delegate. The delegations are implementation details and so should not be tested.

Unfortunately, the popular mocking frameworks encourage this erroneous approach. And so does over zealous use of Behaviour Driven Development.

The second mistake is centered around the very concept of unit testing. We would like each of our unit tests to test one thing, so when there is a fault in one thing, we have one test failure, and locating the fault is easy. And we tend to think of "unit" meaning the same as "method" or "class". This leads people to think that a unit test should involve only one real class, and all other classes should be mocked. This is impossible for all but the simplest of classes. Almost all Java code uses classes from the standard library, such as String or HashSet. Most professional Java code uses classes from various frameworks, such as Spring. Nobody seriously suggests mocking those. We accept that those classes are trustworthy, and so do not need mocking. We accept that it is OK not to mock "trustworthy" classes that the code of our unit uses. But, you say, our classes are not trustworthy, so we must mock them. Not so. You can trust those other classes, by having good unit tests for them. But how to avoid a tangle of interdependent classes that cause a confusing mass of test failures when there is only one fault present? That would be a nightmare to debug! Use a concept from 1970s programming (called, a virtual machine hierarchy, which is now a rather confusing term, given the additional meanings of virtual machine): arrange your software in layers from low level to high level, with higher layers performing operations using lower layers. Each layer provides a more expressive or advanced means of abstractly describing operations and objects. So, domain objects are in a low level, and the service layer is at a higher level. When several tests fail, start debugging the lowest level test failure(s): the fault will probably be in that layer, possibly (but probably not) in a lower layer, and not in a higher layer.

Reserve mocks only for input and output interfaces that would make the tests very expensive to run (typically, this means mocking the repository layer and the logging interface).