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?

Haag

It seems to me the biggest problem here is the way the domain is being modeled. Specifically, that Relationship class looks like a very bad idea. In an OO domain model, relationships between entities are normally represented through associations between their classes, not with classes of their own. So, for example, a Pet would have one or more owners, while a Person would own zero or more pets, ie, a "many-to-many" association between Person and Pet - no extra class needed. – Sabina

@Rogério this is just an example. Btw, many-to-many in database is implemented through dedicated table with foreign keys to both entities. The modelling in the code may be different depending on domain. – Haag

As long as the subject under test is clear I do not think other concrete dependencies (except services doing IO) are that problematic given that they are also unit tested on their own if they are very complex. Would you mock a Date, that's quite a complex value object no? Well it can also be trusted because you know it's been tested and should work fine. If you mock too many things then you may have to end up testing your tests because your mocks will become complex as well. Where does it stop? Just make sure complex dependencies are unit tested on their own and avoid mocks when you can. – Loganiaceous

As long a Relationship is unit tested as well as Person and Pet then taking the dependency hit when testing RelationshipPredicates shouldn't be that problematic. If your RelationshipPredicates fail because your Relationship object is not behaving it wouldn't be hard to identify because the Relationship tests would also be failing. If you have a predicate such as isOwnerOfPetNamedRex then your test could start with rel = ownershipOfPetNamedRexByRandomPerson(). – Loganiaceous

The rel creation details aren't important at all for the test, but if you have many such relationships to create then you could use a builder... rel = new RelationshipBuilder().ownership().of(petNamedRex()).by(randomPerson()). – Loganiaceous

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.

Bors

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.

Fredela

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.

Johppah

predicate may be more complex, but it doesn't really matter, even if it calls only one method it allows to group predicates with AND/OR/NOT/etc. Introducing interface wouldn't help if I have multiple predicates against Relationship which I want to combine. I.e. and(Predicate.isOwner(relationship), Predicate.petIsDog(relationship)) //dog owners. How your builder will create Relationship? what Pet and Person it will use? Random? Your builder is easy to use, but it's also very implicit. foo() is mock() in my case :) And again, to build relationship you need all collaborators. – Haag