Don’t mock value objects: too generic rule without explanation

Question

Here is a quote from Mockito unit testing framework:

Don't mock value objects

Why one would even want to do that?

Because instantiating the object is too painful !? => not a valid reason. If it's too difficult to create new fixtures, it is a sign the code may need some serious refactoring. An alternative is to create builders for your value objects -- there are tools for that, including IDE plugins, Lombok, and others. One can also create meaningful factory methods in the test classpath.

And another quote from here:

There isn't much point in writing mocks for simple value objects (which should be immutable anyway), just create an instance and use it. It's not worth creating an interface / implementation pair to control which time values are returned, just create instances with the appropriate values and use them. There are a couple of heuristics for when a class is not worth mocking. First, it has only accessors or simple methods that act on values it holds, it doesn't have any interesting behaviour. Second, you can't think of a meaningful name for the class other than VideoImpl or some such vague term.

This seems to be a valid reason for dumb value objects holding just the values and nothing more, but things getting more complicated when you have a ValueObject referencing the entities and other value objects.

Let's say I have Person and Pet objects which are entities and Relationship (owner, doctor, etc) which is a ValueObject between two persons and has a RelationshipType which is also a Value Object. So, relationship is basically:

class Relationship {
    private Person person;
    private Pet pet;
    private RelationshipType type;
}

Now, let's say I have a class with the predicate like isOwnerRelationship, isDoctorRelationship, whatever. Basically predicate is as simple as

relationship -> relationship.isOwner(); //delegates to relationshipType.isOwner()

Now, I want to test the predicates and I have two options:

Mock Relationship

public void testIsOwner() {
   Relationship rel = mock(Relationship.class);
   when(rel.isOwner()).thenReturn(true);

   assertTrue(RelationshipPredicates.isOwner(rel));
}

Don't mock Relationship

public void testIsOwner() {
   Person person = PersonBuilder.newPerson();
   Pet pet = PetBuilder.newDogPet();
   RelationshipType type = RelationshipTypes.ownerType();

   Relationship rel = new Relationship(person, pet, type);

   assertTrue(RelationshipPredicates.isOwner(rel));
}

Of course the example is over simplified because for a person you may be required to provide address, for Pet you may have to provide BreedType, whatever, i.e. transitive graph of entities and value objects you may need to provide can be very huge. Of course you can mock Entities, but assume you have more ValueObjects of ValueObjects inside of Relationship. Even if you have fancy builders, you will have to provide each and every part of the original ValueObject even though unit test is going to test only single aspect of it.

In the predicates test, why should I care about full object construction if the predicate cares about calling one particular method of the object or combination of them?

Or is such value object can't be considered as a simple and rule doesn't apply?

Answer 1

UnitTests should test individual units. So, if your ValueObject is simple enough then it should not influence the testing of SUT (subject under test). If, however, the ValueObject has complex behavior then you should mock it. This simplifies the test and isolate testing to only the SUT.

Answer 2

I know this answer comes a bit late but, in my opinion you should try to keep things simpler. Use the "real thing" (use the constructor) when the object creation could not bring side effects to the test and use a mock/stub when, for example, you only need a certain return value from a method. It does not really matter if it's a value object or not.

For example a value object may use a random number generator to give a value to one of its properties on construction. This could have side effects on your test (because of entropy, which in some scenarios could not be sufficient to generate that number) so it would be best to use a stub/mock instead.

Or if you are a perfectionist and want to overengineer your solution, you could have a plain value object and move the construction to a factory class and the random number generator to a interface/infrastructure class (IRandomNumberGenerator in your domain layer and RandomNumberGenerator in your infrastructure layer which will require integration test/stress test to see how good your randomness sources are). In this scenario you should use the real thing in your tests, construct the real value object as the side effect has been moved to other classes.

Apply the KISS (keep it simple stupid) rule. Mock to avoid side effects in your tests and to write just one line of code (when you just need a certain return from a method), otherwise use the real thing (which generally is much simpler than to stub so many methods in more complex behaviors).

Just use whatever makes your code shorter, simpler, easier to follow but always remember to watch out for objects that might bring side effects.

Answer 3

In the predicates test, why should I care about full object construction if the predicate cares about calling one particular method of the object or combination of them?

If the predicate only cares about calling one particular method, then why are you passing an entire value to it?

In test driven design, one of the key ideas is that writing the test is feedback about the API that you are creating; if you are finding that API to be clumsy to test, it's a hint that the API may also be clumsy to use.

In this specific case, the test is trying to tell you that your current design violates the interface segregation principle

No client should be forced to depend on methods it does not use.

All the predicate cares about is a description of ownership, so maybe that idea should be expressed explicitly in your solution

interface DescribesOwnership {
    boolean isOwner();
}

class Relationship implements DescribesOwnership {
    @Override
    boolean isOwner() {
        return this.type.isOwner();
    }
}

That's one possible answer. Another is that the code is trying to tell you that the API for constructing a Relationship needs some work. You're heading that direction with your Builder proposal, but again... listen to the test.

It's trying to tell you that you want:

Relationship rel = Relationship.builder()
                   .using(RelationshipTypes.ownerType())
                   .build();

In other words, this test doesn't care at all what values are used for Owner or Pet; it doesn't even care that those things exist. Maybe that's going to be true elsewhere as well.

Notice that you still get the clean test that you have in your mock example

public void testIsOwner() {
    Relationship rel = Relationship.builder()
                   .using(RelationshipTypes.ownerType())
                   .build();

    assertTrue(RelationshipPredicates.isOwner(rel));
}

Don't like the Builder idiom? that's fine; just notice that all we are really doing is creating a mapping between an instance of RelationshipTypes and an instance of Relationship. In other words, you are just looking for a function.

public void testIsOwner() {
    // foo: RelationshipTypes -> Relationship
    Relationship rel = foo(RelationshipTypes.ownerType());

    assertTrue(RelationshipPredicates.isOwner(rel));
}

And you can use whichever spelling of foo is consistent with your local coding style.

In summary

Don't mock value objects

seems like really good advice -- it's a heuristic to remind you that creating a mock for a value object is solving the wrong problem.