Unit Testing — Stop Verifying Everything

This picture pictures (ha!) how my unit tests look like when they’re littered by pointless verifications.

Photo by Jon Tyson on Unsplash

I get it, testing is important, and unit testing is like the first gate of quality, the primal assurance of working code, staple food of modern programming, you name it.

The idea of unit testing, however, became the source of many debates about its best practice, from how to mock things to how to verify things. The latter is our main subject of this article.

Consider this service class that need to be unit-tested

Now, let’s try to test a condition: when an ItemDTO sent with correct outletId and name and no item with same name exists that item should be created

We have four verifications, all of them indicates that all invocation of itemRepository and itemConverter should be verified.

Now, let me say something rather controversial, here. One of those verifications is inadequate and three of them are useless.

In most businesses, the only tests that have business value are those that are derived from business requirements

That’s a quote from an article written by James O Coplien, a veteran in research of computer science. He wrote a lengthy explanations and examples of useless unit testings, and why most of them deviate far from what considered to be valuable.

Let’s look back on our unit test. From business perspective, the only thing that’s important for that create function is the part that save the data into the database. As a developer, we have to make sure that the method is invoked, and invoked correctly with valid parameter.

1. Remove the useless verification: When a correct ItemDTO is put into the parameter, the itemRepository.save() have to be invoked, and that’s it. We do not need to verify the code that retrieve existing item, let alone the converter. This seems to be a lazy approach to a unit test, but now we can ask one question, does adding those three verifications add anything to our code quality? When a correct value is put into the parameter, itemRepository.findFirstByOutletIdAndNameAndIsDeletedFalse(...) always been invoked, it doesn’t get invoked when the ItemDTO is invalid — and that’s not the scope of this test.
2. Use meaningful verification: When verifying the itemRepository.save() that any(Item.class) is less useful. Look at itemDTO.setId(null), this ugly part is used to make sure that new record is created in DB and doesn’t update any existing item. Check that using verify(this.itemRepository).save(argThat(argument -> argument.getId() == null));

By doing things above, my code will look like this

And no, I will not talk about the result assertion, that’s a topic for another day. See ya.