How to do unit test reviews
At work we have begun to push unit testing and even TDD. Consequently code reviews now include more unit tests. Often, reviewers are very focused on product code, and neglect to review tests in detail.
Letting poor quality test code slip in is a big problem. Fragile tests erode confidence in unit testing as a valuable exercise, and tests which are hard to maintain just slow you down when the inevitable changes are required.
When I review unit test code, I consider the following:
- Unit tests must favour readability. They must be compact, have a clear relationship between cause and effect, and use descriptive and meaningful phrases (DAMP).
- Unit tests should assert once. There must be a clear relationship between test case method name and the assertion(s). This way, it is easy to determine the purpose of the test at a glance, and it is obvious what it means if the test fails.
- Prefer state verification to behaviour verification. If you must verify behaviour (because there is no state, or it is unreasonably difficult to test state), be very careful not to over specify expectations.
- Unit tests must be based on the public interface of a class. Testing internals makes your tests fragile. This is related to behaviour verification, sometimes you need to verify internal interactions, but this is not the same as invoking private methods in order to test them.
- Unit tests must not contain (or contain very little) conditional logic, such as branches or loops. Conditional logic makes code harder to read.
- Unit tests must be easy to maintain. As far as possible, DRY by applying the following techniques: use test setup/tear down methods to execute code common to all tests; implement custom verification functions or classes; use framework features like CollectionAssert, IEqualityComparer or IComparer.