The challenge: Write unit tests for the Booking class:
import java.time.LocalDateTime;
import java.util.List;
import static java.util.Arrays.asList;
public record Booking(
String name,
int age,
LocalDateTime departureDate,
List<String> itinerary) {
public List<String> validate(Validatable validator) {
return validator.validate(validations());
}
List<Validation> validations() {
return asList(
new NameRequired(this),
new AgeMinimum(this),
new FutureDate(this),
new ItinerarySize(this),
new ItineraryAirports(this));
}
}
There are several ways to think about writing tests for this small piece of code, and even more ways to implement such tests depending on which approach I take. In this article, I’ll explore some of the approaches.
Code Review
Disclaimers first: The solution is hand-grown; it might have been better built using the javax.validations framework. It’s also an overblown design if this were all the validations the solution would ever need. (Let’s imagine more validations are on their way, though.) It’s also simplistic, but that fosters simpler expositions about the code.
Each of the five listed validation objects (see the Booking method validations
above) implements the Validation interface:
public interface Validation {
boolean isInvalid();
String errorMessage();
}
Here’s a representative validation class:
import static java.time.LocalDateTime.now;
public record FutureDate(Booking booking) implements Validation {
@Override
public boolean isInvalid() {
return !booking.departureDate().isAfter(now());
}
@Override
public String errorMessage() {
return "Too late!";
}
}
Each validation class has been fully tested; here’s one example:
import org.junit.jupiter.api.Test;
import java.time.LocalDateTime;
import static java.time.temporal.ChronoUnit.SECONDS;
import static org.junit.jupiter.api.Assertions.*;
public class AFutureDate {
@Test
void isInvalidWhenDepartureDateAtOrBeforeNow() {
var booking = new Booking("", 0, LocalDateTime.now(), null);
var isInvalid = new FutureDate(booking).isInvalid();
assertTrue(isInvalid);
}
@Test
void isNotInvalidWhenDepartureDateAfterNow() {
var futureTime = LocalDateTime.now().plus(1, SECONDS);
var booking = new Booking("", 0, futureTime, null);
var isInvalid = new FutureDate(booking).isInvalid();
assertFalse(isInvalid);
}
}
Here’s the Validator class, to which a list of validation objects is passed:
import java.util.List;
public class Validator implements Validatable {
public List<String> validate(List<Validation> validations) {
return validations.stream()
.filter(Validation::isInvalid)
.map(Validation::errorMessage)
.toList();
}
}
Here’s the test for Validator:
import org.junit.jupiter.api.Test;
import java.util.Collections;
import java.util.List;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class AValidator {
Validation passingValidation = new Validation() {
@Override public boolean isInvalid() { return false; }
@Override public String errorMessage() { return ""; }
};
Validation failingValidation = new Validation() {
@Override public boolean isInvalid() { return true; }
@Override public String errorMessage() { return "fail"; }
};
@Test
void returnsEmptyListWhenAllValidationsPass() {
assertEquals(Collections.emptyList(),
new Validator().validate(List.of(passingValidation)));
}
@Test
void returnsListOfFailingValidationMessages() {
assertEquals(List.of(failingValidation.errorMessage()),
new Validator().validate(List.of(
failingValidation,
passingValidation)));
}
}
Finally, here’s the Validatable interface (which is functional, which simplifies a bit of test code):
import java.util.List;
public interface Validatable {
List<String> validate(List<Validation> validations);
}
Overview of Testing Approaches
Back to the Booking record, the question is: How to test the behavior in the validate
method, which asks a Validator instance to validate a list of validation objects associated with Booking fields?
public List<String> validate(Validator validator) {
return validator.validate(validations());
}
List<Validation> validations() {
return asList(
new NameRequired(this),
new AgeMinimum(this),
new FutureDate(this),
new ItinerarySize(this),
new ItineraryAirports(this));
}
Or maybe the question is, does it need to be tested at all?
I’ll work through the various possible approaches and implementations.
Testing Approach #1: Don’t
You might argue that there’s virtually no logic in validate
and validations
that could possibly break. The validate
method appears to be closed to change. The only change to validations
would be if a new validation were added, or an existing one deleted. Those changes are highly unlikely to break anything.
A quick counter: If I choose to not write any tests, I’ve not done my job of fully documenting the logical behaviors in the system. Granted, it’s fairly easy code to understand, but part of one goal of TDD or a good unit-testing strategy is to let the tests answer all the questions.
“How does validate
really behave?” I don’t know, because it has no tests. I must now spend extra time reading it carefully and not making any invalid assumptions.
“Unlikely to break” might be a sufficient basis to avoid testing, depending things like the level of in-context tolerance for risk. Though rare, code like this can break, and breakages usually manifest quickly.
I’m also very wary of dismissing the need for tests around “unlikely to break” code, as it can be a slippery slope. I know how to design systems well enough that most of its methods are “obvious”—I can glance at them and know almost immediately what they do and whether or not they are correctly coded. But I’ve also wasted time mentally piecing things together this way, I’ve wasted time misinterpreting such “obvious” code before, and I’ve wasted time overlooking insidious defects in such code.
A deliberate strategy of hand-waving away all the need for tests might work for you, but it might also suggest that you should give up unit testing altogether and focus on other kinds of tests. I love the fast feedback I get from them, however.
Testing Approach #2: Test All the Things
Even though the validation classes and the validator class are already tested, they are only behind-the-scenes players. The true public facing interest is that all the Booking data gets validated. As such, I might push for testing everything from the context of Booking:
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import java.time.LocalDateTime;
import java.util.List;
import static java.time.temporal.ChronoUnit.DAYS;
import static java.time.temporal.ChronoUnit.SECONDS;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
class ABooking {
Booking booking;
static final int VALID_AGE = 18;
static final LocalDateTime VALID_DEPARTURE_DATE =
LocalDateTime.now().plus(1, DAYS);
static final String INVALID_AIRPORT_CODE = "YYZ";
static final String VALID_AIRPORT_CODE1 = "COS";
static final String VALID_AIRPORT_CODE2 = "PRG";
static final List<String> VALID_ITINERARY = List.of(VALID_AIRPORT_CODE1, VALID_AIRPORT_CODE2);
static final String VALID_NAME = "Aslan";
@Nested
class ValidatesWhen {
@Test
void allFieldsAreValid() {
var booking = new Booking(VALID_NAME, VALID_AGE, VALID_DEPARTURE_DATE, VALID_ITINERARY);
var results = booking.validate();
assertTrue(results.isEmpty());
}
}
@Nested
class DoesNotValidateWhen {
@Test
void nameIsNull() {
String name = null;
var booking = new Booking(name, VALID_AGE, VALID_DEPARTURE_DATE, VALID_ITINERARY);
var results = booking.validate();
assertEquals(List.of("Name is empty"), results);
}
@Test
void nameIsEmpty() {
var name = " ";
var booking = new Booking(name, VALID_AGE, VALID_DEPARTURE_DATE, VALID_ITINERARY);
var results = booking.validate();
assertEquals(List.of("Name is empty"), results);
}
@Test
void departureDateBeforeNow() {
var pastDateTime = LocalDateTime.now().minus(1, SECONDS);
var booking = new Booking(VALID_NAME, VALID_AGE, pastDateTime, VALID_ITINERARY);
var results = booking.validate();
assertEquals(List.of("Too late!"), results);
}
@Test
void ageTooYoung() {
var minorAge = 17;
var booking = new Booking(VALID_NAME, minorAge, VALID_DEPARTURE_DATE, VALID_ITINERARY);
var results = booking.validate();
assertEquals(List.of("Minor cannot fly unaccompanied"), results);
}
@Test
void itineraryTooShort() {
var incompleteItinerary = List.of("COS");
var booking = new Booking(VALID_NAME, VALID_AGE, VALID_DEPARTURE_DATE, incompleteItinerary);
var results = booking.validate();
assertEquals(List.of("Itinerary needs 2+ airport codes"), results);
}
@Test
void itineraryContainsInvalidAirportCode() {
var invalidAirportInItinerary = List.of(VALID_AIRPORT_CODE1, INVALID_AIRPORT_CODE);
var booking = new Booking(VALID_NAME, VALID_AGE, VALID_DEPARTURE_DATE, invalidAirportInItinerary);
var results = booking.validate();
assertEquals(List.of("Itinerary contains invalid airport code"), results);
}
}
}
Even with all those tests, I now realize it misses on the notion of combining multiple failures into a single result.
The validation tests are essentially duplicated, leading to more maintenance: If I add a new validation or change one, I must update two sets of tests. If validation objects are used in multiple contexts—other classes such as Flight, Customer, and so on—the duplication across tests gets worse.
Were I to go this route, I’d probably just remove the individual validation tests… except that I like them. They’re easier to write, maintain, and understand.
A key point of the open-closed principle is that I close off small modules to change. I can also consider their tests closed. Classes reusing these smaller dependencies can trust their behaviors, and minimally shouldn’t have to explicitly re-test them all.
Testing Approach #3: Spot-Checking
If I know that the validation classes and the validator all work (because they’re directly unit-tested), the remaining thing to test is the interaction between the Booking class and these dependencies: Did the validator get invoked with the proper list of validations and did it return the results appropriately to the caller?
Validator validator = new Validator();
@Test
void validatesWhenAllFieldsAreValid() {
var validName = "Aslan";
var validAge = 18;
var validDepartureDate = now().plus(1, DAYS);
var validAirportCode1 = "COS";
var validAirportCode2 = "PRG";
var validItinerary = List.of(validAirportCode1, validAirportCode2);
var booking = new Booking(validName, validAge, validDepartureDate, validItinerary);
var results = booking.validate(validator);
assertTrue(results.isEmpty());
}
@Test
void collectsAllFailedValidations() {
var invalidAge = 1;
var invalidDepartureDate = now();
var invalidAirportCode = "YYZ";
var invalidItinerary = List.of(invalidAirportCode);
var invalidName = "";
var booking = new Booking(invalidName, invalidAge, invalidDepartureDate, invalidItinerary);
var results = booking.validate(validator);
assertEquals(booking.validations().size(), results.size());
}
The assertion in the second test compares the size of the Booking validations list to the number of validation messages received. While I might want a more stringent test, here I’m avoiding re-listing the actual list of booking classes. It’s already clearly declared in Booking.
I might even be able to justify getting away with only one test—collectsAllFailedValidations
. It creates a booking that should fail all validations, then ensures the number of failed messages matches the number of validations declared in Booking.
A small risk exists: The count of results doesn’t neccessarily mean that all the validations failed. It’s possible, perhaps, that the age validation didn’t fail, and that some other validation added two messages to the results. I know that’s not possible currently, given the way the underlying code works. But as things change down the road, this sort of weak assertion creates great hiding places for insidious problems.
A small downside is that my tests for Booking are still dependent on the details in the validation classes. As they change, I might have to update the Booking tests as well as the validation tests. I might consider moving reusable concepts like validAirportCode1
into the appropriate Validation test classes (or a common third location). That would allow me to declare them once and access them from both sets of tests.
Testing Approach #4: Test Doubles
I repeat: Did the validator get invoked with the proper list of validations and did it return the results appropriately to the caller?
I can test both of those using test doubles.
Hand-Crafted Test Doubles
I can write a couple tests in which I pass a Validator test double to Booking’s validate
method:
Booking booking = new Booking("", 21, null, null);
@Test
void passesAllValidationsToTheValidator() {
final var passedInValidations = new ArrayList<Validation>();
booking.validate(validations -> {
passedInValidations.addAll(validations);
return null;
});
assertEquals(passedInValidations, booking.validations());
}
@Test
void returnsResultsToClient() {
var booking = new Booking("", 21, null, null);
var results = booking.validate(x -> List.of("a"));
assertEquals(List.of("a"), results);
}
Mockito-based Test Doubles
I can also use Mockito, which remains the de facto standard for test doubles when using JUnit:
@ExtendWith(MockitoExtension.class)
class TooledMock {
Booking booking = new Booking("", 21, null, null);
@Mock Validatable mockValidator;
@Test
void passesAllValidationsToTheValidator() {
when(mockValidator.validate(booking.validations()))
.thenReturn(List.of("a"));
var results = booking.validate(mockValidator);
assertEquals(List.of("a"), results);
}
}
The mock-based test combines both concerns into a single one (which I could have done with the hand grown tests as well). If the argument to validate
doesn’t match the validations returned by the booking instance, the when
expectation isn’t met and thus validate
returns null.
While I know a lot of people reading this prefer to avoid the use of Mockito, or the use of test doubles at all (I myself prefer to minimize their use), I find this to be the easiest route. The test is as streamlined as a test can get, it is very rapidly understood, and it is resilient to changes in the validation classes.
Rethink the Design?
A few people commenting on this example at LinkedIn noted that the design is all wet. Data about a booking is conflated with validation of said same. And it perhaps shouldn’t be possible to create a Booking with invalid data. (If it helps, imagine the class is called BookingDTO, and it represents raw, untrusted data coming in from a request.)
I can extract the validation from the booking record. The challenge remains, however, that somewhere, some code has to ensure that all the validations for bookings are applied to the data in a booking. I might end up with code like:
public class BookingService {
public void add(BookingDTO bookingDTO) {
var validator = new BookingValidator(bookingDTO);
var results = validator.validate(new Validator());
// ...
}
}
Somewhere, somehow, the orchestration has to occur:
import java.util.List;
import static java.util.Arrays.asList;
public class BookingValidator {
private final BookingDTO booking;
public BookingValidator(BookingDTO booking) {
this.booking = booking;
}
public List<String> validate(Validatable validator) {
return validator.validate(validations());
}
List<Validation> validations() {
return asList(
new NameRequired(booking),
new AgeMinimum(booking),
new FutureDate(booking),
new ItinerarySize(booking),
new ItineraryAirports(booking));
}
}
… and it must be tested. All the testing considerations and approaches discussed above once again come to bear (unless you defer all of this to integration testing efforts, a potentially acceptable meta-approach), though the implementation might differ wildly. Ultimately, the question is always: Are all the fields on a booking object getting validated.
Summary
All of the above approaches will work, and all have their detractors and come with concessions. Use the one most useful in your current context, and adapt it if it starts costing you more over time.