Are You Using Mocks When You Should Be Using Test Data Builders?
I want to show what happens when you use mock objects when you should be using test data builders.
This post is a follow-up to a question that came up during a presentation that Jason and I gave at the March XP Toronto meeting.
We want to test the addMember() method of the CareTeamMembershipService. This method makes a Patient a member of a CareTeam.
As you can see from the implementation below there are two business rules to enforce: (1) patients can only be added to care teams at facilities where the patient is registered and (2) patients have to meet the care team membership criteria.
public void addMember(Patient patient, CareTeam careTeam) {
if (!patient.isRegisteredAt(careTeam.getFacility())) {
throw new CareTeamAdminException();
}
if (!patient.meets(careTeam.getMembershipCriteria())) {
throw new CareTeamAdminException();
}
careTeamMembershipDao.create(patient.getId(), careTeam.getId());
}
Here is the test for the addMember() method using mock objects and test data builders:
public void should_permit_add_for_appropriate_care_team() {
final Facility jacobi = aFacility().build();
final Patient patient = aPatient().at(jacobi).age(18).with(DIABETES).build();
final CareTeam careTeam = anAdultCareTeam().at(jacobi).with(DIABETES).build();
context.checking(new Expectations() {{
one(careTeamMembershipDao).create(patient.getId(), careTeam.getId());
}});
sut.addMember(patient, careTeam);
}
Now that I’m quite used to jMock and test data builder syntax, I find that this test reads like a specification.
During our presentation there was one very interesting question from the audience that was never answered directly: Could you use a Mock instead of a Test Data Builder for the Facility instance?
How about we try right now and see what happens?
Use a mock for the jacobi Facility instance.
public void should_permit_add_for_appropriate_care_team() {
final Facility jacobi = context.mock(Facility.class);
final Patient patient = aPatient().at(jacobi).age(18).with(DIABETES).build();
final CareTeam careTeam = anAdultCareTeam().at(jacobi).with(DIABETES).build();
context.checking(new Expectations() {{
one(careTeamMembershipDao).create(patient.getId(), careTeam.getId());
}});
sut.addMember(patient, careTeam);
}
The test fails because Facility is not an interface.
One solution is to create interfaces for all your domain objects. Yech. Alternatively the jMock framework will let you mock concrete and abstract classes although it is discouraged. To enable this feature you have to configure the context in a special way.
Use a ClassImposteriser mockery.
final Mockery context = new JUnit4Mockery() {{
setImposteriser(ClassImposteriser.INSTANCE);
}};
Now the test fails because of an unexpected invocation: facility.register().
Add expectations on the facility mock object.
public void should_permit_add_for_appropriate_care_team() throws Exception {
final Facility jacobi = context.mock(Facility.class);
context.checking(new Expectations() {{
one(jacobi).register(with(any(Patient.class)));
one(jacobi).register(with(any(CareTeam.class)));
}});
final Patient patient = aPatient().at(jacobi).age(18).with(DIABETES).build();
final CareTeam careTeam = anAdultCareTeam().at(jacobi).with(DIABETES).build();
context.checking(new Expectations() {{
one(careTeamMembershipDao).create(patient.getId(), careTeam.getId());
}});
sut.addMember(patient, careTeam);
}
This fix is not so simple because we have to put both of the register expectations before we’ve actually created our patient and careTeam. We also have to specify the expected arguments with matchers rather than the actual arguments.
Now the test fails because of an unexpected invocation: facility.getPatients().
Add another expectation on the facility mock object.
public void should_permit_add_for_appropriate_care_team() throws Exception {
final Facility jacobi = context.mock(Facility.class);
context.checking(new Expectations() {{
one(jacobi).register(with(any(Patient.class)));
one(jacobi).register(with(any(CareTeam.class)));
}});
final Patient patient = aPatient().at(jacobi).age(18).with(DIABETES).build();
final CareTeam careTeam = anAdultCareTeam().at(jacobi).with(DIABETES).build();
context.checking(new Expectations() {{
one(jacobi).getPatients(); will(returnValue(singleton(patient)));
one(careTeamMembershipDao).create(patient.getId(), careTeam.getId());
}});
sut.addMember(patient, careTeam);
}
Now the test finally passes. However, I find it way less readable as a specification. Why does the specification for the addMember() function require expectations on the Facility instance?
public void should_permit_add_for_appropriate_care_team() throws Exception {
final Facility jacobi = context.mock(Facility.class);
final Patient patient = context.mock(Patient.class);
final CareTeam careTeam = context.mock(CareTeam.class);<
final MembershipCriteria membershipCriteria = context.mock(
MembershipCriteria.class);
context.checking(new Expectations() {{
one(patient).getId(); will(returnValue("id1"));
one(careTeam).getId(); will(returnValue("id2"));
one(careTeam).getFacility(); will(returnValue(jacobi));
one(patient).isRegisteredAt(jacobi); will(returnValue(true));
one(careTeam).getMembershipCriteria(); will(returnValue(membershipCriteria));
one(patient).meets(membershipCriteria); will(returnValue(true));
one(careTeamMembershipDao).create("id1", "id2");
}});
sut.addMember(patient, careTeam);
}
This is what my tests looked like when I used mock objects without test data builders.
I find it really hard to separate the expectations that have to do with the system under test from the expectations that have to do with the data setup. I also have to know lots of details about the internals of the depended-on components: patient, careTeam, membershipCriteria, and facility.
public void should_permit_add_for_appropriate_care_team() {
final Facility jacobi = aFacility().build();
final Patient patient = aPatient().at(jacobi).age(18).with(DIABETES).build();
final CareTeam careTeam = anAdultCareTeam().at(jacobi).with(DIABETES).build();
context.checking(new Expectations() {{
one(careTeamMembershipDao).create(patient.getId(), careTeam.getId());
}});
sut.addMember(patient, careTeam);
}
Sweet.
Hi, folks,
First, I want to say that overall I like your ideas very much, thanks for the excellent presentation.
I’ve been using the Builder pattern for my production code in certain cases. Now I see that it’s quite applicable for the tests.
Now a few thoughts I have on the matter.
1) Having interfaces for your domain objects is not necessary a bad idea. Well, even if you don’t like it, you can mock the classes quite easily now, as you mentioned.
One situation when you may have to do that is when you use objects from some 3d party library in your code. It’s not always convenient or even impossible to create builders for them.
2) Consider overriding equals instead of defining a matcher. May bring additional benefits.
3) One bit I don’t completely agree with: if we mock everything, we “also have to know lots of details about the internals of the depended-on components: patient, careTeam, membershipCriteria, and facility”
I think it’s actually the opposite. We have to know the internals of the data objects to write and instantiate the appropriate data builders.
For example, to test the case when a patient doesn’t meet the membership criteria we have to know that – internally in the membership criteria – his age is gonna be compared with some constant. Whereas if we mock the class, we just expect the meets method to return false not caring about the exact reason.
It is a bit of a disadvantage sometimes, imho. However, I agree that it’s mitigated by the encapsulation of the most part of the logic in the builders.
Thanks,
Andrey.
Maybe I don’t know what a test data builder is, so I might have this wrong, but it seems like you set up a false dichotomy here: use a test data builder or set an expectation on Facility.
What about stubs?
Reading your implementation, you could stub only these two methods:
(JMock 1 syntax, because that is how I roll.)
mockPatient.stubs().method(“isRegisteredAt”).withAnyArguments().will(returnValue(true));
mockPatient.stubs().method(“meets”).withAnyArguments().will(returnValue(true));
Now your test is straightforward: two stubs, the expectation on careTeamMembershipDao.create(). Badda bing.
I admit, the test data builder syntax is cool; however, your test data has some details that look irrelevant for the test:
final Patient patient = aPatient().at(jacobi).age(18).with(DIABETES).build();
final CareTeam careTeam = anAdultCareTeam().at(jacobi).with(DIABETES).build();
What significance do 18 years of age, diabetes, and adult care team have here? Do I need to know these things in order to write this test? If so, then I don’t think that’s a good thing.
You appear to register the patient with a care team in order for isRegistered() to return true at the appropriate time. Why do I need to know what isRegistered() means for this test? Why should I care? Don’t I have enough to think about?
Finally, to Andrey Shulinsky, I have to disagree strongly. If you want interfaces for your domain objects–specifically when they operate as Value Objects–then it’s a sign that something somewhere is very, very wrong. A Value Object is something you create and throw away as needed. If it has behavior complex enough to want to simulate, then it almost certainly has too many responsibilities.
Hey, J.B.,
I assume that by value object you mean an object with a State but no Behavior, right? Please correct me if you meant something else.
First of all, Domain Object != Value Object. It is perfectly appropriate for a Domain object like a patient in the example have some logic in it – like the meets method. One can have a different design, of course, but this is quite valid. And then having an interface (or interfaces) may be useful.
Having Value Objects, objects with no responsibilities, by the way, might be a sign of a flawed design. They were “invented” to pass the data between apps or remote parts of an app and their usage in different situation is questionable.
But even the pure Value Objects may benefit from interfaces. For instance, one may want to expose just the getters to ensure immutability.
@Audrey: a Value Object is one that represents an unchanging value, in contrast to a thing that changes over time. They do not refer to “dumb structs” that are just used for data transfer.
It’s reasonable for a Value Object to have behaviour associated with it, but that behaviour cannot change the value, only transform it into another value (a function), or affect a stateful object that is passed to it (a callback).
Value Objects are immutable because they represent unchanging values. Code that uses them will rely on that immutability. Interfaces do not guarantee immutability, so you should not make objects refer to values by interface, only by their concrete type, otherwise you run the risk of all sorts of aliasing problems. In a large codebase they are very difficult to track down.
As for builders exposing internal structure of the objects being built, it is the responsibility of the builder to hide that information. The calls to the builder should reflect what the tests need to express and translate that into the construction of domain objects. They need not exactly reflect the structure of domain objects (although if they don’t then the tests and domain model may be drifting apart, which is something to look into).
Nat,
name’s Andrey ;-)
On the matter – there are too many definitions, I guess. :-) I thought about “Transfer Object Previously known as Value Object”: http://java.sun.com/blueprints/patterns/TransferObject.html
I see what you mean. However, if there’s some behavior, having an interface for it is often a good idea – avoiding dependencies on concrete implementations is quite helpful.
Interfaces may not guarantee immutability, yes, my statement was too strict. :-) They may help in that though. For example, sometimes you may want to use setters to create your objects, but you don’t want to expose them.
And I totally agree with the builders-related part of you post.