Code Review - Review the tests

This section is concerned about reviewing a pull request’s testing and deployment procedures.

Review testing and deployment procedures

A quality code review should encompass more than the code written. It should review the testing and deployment portions of the change as well. The goal here is to reliably and consistently deliver robust software. Each part of the SDLC can be susceptible to misunderstanding and failure. We should strive to put at least two sets of eyes and minds on each step of the process.

If the change is a complex one, ask for a test plan. If the change may need multiple deployment steps, ask for a deployment plan.

Do the tests cover the changes appropriately?

A code coverage metric is very helpful for this. If you’re not using one, I strongly recommend adding it. Knowing that code ran is half the battle of testing.

Review the tests to see which aspects of the code are being tested. Could there be another context for the code to be run within? For example, do you have multiple types of users that could impacted by this code? If so, do the tests validate all experiences?

You should check to see if the tests are covering the happy flow and the reasonable exception flows. The happy flow being the expected, typical flow for the code. A reasonable exception flow is something that is likely to happen, but not typical. For example, an unauthenticated user making a request to a view. A potentially unreasonable exception flow is the database being down. If your entire application is going to be unuseable without a database, it doesn’t make sense to hold up this particular PR. It may also be beyond the knowledge of the author.