Skip to content

build: the coverage gate's statement count depends on test-cache warmth #369

Description

@OmarAlJarrah

Problem

scripts/check-coverage.sh counts statements from the profile rather than reading
go test's rounded percentage, and its header says why:

Coverage is counted from the profile, statement by statement, rather than read
from go test's "coverage: N%" summary lines. Those are rounded to one decimal
place, so a package at 99.96% prints "100.0%" and a gate reading them cannot
see a single uncovered statement.

The number it reports instead is not a property of the tree. It depends on how much
of the test cache is warm, because the merged profile contains duplicate blocks
for packages that ran rather than replayed.

Reproduction

On main @ dbf0054, in a clean worktree:

$ ./scripts/check-coverage.sh | tail -1
Coverage gate passed: all 4942 statements covered.
$ tail -n +2 cover.out | wc -l ; tail -n +2 cover.out | sort -u | wc -l
    3517
    3517

$ go clean -testcache
$ ./scripts/check-coverage.sh | tail -1
Coverage gate passed: all 8672 statements covered.
$ tail -n +2 cover.out | wc -l ; tail -n +2 cover.out | sort -u | wc -l
    6319
    3517

Same tree, same script, same commit: 4942 or 8672 depending only on the cache. The
unique block count is 3517 either way, so nothing about the code changed — 2802
profile lines are repeats.

A partially-warm cache lands in between. A run during which one package re-ran
reported 5452, with exactly compilers/openapi/internal/annotation's 377 blocks
duplicated; another reported 5325.

What is and is not affected

The gate itself is still sound, and in the safe direction. hit sums $2 where
$3 > 0 while total sums every $2, so a block appearing twice — once covered,
once not — is counted twice in total and once in hit, and the run fails. Duplicates
can only produce a false failure, never a false pass.

What is affected is the number, and the number is the script's whole premise. "all N
statements covered" reads as a fact about the tree; it is a fact about the cache. It
also makes the figure useless for the thing a statement count is otherwise good for —
noticing that a change added far more statements than its diff explains.

Notes

Found while quoting the figure in a PR body and noticing it had moved by 383 for a
six-line change. GOFLAGS is empty, so no ambient -coverpkg is in play; the
duplication comes out of the per-package profile merge for ./....

Worth deciding whether the fix is to de-duplicate blocks before counting (a block is
identified by its file:span, so sort -u on the profile body is nearly the whole
change), or to set an explicit -coverpkg so each block is emitted once. The first is
smaller and does not change what is instrumented.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions