PSA: Stricter incremental coverage requirements for //net

61 views
Skip to first unread message

Adam Rice

unread,
May 30, 2024, 10:43:17 AMMay 30
to net-dev
In response to an incident with Cronet, we have increased the incremental coverage requirement for //net to 90%.

What this means in practice is that 90% of lines added or modified in a CL must be covered by tests.

In most cases this shouldn't be a problem. Existing coverage rates in //net are high, so most of the time it will be straightforward to add new tests if needed.

If a CL you write is blocked for low coverage, try adding tests.

If you can't do that, then https://chromium.googlesource.com/chromium/src/+/HEAD/docs/testing/code_coverage_in_gerrit.md has documentation on how to override the coverage check.

For example, if you're doing a large-scale refactor, add

Low-Coverage-Reason: LARGE_SCALE_REFACTOR No functional change.

to the CL description.

If the existing code is untested and it would be unreasonably difficult to make it testable, add

Low-Coverage-Reason: HARD_TO_TEST Code is not testable because reasons

Hopefully this will be rare.

If this change causes you problems please get in touch on this list or elsewhere.

Probably asked questions:

Why is //net special?

The //net code is unusual in that it is used by Cronet in different configurations than are shipped by Chrome. This means that code paths that are not normally exercised by Chrome users may be critical for some Android application. In particular, when adding feature flags, make sure you test both cases using ScopedFeatureList with parameterized tests.

Why line coverage and not branch coverage?

Because that's what the tooling supports.

Code coverage doesn't guarantee quality or lack of bugs.

True, but that's not a question.
Reply all
Reply to author
Forward
0 new messages