Waiting to happen by Mark Seemann
A typical future test maintenance problem.
In a recent article I showed a unit test and parenthetically mentioned that it might have a future maintenance problem. Here's a more recent version of the same test. Can you tell what the future issue might be?
[Theory] [InlineData("2023-11-24 19:00", "juliad@example.net", "Julia Domna", 5)] [InlineData("2024-02-13 18:15", "x@example.com", "Xenia Ng", 9)] [InlineData("2023-08-23 16:55", "kite@example.edu", null, 2)] [InlineData("2022-03-18 17:30", "shli@example.org", "Shanghai Li", 5)] public async Task PostValidReservationWhenDatabaseIsEmpty( string at, string email, string name, int quantity) { var db = new FakeDatabase(); var sut = new ReservationsController( new SystemClock(), new InMemoryRestaurantDatabase(Grandfather.Restaurant), db); var dto = new ReservationDto { Id = "B50DF5B1-F484-4D99-88F9-1915087AF568", At = at, Email = email, Name = name, Quantity = quantity }; await sut.Post(dto); var expected = new Reservation( Guid.Parse(dto.Id), DateTime.Parse(dto.At, CultureInfo.InvariantCulture), new Email(dto.Email), new Name(dto.Name ?? ""), dto.Quantity); Assert.Contains(expected, db.Grandfather); }
To be honest, there's more than one problem with this test, but presently I'm going to focus on one of them.
Since you don't know the details of the implementation, you may not be able to tell what the problem might be. It's not a trick question. On the other hand, you might still be able to guess, just from the clues available in the above code listing.
The code shown here is part of the sample code base that accompanies my book Code That Fits in Your Head.
Sooner or later #
Here are some clues to consider: I'm writing this article in the beginning of 2021. Consider the dates supplied via the [InlineData] attributes. Seen from 2021, they're all in the future.
Notice, as well, that the sut takes a SystemClock dependency. You don't know the SystemClock class (it's a proprietary class in this code base), but from the name I'm sure that you can imagine what it represents.
From the perspective of early 2021, all dates are going to be in the future for more than a year. What is going to happen, though, once the test runs after March 18, 2022?
That test case is going to fail.
You can't tell from the above code listing, but the system under test rejects reservations in the past. Once March 18, 2022 has come and gone, the reservation at "2022-03-18 17:30" is going to be in the past. The sut will reject the reservation, and the assertion will fail.
You have to be careful with tests that rely on the system clock.
Test Double? #
The fundamental problem is that the system clock is non-deterministic. A typical reaction to non-determinism in unit testing is to introduce a Test Double of some sort. Instead of using the system clock, you could use a Stub as a stand-in for the real time.
This is possible here as well. The ReservationsController class actually depends on an IClock interface that SystemClock implements. You could define a test-specific ConstantClock implementation that would always return a constant date and time. This would actually work, but would rely on an implementation detail.
At the moment, the ReservationsController only calls Clock.GetCurrentDateTime() a single time to get the current time. As soon as it has that value, it passes it to a pure function, which implements the business logic:
var now = Clock.GetCurrentDateTime(); if (!restaurant.MaitreD.WillAccept(now, reservations, reservation)) return NoTables500InternalServerError();
A ConstantClock would work, but only as long as the ReservationsController only calls Clock.GetCurrentDateTime() once. If it ever began to call this method multiple times to detect the passing of time, using a constant time value would mostly likely again break the test. This seems brittle, so I don't want to go that way.
Relative time #
Working with the system clock in automated tests is easier if you deal with relative time. Instead of defining the test cases as absolute dates, express them as days into the future. Here's one way to refactor the test:
[Theory] [InlineData(1049, 19, 00, "juliad@example.net", "Julia Domna", 5)] [InlineData(1130, 18, 15, "x@example.com", "Xenia Ng", 9)] [InlineData( 956, 16, 55, "kite@example.edu", null, 2)] [InlineData( 433, 17, 30, "shli@example.org", "Shanghai Li", 5)] public async Task PostValidReservationWhenDatabaseIsEmpty( int days, int hours, int minutes, string email, string name, int quantity) { var at = DateTime.Now.Date + new TimeSpan(days, hours, minutes, 0); var db = new FakeDatabase(); var sut = new ReservationsController( new SystemClock(), new InMemoryRestaurantDatabase(Grandfather.Restaurant), db); var dto = new ReservationDto { Id = "B50DF5B1-F484-4D99-88F9-1915087AF568", At = at.ToString("O"), Email = email, Name = name, Quantity = quantity }; await sut.Post(dto); var expected = new Reservation( Guid.Parse(dto.Id), DateTime.Parse(dto.At, CultureInfo.InvariantCulture), new Email(dto.Email), new Name(dto.Name ?? ""), dto.Quantity); Assert.Contains(expected, db.Grandfather); }
The absolute dates always were fairly arbitrary, so I just took the current date and converted the dates to a number of days into the future. Now, the first test case will always be a date 1,049 days (not quite three years) into the future, instead of November 24, 2023.
The test is no longer a failure waiting to happen.
Conclusion #
Treating test cases that involve time and date as relative to the current time, instead of as absolute values, is usually a good idea if the system under test depends on the system clock.
It's always a good idea to factor as much code as you can as pure functions, like the above WillAccept method. Pure functions don't depend on the system clock, so here you can safely pass absolute time and date values. Pure functions are intrinsically testable.
Still, as the test pyramid suggests, relying exclusively on unit tests isn't a good idea. The test shown in this article isn't really a unit test, but rather a state-based integration test. It relies on both the system clock and a Fake database. Expressing the test cases for this test as relative time values effectively addresses the problem introduced by the system clock.
There are plenty of other problems with the above test. One thing that bothers me is that the 'fix' made the line count grow. It didn't quite fit into a 80x24 box before, but now it's even worse! I should do something about that, but that's a topic for another article.
