ci: compute coverage gate from profile, not 'go tool cover -func' total#219
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Benchmark ComparisonComparing PR branch against main branch: Legend: ✅ OK (no change or faster) | |
There was a problem hiding this comment.
Pull request overview
Updates the CI coverage gate to compute total statement coverage directly from the Go coverage profile (matching go test -cover), addressing under-reporting from go tool cover -func totals for function-literal blocks. Adds a regression test to cover the check command’s invalid --format path that was previously missed by the (now-honest) 100% gate.
Changes:
- Replace
go tool cover -func | grep ^totalwith profile-basedcovered_stmts / total_stmtscomputation inmake check-coverage. - Add
TestCheckCmd_RunE_InvalidFormatto exercise thevalidateFormaterror branch in the CobraRunEclosure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Makefile | Computes coverage threshold from the raw coverprofile to match go test -cover behavior. |
| cmd/colref/check_test.go | Adds a test to cover the invalid --format error path in the check command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shinagawa-web
added a commit
that referenced
this pull request
Jul 2, 2026
Address Copilot review on #219: - The check-coverage recipe is one ;-joined shell block ending in 'echo Coverage OK' (exit 0), so a go test *failure* (which still yields a full-coverage profile) was masked. Add 'set -e' so a non-zero go test aborts the target. Verified end-to-end: an injected failing test makes 'make check-coverage' exit non-zero without printing 'Coverage OK'. - TestCheckCmd_RunE_InvalidFormat now asserts the error message contains the --format validation failure, verifying that path specifically rather than accepting any error.
'go tool cover -func | grep ^total' under-counts uncovered blocks inside function literals (e.g. the cobra RunE: closure), reporting a higher percentage than real statement coverage. On a gappy cmd/colref profile the old metric read 98.3% while go test -cover reported the true 97.3%, so a func-literal gap could pass the 100% gate unnoticed — which is exactly how the RunE validateFormat error branch stayed uncovered. Compute the total directly from the profile (executed statements / total statements), matching go test -cover. The honest metric then exposed that uncovered validateFormat branch, so add TestCheckCmd_RunE_InvalidFormat to close it and keep the gate green at a real 100%. Closes #218
Address Copilot review on #219: - The check-coverage recipe is one ;-joined shell block ending in 'echo Coverage OK' (exit 0), so a go test *failure* (which still yields a full-coverage profile) was masked. Add 'set -e' so a non-zero go test aborts the target. Verified end-to-end: an injected failing test makes 'make check-coverage' exit non-zero without printing 'Coverage OK'. - TestCheckCmd_RunE_InvalidFormat now asserts the error message contains the --format validation failure, verifying that path specifically rather than accepting any error.
shinagawa-web
force-pushed
the
ci/218-coverage-metric
branch
from
July 2, 2026 08:18
cb8c4e0 to
e40213e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #218
Problem
make check-coveragederives the enforced total fromgo tool cover -func <profile> | grep ^total. That total under-counts uncovered blocks inside function literals (e.g. the cobraRunE:closure,main.goclosures), so it reports a higher percentage than the real statement coverage.On a deliberately gappy
cmd/colrefprofile:go test -cover(truth)go tool cover -functotal (old gate)covered_stmts / total_stmts(new gate)Because the gate read the lenient number, a func-literal gap could pass the 100% threshold unnoticed — which is exactly how the
RunEvalidateFormaterror branch stayed uncovered onmain(real coverage was 99.9%, gate said 100%).Fix
go test -cover. Guards an empty profile as0.0. Threshold stays 100%.validateFormatRunEbranch, so addTestCheckCmd_RunE_InvalidFormatto close it and keep the gate green at a real 100%.Verification
make check-coverage→Total coverage: 100.0%/Coverage OK.0.0(no divide-by-zero)golangci-lint0 issues,go vetclean, unit + e2e passRelationship to other PRs
TestCheckCmd_RunE_InvalidFormatand_InvalidColoron its own branch; whichever merges second will need a trivial rebase to drop the duplicate_InvalidFormattest. Flagging so it is expected.