Why is it important for testers to read pull requests?

I attended the January DAQAA meeting where Kylee Tilley presented on “Reading Pull Requests”, and how they can be useful for testers, not just developers.

As Kylee mentioned in his talk, there aren't a lot of resources available online that even cover the basics of Pull Requests, so here's an overview of his talk, as well as my personal experiences with using pull requests to improve my testing.

What are Pull Requests?

Pull requests are usually the final step developers make when they're ready to merge code changes. A pull request is a collection of all the individual changes, defined as “commits”, that have been introduced into your software. Each commit has a message associated with it, describing the change that was made.

The pull request process isn't unique to any single type of version control. Pull requests serve as a “Code Review” step in the development process, allowing for people to review code changes before they are merged, and to provide comments to allow for additional changes before a pull request is approved and code is merged, ready to be deployed.

Once pull requests are approved, a deployment can be triggered based on a merge, a specified schedule, or can be triggered manually.

Good pull requests are generally made up of a small number of commits, each with a limited focus and a clear intent. Commits can be made up of many small changes to a large number of files. The greater the number of changes, the more files that were updated, the more difficult it becomes to review all the code changes that were made.

Most version control systems such as GitHub allow you to view an individual commit, to view the changes made in a line-by-line comparison, highlighting differences in the code, including code that was added, updated, or removed.

By contrast, you can have a list of Open Pull Requests from multiple developers on your team that may need to be reviewed and approved.

By selecting an individual pull request, you can view any conversation and comments regarding the pull request.

Selecting the “Commits” tab will display a list of all commits that are part of a Pull Request.

The “Files changed” tab displays a list of all files that were changed as part of a pull request, and displays the changes that were made to each file.

How Pull Requests Relate to Testers

Since pull requests are essentially a Code Review step in the development process, it's important for testers to be involved. Testers should be looking at pull requests to gain a better awareness of changes in the system. It's easier to review a pull request of all the changes than to look over the shoulder of your developers as they work (although it can be very beneficial to pair with developers on a regular basis).

Chris Kenst wrote about the importance of getting testers involved in the code review process. He mentions one of the added benefits that comes from participating in code reviews is that “once developers know you pay attention to code reviews, they'll explain things to you.” This opens up the lines of communication in both directions, enabling a tester to review code and ask questions, and even help testers to get involved earlier in the process. The time to write better tests and to discover issues is before a pull request is approved and code changes are merged, not after.

Reviewing pull requests helps with updating regression test suites, and with identifying risk and potential scenarios that need to be tested. Testing should not wait until a story is done.

I used to work at a company that was a Microsoft shop, using Visual Studio, Team Foundation Server (TFS), Microsoft Test Manager (MTM), .NET, and SQL Server. We used Octopus for our deployment process, and kept all of our stories in TFS. We used TFVC for our version control. There weren't any links between the stories in TFS and the individual commits that were made for code changes related to those stories. Perhaps such a link existed in the code, but it wasn't visible in TFS. I had no way to view the code changes related to a story.

Compare that to my more recent experience at a company where we used Jira for all of our user stories, connected to Git / Bitbucket for our version control, using Jenkins for our deployments. From a story within Jira, there was a list of each commit and pull request for that story. I could follow a link to view each commit separately in Bitbucket, and I could view the pull request and all the comments made during the code review process.

The pull request process is rarely as cut and dried as simply making code changes, committing those changes, and submitting a pull request, having it approved almost automatically following a review of all changes. Usually, it's a back and forth process between the developer and the reviewer to make suggested changes and improvements before a pull request is finally approved, and code changes are merged.

I found it beneficial to be able to review all commits and pull requests as an aid in my testing.

• I want to be able to view the code that has changed, whether it has been added, updated, or removed.
• I want to view the tests that have changed (unit, integration, functional), whatever tests that exist as part of the code base.
• I want to be able to see the individual files that were updated.
• I want to be able to view the pull request and all the comments that were made during the code review process, and the conversation about any needed changes.

I use this information to identify additional scenarios that need to be tested, areas of potential risk and impact to other features, and to verify that all parts of the acceptance criteria in a story are covered by changes in the code. It's a common occurrence that if there are 5 critical elements in the acceptance criteria, only 4 of them will be met. If the missing element can be identified early, it can be added during development, instead of having to create yet another story to cover the missing functionality in a later build.

You should keep in mind the importance to developers of having pull requests approved. In a recent presentation at Iowa Code Camp, Matt Travi made an excellent point, stating that “Working code, not yet in master is DEBT.” Code that hasn't been committed yet to master is not available to others on your team who may also be making changes. This is why it's so important to make small, frequent changes, and small, frequent commits. It's also why it's important to test early, so as to not hold up the rest of the team.

Different Types of Pull Requests

Kylee covered several different types of pull requests in his presentation. I won't go over them in much detail, but at the end, I'll provide a link to the example pull requests from his presentation, and a link to the deployed version for each example.

• Feature – a pull request for new functionality being added, or existing functionality being updated
• Refactor – a pull request for changes to the code which should not affect the functionality; if the functionality changes, it's not a refactor
• Defect – a pull request for code changes to fix a defect
• Documentation as Code – a pull request for changes to documentation only, such as for markdown files, e.g. a README file
• Maintenance – a pull request for changes to code dependencies or resource references; the level of risk will determine the type of testing that may be required

It's important not to co-mingle these types of pull requests. It may be easy for developers to do some refactoring while working on a new feature, but it's a much better practice to keep unrelated changes separate.

Challenges and Misconceptions

Why aren't more testers involved in the code review process, or taking the time to review pull requests? It's a common misconception that you need to know how to code in order to read a pull request. You don't have to know exactly what the code does. You can gain context around any changes based on the commit message, the filename, or the tests associated with the code, and you can always ask a developer to help explain what you're looking at.

Bad commit messages were a topic of discussion at my previous company. If the commit message doesn't communicate anything about the change being made, it's of no value. A commit message should be brief, but should enable a person to gain context by simply reading the message.

Large pull requests were also a common problem. Sometimes the issue was the large number of changes made all at once to the same file. Sometimes it was the large number of files that were changed. It wasn't uncommon to find instances where 50+ files were being updated in a single commit, in a pull request with multiple such commits. It's very difficult to view such a large number of changes and to build a holistic view of what was changed.

Going back to the point about co-mingling pull requests, when multiple, unrelated changes are contained in the same pull request, it adds overhead when determining which changes are related to one another. If an issue is discovered later on, it can be very confusing when reviewing the changes made that may have caused it, when required to separate out things that are unrelated.

Summary

As Chris mentioned in his article, “Code reviews are a popular method of catching bugs early in development through peer-reviewing someone's code.” There's a lot of benefit for testers who get involved in reading the pull requests made for changes to software they're responsible to test.

The next time someone is assigned to review a pull request, ask if you can pair with them while they review the changes. It's a great opportunity to learn as much as you can about the changes being made, to learn about how the code is organized, and a way to help you test earlier, simply by reviewing the changes made.

References

Here is a list of the Pull Request examples that Kylee provided, along with the deployed version for each:

https://github.com/testingrequired/talk-reading-pull-requests/pull/1
https://reading-pull-requests-counter.netlify.com/

https://github.com/testingrequired/talk-reading-pull-requests/pull/2
https://reading-pull-requests-reset.netlify.com/

https://github.com/testingrequired/talk-reading-pull-requests/pull/3
https://reading-pull-requests-refactor.netlify.com/

https://github.com/testingrequired/talk-reading-pull-requests/pull/4
https://reading-pull-requests-bugfix.netlify.com/