What happens
removedPhases maps a retired phase name to an actionable message. Its first key is
"check" — a valid phase — with the message "folded into check (ADR-0035); rename
the phase". The key should be "pre-check", the name that was folded.
Probed against the parser:
| written |
result |
check |
parses — phaseNames is consulted first (def_parser.go:78), so the entry is unreachable |
pre-check |
expected a phase, got "pre-check" — the generic message, not the actionable one |
post |
phase "post": removed (ADR-0035)… — correct |
banana |
expected a phase, got "banana" |
So the entry is dead code that also fails to do its job: the one name it was written for
gets the message every unknown word gets.
The test is green for the wrong reason
TestPhases_RemovedNamesRefused (internal/lang/control_test.go) asserts:
"pre-check": "check",
…
if !strings.Contains(err.Error(), want) {
pre-check contains check, so strings.Contains is satisfied by the generic message
expected a phase, got "pre-check". The test would pass with no removedPhases entry at
all — which is exactly the state it is in.
That is the sharper half of this issue: a substring assertion that cannot fail.
Build
- Key the entry
"pre-check".
- Make the test assert something the generic message cannot satisfy — the distinctive part
of the actionable text (rename the phase, ADR-0035), not a fragment of the input.
- While there:
post's case is correct and its assertion has the same weakness
("removed" appears only in the actionable message, so it happens to hold — check it
rather than assume it).
Validation
pre-check produces the actionable message, asserted on a substring that appears only
in it.
- The four surviving phases still parse.
- The corrected test fails when the
removedPhases entry is removed — verify by removing
it once, rather than trusting that it would.
What happens
removedPhasesmaps a retired phase name to an actionable message. Its first key is"check"— a valid phase — with the message "folded intocheck(ADR-0035); renamethe phase". The key should be
"pre-check", the name that was folded.Probed against the parser:
checkphaseNamesis consulted first (def_parser.go:78), so the entry is unreachablepre-checkexpected a phase, got "pre-check"— the generic message, not the actionable onepostphase "post": removed (ADR-0035)…— correctbananaexpected a phase, got "banana"So the entry is dead code that also fails to do its job: the one name it was written for
gets the message every unknown word gets.
The test is green for the wrong reason
TestPhases_RemovedNamesRefused(internal/lang/control_test.go) asserts:pre-checkcontainscheck, sostrings.Containsis satisfied by the generic messageexpected a phase, got "pre-check". The test would pass with noremovedPhasesentry atall — which is exactly the state it is in.
That is the sharper half of this issue: a substring assertion that cannot fail.
Build
"pre-check".of the actionable text (
rename the phase,ADR-0035), not a fragment of the input.post's case is correct and its assertion has the same weakness(
"removed"appears only in the actionable message, so it happens to hold — check itrather than assume it).
Validation
pre-checkproduces the actionable message, asserted on a substring that appears onlyin it.
removedPhasesentry is removed — verify by removingit once, rather than trusting that it would.