链接：https://pan.baidu.com/s/1zB-KDzudbxWsOjvfiIE20w 

提取码：tqpp 

 

Tests

In the previous chapter we talked about a wide variety of things you could look for in a code review.

Now we’ll focus on one area: what to look for in the test code.

We’re assuming that “ * Your team already writes automated tests for code. * The tests are regularly

run in a Continuous Integration (CI) environment. * The code under review has been through an

automated compile/test process and passed.

Ask yourself these questions

In this chapter we’ll cover some of the things a reviewer could be thinking about when looking at

the tests in a code review.

Are there tests for this new/amended code?

It would be rare that new code, whether a bug fix or new feature, wouldn’t need a new or updated

test to cover it. Even changes for “non-functional” reasons like performance can frequently be proved

via a test. If there are no tests included in the code review, as a reviewer the first question you should

ask is “why not?”.

Do the tests at least cover confusing or complicated sections of

code?

One step beyond simply “is there a test?” is to answer the question, “is the important code actually

covered by at least one test?”.

Checking test coverage is certainly something we should be automating. But we can do more than

just check for specific percentages in our coverage: we can use coverage tools to ensure the correct

areas of code are covered.

Consider this for example:

6

Tests 7

Code Review Changes

You can check the coverage report for the new lines of code (which should be easy to identify,

especially if you’re using a tool like Upsource) to make sure it’s adequately covered.

Code Review Coverage

In this example above, the reviewer may ask the author to add a test to cover the case where the if

on line 303 evaluates to true, as the coverage tool has marked lines 304-306 with red to show they

aren’t tested.

100% test coverage is an unrealistic goal for pretty much any team, so the numbers coming out of

your coverage tool might not be as valuable as insights into which specific areas are covered.

In particular, you want to check that all logic branches are covered, and that complex areas of code

are covered.

Code Review Logic

Can I understand the tests?

Having tests that provide adequate coverage is one thing, but if I, as a human, can’t understand the

tests, they have limited use. What happens when they break? It’ll be hard to know how to fix them.

Tests 8

Consider the following:

Code that is hard to test

It’s a fairly simple test, but I’m not entirely sure what it’s testing. Is it testing the save method? Or

getMyLongId? And why does it need to do the same thing twice?

The intent behind the test might be clearer as:

Easier code to test

The specific steps you take in order to clarify a test’s purpose will depend upon your language,

libraries, team and personal preferences. This example demonstrates that by choosing clearer

names, inlining constants and even adding comments, an author can make a test more readable

by developers other than him- or herself.

Do the tests match the requirements?

Here’s an area that really requires human expertise. Whether the requirements being met by the code

under review are encoded in some formal document, on a piece of card in a user story, or contained

in a bug raised by a user, the code being reviewed should relate to some initial requirement.

The reviewer should locate the original requirements and see if:

• The tests, whether they’re unit, end-to-end, or something else, match the requirements. For

example, if the requirements are “should allow the special characters ‘#’, ‘!’ and ‘&’ in the

password field”, there should be a test using these values in the password field. If the test uses

different special characters then it’s not proving the code meets the criteria.

• The tests cover all the criteria mentioned. In our example of special characters, the requirements might go on to say “…and give the user an error message if other special characters are

used”. Here, the reviewer should be checking there’s a test for what happens when an invalid

character is used