Improving unit test readability: helper methods & named arguments
'Uncle Bob' wrote the following in Clean Code: A Handbook of Agile Software Craftsmanship:
"The ratio of time spent reading (code) versus writing is well over 10 to 1 ... (therefore) making it easy to read makes it easier to write."
So think about that when you write your next bit of code. You need to make sure your code is easily readable and understandable for others, you hardly ever write code just for yourself.
Readable unit tests
Unit test code is not different than 'production' code. Readability is key here because unclear unit tests will be distrusted, ignored and possibly removed.
Bulky Arrange section
Consider the following unit test for testing the GetHighestRatedMovies
method in an imaginary MovieService
:
UnitTestWithBulkyArrangeSection.cs
[Fact]
public void GetHighestRatedMovies_RepositoryContainsMoviesWithRatings_ReturnsMoviesOrderedByDescendingRating()
{
// Arrange
var fixture = new Fixture();
var movieCollection = fixture.CreateMany<Movie>(20).ToList();
var fakeMovieRepository = A.Fake<IMovieRepository>();
A.CallTo(() => fakeMovieRepository.GetAll()).Returns(movieCollection);
IMovieService movieService = new MovieService(fakeMovieRepository, null, null);
MovieServiceRequest request = new MovieServiceRequest { NumberOfMoviesToReturn = 5 };
// Act
IList<Movie> movies = movieService.GetHighestRatedMovies(request);
// Assert
movies.Should().BeInDescendingOrder(movie => movie.Rating);
}
Although this unit test uses some great frameworks such as xUnit, AutoFixture, FakeItEasy and FluentAssertions (big fan of these!) there are still some things I don't like, particularly in the Arrange section:
- In lines 5-6 a collection of
Movie
objects is set up using AutoFixture. I get why this collection is neccesary but I really don't care about how it's done. In addition you could argue that 20 is a magic number although the intent is quite clear here. It can definitely be a bit more clear. - In lines 7-8 a fake object based on
IMovieRepository
is created using FakeItEasy. The fake repository is required to be able to returnMovie
objects from theMovieService
but again I don't really care how that is done. - In line 9 the
MovieService
is instantiated and the fake repository is passed in the constructor. But what are thenull
arguments there? What do they represent? - In line 10 a
MovieServiceRequest
object is constructed. It's a simple value object with just one property. But what will happen if more properties are added later? Then the construction of this request will take up quite some space which has a negative impact on readability.
In general I feel there is too much detail in this Arrange section which is not relevant for understanding the unit test. Although 6 lines is not that much I do believe fewer lines in the Arrange and clear usage of arguments will improve the readability a lot.
Lean Arrange section
Here's how I refactored the unit test:
UnitTestWithLeanArrangeSection.cs
[Fact]
public void GetHighestRatedMovies_RepositoryContainsMoviesWithRatings_ReturnsMoviesOrderedByDescendingRating()
{
// Arrange
IMovieRepository fakeMovieRepository = GetFakeMovieRepository(nrOfMoviesInRepository: 20);
IMovieService movieService = new MovieService(fakeMovieRepository, context: null, logger: null);
MovieServiceRequest request = GetMovieServiceRequest(nrOfMoviesToReturn: 5);
// Act
IList<Movie> movies = movieService.GetHighestRatedMovies(request);
// Assert
movies.Should().BeInDescendingOrder(movie => movie.Rating);
}
private static IMovieRepository GetFakeMovieRepository(int nrOfMoviesInRepository)
{
var fixture = new Fixture();
var movies = fixture.CreateMany<Movie>(nrOfMoviesInRepository).ToList();
var fakeRepository = A.Fake<IMovieRepository>();
A.CallTo(() => fakeRepository.GetAll()).Returns(movies);
return fakeRepository;
}
private static MovieServiceRequest GetMovieServiceRequest(int nrOfMoviesToReturn)
{
return new MovieServiceRequest
{
NumberOfMoviesToReturn = nrOfMoviesToReturn
};
}
There are two major differences:
- Creation of fake objects and the request object is now done in private methods. This allows re-usage of these methods in future unit tests. When the number of these helper methods grows you should consider moving them to a seperate class.
- Named arguments (available since C# 4.0) are used when calling the helper methods and for constructing the
MovieService
. It is now evident what thenull
values actually represent. An alternative would be to declare seperate variables for all these arguments. Although that would be very clear and descriptive I believe that would hurt readability because the Arrange section would get quite bulky again.
Tips for keeping your unit tests lean and understandable
- I usually only 'new up' the class under test directly in the Arrange section, other (fake) objects are created in helper methods or classes.
- Use existing and proven libraries & frameworks so you don't have to write boilerplate code and you can focus on more difficult problems.
- Try to use a mocking framework (such as FakeItEasy) over a custom made mock/stub framework. Using a custom framework costs time in two ways: it needs to be maintained and it needs to be explained to every new member on the team.
- Need to create collections of fake objecs? Consider using AutoFixture's
CreateMany<T>
. - The FluentAssertions library contains dozens of useful assert methods, especially for collections.
- Always be very explicit in naming the methods and arguments to avoid unclarity. Consider using named arguments when you need to pass numbers, strings or null to methods. If you're using code analysis tools such as Resharper you'll get warnings that the usage of named arguments is not required most of the time. You might want to lower the severity of that message so the code analysis stays 'green'.
Happy unit testing!