One of the things that I like to show people during my TDD courses, is how much easier it is to do a test review rather than a simple code review.
A code review is like literally debugging the code with your eyes, trying to understand not only how it works, but what it's trying to do. The latter is much harder to understand from reading code, because understanding why things are the way they are requires looking at the meta level of the code.
Stuff like "oh, so you're looping here and your throwing there because it's illegal for any of these variables to be null" are functional demands which the code executes, but you need to figure them out from reading the code and talking to the person who wrote it. That's fine, but it can be very time consuming, and most developers don't write code that automatically lends itself to understanding its functional requirements or easy to understand at all. most code written on the face of this planet is mostly unreadable, if my travels between different code bases is of any value whatsoever.
But when you have unit tests you can use those unit tests not only as a way to make sure stuff isn't breaking, but also as a way to understand and review the functional details of what the developer has written. Unit tests can be used both as good API documentation (assuming they are written in a readable manner, using naming standards), but are also great, as I've found, for code reviews. I call them test reviews because the process of the review starts with the reviewer looking at the tests that developer has written instead of starting at the actual code level.
Some of the benefits in a test review:
- Tests reveal the intention behind the code much better than the code itself. That means it's easier to discover logical bugs in the code by reading a test. "Why are you testing that it throws an exception when it should always simply log errors?"
- Tests are declarative by default - the developer declares what the code is supposed to be accomplishing, but the test itself is usually very easy to read (or at least should be).
- Tests are faster and shorter to read and understand. I can review a test in 10th the amount of time it takes me to review code. I'll usually flip between the test and the code only if I need to, but I can usually find logical errors in a test case in less than a minute or so, without even looking at the code. that saves plenty of time for everyone.
- Reviewing tests is also a good idea to keep the quality of the test up - making sure it's readable, maintainable etc..
Here's stuff you should be doing as part of a test review:
- If there are any new tests whatsoever relating to the new code base, you need to read them before even looking at the code.
- Make sure all the tests pass.
- Verify test naming conventions and readability and maintainability (promote code-reuse in tests and remove code duplication in the test class as much as possible without hurting readability)
- if the tests for a piece of code all seem ok, you can move on to that piece of code
- When review the piece of code, you can also verify the test coverage if you feel there isn't enough using this technique, or by using NCover or Visual Studio Test Coverage tools.
- Review the code as you did always (most people never review code, so at least settle for review the tests..)
Are there any steps that you like to do when reviewing code at your company?