Web Development Business

The Executable Code Review

Tim Broyles Programming, Testing Leave a Comment

Testing has a bad rap. The thought of writing unit tests to exercise code with the goal of 100% code coverage can be overwhelming for many projects. The number of man-hours to set up tests, create mocks when needed, test boundary conditions, contrive odd ball test cases can take some steam out of the project. If this is the definition of test, then yes, writing these types of tests can be tedious and feel meaningless.

In this blog I will talk about my suggestions for writing meaningful tests in the context of a code review.

Code Review Checklist

I am a proponent of writing tests with a narrow focus. The tests I describe here show the completion of a story or the resolution of a bug.

With this narrowness in mind, the task is much less daunting. My goal now is not about code coverage, but more about quality code. With this test, I want to be able to demonstrate to myself (and to whoever is reviewing my changes), that I have successfully resolved my task.

A typical code reviewer will go through a checklist. The process goes something like this:

  1. First, load the project and build. Are there errors? Warnings?
    • How often do we overlook this step?
  2. If this is an API, is there a Swagger page?
    • With Swagger, you can test a deployment of your API with real data. This is the first place I go to evaluate functionality.
  3. Look for and remove orphan code.
    • Remove code that is commented out. If you need this code in the future, you can get it from your repository history.
    • Review comments – 90% of comments in code are obsolete in my opinion, whoever updates the comments after a change?
    • Remove unused imports.
  4. Ask yourself: does it adhere to other team coding standards?
  5. Are there unit tests?

Some of these are pretty self explanatory. However I will touch base on two of the above that need a bit more real estate.

Callout: Does It Work?

The first group is general coding standard checks. Many development environments have macros that you can run at various times to handle a variety of these standards.

Additionally, Jenkins can run various static analysis tools and linters and provide high quality analysis based on the patterns used in your project.

Callout: Unit Tests?

But the last one may seem odd to you and you may ask, “why unit tests in a code review?” After all, you test your code like any good developer does. I use good test data, or a scenario given to me by the bug ticket or story; why write a test for it?

There are three main reasons for why I like this approach:

1. A unit test demonstrates the solution.

Test methods are unique in that they exist for only one reason: to validate that the written code satisfies the requirements (whether written or implied).

This provides an opportunity for the developer to demonstrate how these requirements are satisfied. Much of the meaningful information regarding the issue and solution can be encoded into the test name so there is no ambiguity as to what the test is doing.

For example:


/* Short summary of issue/resolution */

CustomerHistory_GetLatest_ReturnsMostRecentRecordHistory_Issue1234(){}

Additionally, there is a benefit to including this much information. When the test fails sometime in the future, the test name pegs the area of the code with the issue. It will be simpler at that point to evaluate if the bug has re-appeared or if conditions have changed that invalidate the test. In the latter scenario, that would even indicate that the test is invalid, has served its purpose, and can be removed.

2. A unit test helps me write better code.

When you write a test that exercises your solution, you discover that there are areas of functionality that may be a bit congested. That is, areas of the method that may be expressed as decisions, trees, or other complex logic (if, switch, while, etc.).

You may recall that each one of these decisions in the code increases the complexity metric of the method. Pulling these areas out of large methods can only enhance readability, maintainability, and testability.

3. A unit test forces me to define the unit under test.

If the change is isolated, this may only need one unit defined. For example, a service method has a bug in getLatest(). This method is the unit under test.

However if you are testing a new controller/endpoint, the unit under test may be the controller, service, and the repository. You may choose in this instance to create unit tests for each layer.

Final Thoughts

For the purpose of code reviewing, a primary concern is to determine if the issue has been resolved (from #3), if the code is readable (from #2), and if the solution conforms to the team’s coding standards.

I encourage you to try this out with your project, whether your project is legacy and void of tests or green fields of code yet written. Don’t be intimidated by testing. The test frameworks are easy to learn.

The great thing about this approach is that a small change to your process, will have a large impact improving the quality of code in the project, one issue at a time.


About the Author
Tim Broyles

Tim Broyles

Tim is a Software Consultant with Keyhole. A skilled software engineer with 21+ years experience, Tim’s expertise lies in Software Development, Applications Software, Data Analysis and Synthesis, Embedded Systems, and Project Management.


Share this Post

Leave a Reply