fix(ci): stop patch coverage counting comments as uncovered lines - #153
Merged
Conversation
A comment cannot be executed, so it can be neither covered nor uncovered -- and no test anyone writes will ever turn one green. The gate counted them anyway, so the only way to raise the number on a well-explained change was to delete the explanation, which inverts what the gate is for. The cause is a lossy conversion, not diff-cover. Go's coverprofile records BLOCKS -- "lines A..B hold N statements" -- and never says which lines in the block are the statements. gocover-cobertura expands each block into one <line> per line of the range, so every comment and blank line inside a function body arrives as hits="0", indistinguishable from real untested code. hack/strip-comment-lines.go removes those entries before diff-cover sees them, using go/scanner rather than matching on "//": a line is code when the scanner emits at least one non-COMMENT token on it. That keeps `x := "http://example.com"` (a string that looks like a comment) and `doThing() // why` (code sharing a line with one) -- the two cases a regex gets wrong in opposite directions. It is deliberately conservative: a file it cannot read or cannot lex keeps every line it had, since dropping lines there would hide genuinely uncovered code. Measured in peeq (PR #145) -- same code, same tests, only the gate's arithmetic differs: 190 lines/31 missing/75% FAIL -> 164/18/89% pass. The 26-line difference is entirely comments and blanks. patch-coverage.sh is otherwise unchanged: this copy already carried the skip-patch-coverage hatch (spelled without its brackets here, so this commit does not trip the hatch it describes) and the report-paths mapping guard, so the whole-file sync restores byte-identity with the rest of the family (peeq, loom, music, lens, lens-console) without altering anything else. This repo keeps go.mod at the ROOT rather than under backend/, so hack/ sits inside the module. The //go:build ignore tag keeps the new file out of `go build ./...` and `go vet ./...` while `go run hack/...` still works -- both verified here, along with the nine-case self-test and a full gate run against this repo's real coverage report.
trick77
force-pushed
the
fix/patch-coverage-ignores-comments
branch
from
July 25, 2026 09:09
d201fcf to
e6c5f81
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.
Why
Comments were counted as uncovered lines.
A comment cannot be executed, so it can be neither covered nor uncovered — and no test anyone writes will ever turn one green. The gate counted them anyway, so on a well-explained change the only way to raise the number was to delete the explanation. That inverts what the gate is for.
Found in peeq, where PR #145 failed at 75% with every test passing.
The cause is a lossy conversion, not diff-cover
Go's coverprofile records blocks —
lines A..B hold N statements— and never says which lines in the block are the statements.gocover-coberturaexpands each block into one<line>element per line of that range, so every comment and blank line inside a function body arrives ashits="0", indistinguishable from real untested code.hack/strip-comment-lines.goremoves those entries before diff-cover sees them, usinggo/scannerrather than matching on//: a line counts as code when the scanner emits at least one non-COMMENTtoken on it.That gets right the two cases a regex gets wrong in opposite directions:
//url := "http://example.com"doThing() // why// an own-line commentIt is deliberately conservative — it may only remove lines it can prove carry no code. A file it cannot read or cannot lex keeps every line it had. A file that merely fails to parse is still stripped: it tokenizes fine, so which lines carry code is still known exactly.
Measured
From peeq's PR #145 — same code, same tests, only the gate's arithmetic differs:
The 26-line difference is entirely comments and blanks; every one of the 18 remaining misses is real uncovered code.
Nothing else changes
This copy already carried the
[skip patch-coverage]hatch and the report-paths mapping guard, so the whole-file sync restores byte-identity with the rest of the family (peeq, loom, music, lens, lens-console) without altering anything else.Root module layout
This repo keeps
go.modat the root rather than underbackend/, sohack/sits inside the module. The//go:build ignoretag keeps the new file out ofgo build ./...andgo vet ./...whilego run hack/…still works.Verified here, not only in peeq:
go build ./...andgo vet ./...unaffectedhack/strip-comment-lines.test.shpasses — nine cases, including the string-containing-//, the trailing comment, block-comment interiors, and both conservative fallbacks