Code Review and Test First Development

One of the most important practices that encourage team members to learn with each other is called Code Review.

Some of the benefits of Code Review are:

• Knowledge sharing.
• Possibility for somebody else to spot honest mistakes through an unbiased view of the code.
• Make sure the code comply with the project standards.

In Code Review, a developer hand over the diff of a small set of code changes to another team member. They look at it for the purpose of finding problems early and increase the quality of the output.

A common pattern of Code Review is to hand over a small set of code changes to somebody else for the purpose of increasing the quality of it.

It may be possible to use Code Review and the order of commits to help the team members to improve their testing skills.

Let’s say there’s a username validation regex with no Test-Driven specs to back it up. Somebody decides to create some tests for it and you’re the one reviewing it.

While it’s possible to add efficient test coverage to an existing Regular Expression after the code has been written, it's very hard to review Test First has been applied to it. It’s not easy to understand the behavior of a regex to make sure all the use cases have been covered by tests and all the rules are really necessary.

The surefire way to review if an efficient Test First approach has been used is to make some changes and run the tests again together with the application code.

However, by doing that, you’ll move away from doing a quick Code Review into something that takes much more time.

Verifying a Test First approach using Code Review is hard when the final changes contain the application code and the passing tests.

It’s hard to verify that the tests were created using Test First if the only thing you have is the final code. If you can’t statically verify, by looking at the diff, that the tests were written before or after the application code, there are no opportunities to:

• Share knowledge of how to write efficient tests using a Test First approach.
• Find honest mistakes that happen when doing Test First.
• Make sure the changes comply with the project’s Test First standards.

But if you look at Code Review from another perspective and make some exceptions to the better practice of atomic commits, it may be possible to verify if Test First has been applied or not.

Let’s say you’re using a Github Code Review workflow with feature Pull Requests:

• The first commit of the Pull Request will contain the test code that makes the test suite fail.
• The second commit will contain the actual changes from the application code that makes the test suite pass.

Here is an example of this idea being applied to Open Source: A Pull Request with one Atomic Commit that fails and one that passes, exposing the issue and the fix, see the Travis Console.

By doing this way, the author can expose their inner thoughts and the reviewer can verify if Test First was used correctly by looking at the changes in each commit. It will also help the author to validate if the next commit really fixes the issue the test is trying to cover.

Creating a commit that makes the test suite fail and another one that makes the test suite pass allow the author to expose their inner thoughts and make sure they're fixing the right thing.

For obvious reasons, this style of commits should not exist in the main trunk because it can mess with things like the git bisect. After the Code Review has finished, the changes can be squashed into an atomic commit and merged into the mainline. That squashed commit can contain the application code and the passing test together. There will be no need to have them split.

This technique of creating one commit for tests and one for the application code in Code Review doesn’t come without tradeoffs. There's a bigger cost than having both changes in the same commit done locally.

Now, what about actual TDD? It’s very hard to infer TDD from the code and the order of commits, although you can infer Test First.

Pair Programming is the best way to verify TDD. However, there might be some people in a team who are against Pair Programming. The cost of change of having multiple commits for failing and passing tests can be reasonable in order to expose at least how everyone else writes the tests. Maybe that can uncover TDD being done wrong.

Creating a process like this and put it as one of the criteria for the Code Review might help to drive the mindset of crafting changes in a way that makes it easier to expose potential problems for the reviewer. If the reviewer can't see a problem, then it will be unlikely that the author of the changes will receive an efficient feedback. Efficient feedback is the main goal of Code Review.

The Code Review process doesn’t need to be restricted by only looking at the final changes. Don’t be afraid of deviating from best practices and experimenting different things, as weird as they might seem. The best practice should be understood and experiments should have a clear purpose. They can be treated as the exception, not the rule.

There is no 100% solution to make sure someone is following a practice like TDD or Test First without watching how they code. What you can try is to incur a small cost of change in order to help the practice to become a habit.

After the experiment has been tried and served its purpose, everybody can go back and do everything else again without the temporary friction.