How to do test reviews

Related: Definition of a unit test

Summary

Test Reviews (like code reviews, but on tests) can offer you the best process for teaching and improving the quality of your code and your unit tests while implementing unit testing into your organization. Review EVERY piece of unit testing code, and use the following points as a simple check list of things to watch out for. (you’ll find this is mainly useful when working in statically types languages such as Java or C#).

You can find these guidelines and more on the last page of the book. (but this page contains additions not found in the book)

 

Readability 

  • Make sure setup and teardown methods are not abused. It’s better to use factory methods for readability  (p. 188, 214)
  • Make sure the test tests one thing only (p. 179)
  • Check for good and consistent naming conventions (p. 210-211)
  • Make sure that only meaningful assert messages are used, or none at all (meaningful test names are better) (p. 212)
  • Make sure asserts are separated from actions (different lines). (p. 214)
  • Make sure tests don’t use magic strings and values as inputs. use the simplest inputs possible to prove your point.
  • Make sure there is consistency in location of tests. make it easy to find related tests for a method, or a class, or a project. 

Maintainability

  • Make sure tests are isolated from each other and repeatable (p. 191)
  • Make sure that testing private or protected methods is not the norm (public is always better) (P. 182)
  • Make sure tests are not over-specified (p. 205)
  • Make sure that state-based testing is preferred over using interaction testing (p. 83)
  • Make sure strict mocks are used as little as possible (leads to over specification and fragile tests) (p. 106)
  • Make sure there is no more than one mock per test (p. 94)
  • Make sure tests do not mix mocks and regular asserts in the same test (testing multiple things) 
  • Make sure that tests ‘verify’ mock calls only on the single mock object in the test, and not on all the fake objects in the tests (the rest are stubs, this leads to over specification and fragile tests) (p.123)
  • Make sure the test verifies only on a single call to a mock object. Verifying multiple calls on a mock object is either over specification or testing multiple things.
  • Make sure that only in very rare cases a mock is also used as a stub to return a value in the same test (p. 84)

 

Trust

  • Make sure the test does not contain logic or dynamic values (p. 178)
  • Check coverage by playing with values (booleans or consts) (p. 180)
  • Make sure unit tests are separated from integration tests (p. 180)
  • Make sure tests don’t use things that keep changing in a unit test (like DateTime.Now ). Use fixed values.
  • Make sure tests don’t assert with expected values that are created dynamically - you might be repeating production code.