Writing Good Tests (Part 2) - What Makes a Test Good

In the previous post, I ranted about what not to do when writing tests and how to detect a bad test. In this post, I'm going to attempt to provide some pointers as to how you go about writing good tests.

What makes a test good?

Just like good code is a bit hard to pin down, so is a good test, but you will not be in any doubt when you read a good test, especially if it fails as it achieves its purpose - it will tell you exactly what went wrong and what needs fixing!

Let us demonstrate this by having a look at a failure. Consider our first example Test1. If it fails, you will be presented with text similar to "UnitTest1.Test1 failed: false != true". "This is true" said the tautalog. Instead consider the following test:

public readonly CmsService subject;
public readonly Mock<ICmsReadRepository> readRepoMock;
public readonly Mock<ICmsWriteRepository> writeRepoMock;

public CmsServiceUnitTest()
{
    writeRepoMock = new Mock<ICmsWriteRepository>();
    readRepoMock = new Mock<ICmsReadRepository>();
    subject = new CmsService(Mock.Of<ILogger<CmsService>>(), readRepoMock.Object, writeRepoMock.Object);
}
...
const string InvalidKeyDuoInvalidChar = "An invalid key e><ample";
...
[Fact]
public async Task ReadSectionAsync_InvalidCharKey_ThrowsInvalidCharException()
{
    // Arrange
    DefaultSetup();
    // Act
    var taskExpectedToThrow = subject.ReadSectionAsync(InvalidKeyDuoInvalidChar);
    // Assert
    await Assert.ThrowsAsync<InvalidCharException>(() =>  taskExpectedToThrow);
}
...

And the following message

This tells you that an InvalidCharException was expected from the CmsService when calling ReadSectionAsync, with a key containing an invalid char. Now, in this case, a few things could have gone wrong. Perhaps we no longer indicate failure to read with exceptions anymore but use the optional pattern instead, or perhaps we forgot to validate the key. I happen to know that we did not switch to the optional pattern which means the latter is true. Whoopsie.

All of this, I was able to infer from the test case, thus saving me tons of time I could have wasted Bughunting.

Furthermore, with a nice test coverage % and the most important and most common features under test, I can rest easy at night knowing my software is functional and not just a stinking heap of-. And, I also save time not manually testing which is time well spent in my opinion.

The purpose of a test

Without getting too metatheoretical, I would like to briefly discuss what "the purpose" of a test is, explicitly.

Say, I have a CMS service I want to test. I have some assumptions with respect to its functionality: It should be safe, practical and efficient. Moreover, say, that to be safe, it shouldn't allow all characters but only a specific subset of characters. Now I want to assert that my CmsService doesn't accept any of the illegal characters. Therefore, I write a unit test of which the purpose is to verify this. It initially fails, but after the introduction of a guard-clause, the test passes and I believe everything is good. The next day, I want to demonstrate this new behavior to my team, and I open the webpage utilizing the service to update the entries. You input an illegal character, press save and ... It saves!
You dive in to the database to discover a strange sequence of letters prefixed with an &. Somewhere there was a mismatch between the purpose of the test and the actual test.

This is obviously a very constructed example, but my point is that the test did not fulfill its purpose. A more appropriate test to write when verifying that the system does not accept invalid characters in its frontend is a frontend test, or perhaps a system test. Had I written a frontend test, I might have discovered that some middleware translates user input into safe strings that cannot be utilized in injection attacks. Had I written a system test, I might have discovered that the underlying CMS API has protection. It would have been more appropriate to write a unit test when asserting unit behavior, such as "I expect the CmsService to call the CmsApiClient when GetSectionAsync is called".

Conclusion

In the first post we discussed that if a test is not informative and exact, simple, timely and repeatable, then it's a bad test. In this post, we expanded on the concept of a test being good by discussing what a test needs to do; achieve its purpose of validating an assertion that we make about how some part of, or the whole, system works.

We also determined that this can only be done when we know what the assertion is and create a test based on this. In particular, we want the type of the test to match the layer of abstraction. If only there was some literature on the different kinds tests types paradigms that exist...

Remark

If you have any questions, disagree with anything, or if you would like something added to this blog post - leave a comment!

References

My personal experience,
https://docs.microsoft.com/en-us/dotnet/core/testing/unit-testing-best-practices
https://martinfowler.com/bliki/GivenWhenThen.html
https://gist.github.com/wojteklu/73c6914cc446146b8b533c0988cf8d29 (Clean Code by Robert C. Martin summary)
https://docs.microsoft.com/en-us/aspnet/core/test/integration-tests?view=aspnetcore-6.0