Skip to content

Commit 8999661

Browse files
dfa1claude
andcommitted
docs(testing): explain reading Sonar/PIT as data — coverage and duplication triage
Document the interpretation framework discussed while clearing the Sonar findings: an uncovered line is one of missing-test / dead-code / defensive-by-contract, coverage cannot tell them apart but mutation testing can, and flagged duplication is usually real but sometimes the deliberate price of the hot-loop vectorisation rule (BitpackedEncodingDecoder). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e683a63 commit 8999661

1 file changed

Lines changed: 44 additions & 0 deletions

File tree

docs/testing.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,47 @@ SonarCloud daily. Generated `fbs/`/`proto/` sources and the `performance/` bench
122122
are excluded — they have no hand-written behaviour worth covering. The quality gate requires
123123
zero bugs and zero vulnerabilities; the build itself fails on any javac warning
124124
(`-Xlint:all -Werror`), zero Checkstyle violations, and zero Javadoc warnings.
125+
126+
## Reading the signals: Sonar and PIT as data, not verdicts
127+
128+
SonarCloud and PIT both report facts, not judgements. A Sonar finding ("this line is
129+
uncovered", "these blocks are duplicated") is a pointer to look, not a defect by itself —
130+
the interpretation is the engineering work. Two patterns recur often enough to be worth
131+
naming.
132+
133+
### An uncovered line is one of three things
134+
135+
When Sonar flags a line as not covered, it is exactly one of:
136+
137+
1. **Missing test** — reachable by valid input, just never exercised. Add the test.
138+
2. **Dead code** — unreachable by any input. Delete it; a test would only pin behaviour
139+
that can never run.
140+
3. **Defensive-by-contract** — reachable only if an invariant is already broken: the
141+
`default -> throw new VortexException(...)` arms, the `catch (IOException)` on metadata
142+
decode, the `logicalIdx < 0 || >= rowCount` guards on malformed offsets. Not dead (it
143+
guards a real corruption case), but unreachable through the *writer*, which only emits
144+
valid files. Keep it, and either cover it with a hand-crafted malformed-input test or
145+
leave a comment stating the invariant it defends.
146+
147+
Coverage alone cannot tell these apart — it only says "not executed". The deciding question
148+
is *can any input reach this line?* **Mutation testing answers it where line coverage cannot:**
149+
a mutant that survives on a covered line is either an untested-reachable edge (bucket 1) or
150+
an equivalent mutant on a clause that can never change the outcome (bucket 2, dead code).
151+
That is why PIT is scoped to the bounds/parse classes — those are dense with bucket-3 guards,
152+
and the kill rate tells us which guards are genuinely load-bearing. Read a survivor
153+
**simplify-first**: prefer deleting the clause over writing an unkillable test.
154+
155+
### Duplication can be real or deliberate
156+
157+
Sonar's duplication metric is also a pointer, not an order. Most flagged duplication is real
158+
and should be factored out — e.g. the four `unpackLoop8/16/32/64` methods in
159+
`BitpackedEncodingDecoder` each rebuilt an identical per-row schedule, now hoisted into one
160+
`schedule(typeBits, bitWidth)` helper. But some duplication is the price of a hard
161+
constraint: the per-element inner unpack loops in those same methods stay specialised per
162+
width on purpose, because a generic `ValueLayout`/accessor would stop C2 from constant-folding
163+
the typed access and block superword vectorisation (the hot-loop rule). When duplication and a
164+
performance or safety invariant conflict, the invariant wins — factor out the cold,
165+
run-once part and leave the hot, specialised part alone, with a comment saying why.
166+
167+
The throughline: let the tools point at the data, then decide with the context they do not
168+
have.

0 commit comments

Comments
 (0)