Close the coverage gaps a mutation review found, and make the coverage contract enforceable - #72
Merged
Merged
Conversation
An adversarial review applied 82 mutations to defaults.go and ran the suite
against each one. 21 survived, and three of the tests turned out to be
structurally incapable of failing.
The three that could not fail:
- `uint8 default:"256"` asserting the zero value cannot detect a widened bit
size, because uint8(256) is 0. Every integer case passes its width to
strconv as a literal, so 14 of the survivors were width and base mutants:
int, uint, uintptr, int32, uint16, uint32 and float32 had no boundary test
at all. Replaced with the largest and smallest values that must parse, plus
past-the-boundary values chosen to truncate to something non-zero (257,
65537, 4294967297), and uintptr added to the base subtests.
- TestSet_JSONUnmarshalerSkipsEmptyContainers claimed to pin that `{}` and
`[]` never reach a custom UnmarshalJSON, but used a type that also
implements UnmarshalText, so text won first and the guard was never
evaluated. With a json-only type the two turn out not to be symmetric: `{}`
is withheld, while `[]` reaches UnmarshalJSON through encoding/json. That
asymmetry is now pinned.
- assert.Zero on the cross-package unexported field could not fail, because
that field carried no default tag. It has one now, so dropping the CanSet
guard fails the test.
Also dropped a claim the suite cannot make: which of ParseDuration and
ParseInt runs first is unobservable, since the two accept disjoint inputs.
The test pins the outcomes and says as much.
Shapes that had no test at all: arrays, whose tag is dropped and whose
elements are not recursed into, unlike a slice's; []byte in all three
spellings; time.Time; non-nil interface fields; unexported fields of
composite type, where the CanSet guard skips a whole subtree; map values
pointing at a slice or a map; and a pointer to a non-struct type with a
setter, which is the one shape whose setter is called only by the pointer
branch -- so a plausible fix for #67 would silently break it. Restored the
old suite's embedded-struct-inside-a-container coverage, and added the second
decode failure mode (*json.UnmarshalTypeError) and whitespace tags.
Every mutant spot-checked afterwards is killed. No library behavior changed:
defaults.go and setter.go are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JjAoN3BBvc6dfT9pog3kMB
CLAUDE.md says statement coverage stays at 100%, and nothing could fail on a drop: both Codecov statuses are informational, and the summary step always exits 0. `make cover` now fails below the threshold, verified by deleting a test file, which takes it to 95.5%. It also profiles the library package alone. Under ./... the total read 91.7%, diluted by example/ and internal/fixture, which have no tests -- so the number CI printed was not the number the contract is about. The Codecov upload loses its `matrix.go == '1.27'` condition. A literal version silently stops matching the day 1.27 leaves the matrix, and with the token in place there is no reason to single out one leg. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JjAoN3BBvc6dfT9pog3kMB
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #72 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 111 149 +38
=========================================
+ Hits 111 149 +38 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Follow-up to #65. An adversarial review applied 82 mutations to
defaults.goand ran the suite against each: 21 survived, and three tests turned out to be structurally incapable of failing. Test and CI only —defaults.goandsetter.goare byte-identical tomaster.Three tests that could not fail
uint8 default:"256"asserting the zero value cannot detect a widened bit size, becauseuint8(256)is 0. Every integer case passes its width to strconv as a literal, so 14 of the 21 survivors were width and base mutants —int,uint,uintptr,int32,uint16,uint32andfloat32had no boundary test at all. Replaced with the largest and smallest values that must parse, plus past-the-boundary values chosen to truncate to something non-zero (257,65537,4294967297,1e39), anduintptradded to the base subtests.TestSet_JSONUnmarshalerSkipsEmptyContainersnever evaluated the guard it claimed to pin. It used a type that also implementsUnmarshalText, so text won first and the JSON branch was never reached. With a json-only type the two are not symmetric:{}is withheld from a customUnmarshalJSON, while[]reaches it throughencoding/json. That asymmetry is live behavior and is now pinned.assert.Zeroon the cross-package unexported field could not fail, because that field carried nodefaulttag — nothing would have set it even if reflect were allowed to. It has one now, so removing theCanSetguard fails the test.Plus one claim the suite cannot make: which of
ParseDurationandParseIntruns first is unobservable, since the two accept disjoint inputs — flip them and every assertion still passes. The test now pins the outcomes and says so, andTestSet_DurationIsTriedBeforeIntegeris renamed accordingly.Shapes that had no test
[2]int,[1]Structreflect.Arrayhas no case: the tag is dropped without an error and elements are not recursed into, unlike a slice's[]bytetime.TimeUnmarshalTextsucceeds; an unparsable value is a hard errorinterface{}*structwith tagsCanSetguard skips a whole subtree, not just a scalarmap[string]*[]T,map[string]*map[string]T*levelwith aSetDefaultsSet[]int default:"{}"*json.UnmarshalTypeError, not a syntax errordefault:" "Coverage is now enforceable
CLAUDE.mdsays statement coverage stays at 100%, and nothing could fail on a drop: both Codecov statuses areinformational: true, and the summary step always exits 0. Delete a test file and CI stayed fully green.make covernow fails below the threshold, and it profiles the library package alone — under./...the total read 91.7%, diluted byexample/andinternal/fixture, which have no tests, so the number CI printed was not the number the contract is about.The Codecov upload also loses its
matrix.go == '1.27'condition: a literal version silently stops matching the day 1.27 leaves the matrix, and with the token in place there is no reason to single out one leg.Verified
Each claim above was run, not reasoned:
int/uint/uintptrwidths, theuintptrbase,float32's bit size, the{}/[]guard, theCanSetguard, the map pointer-element kinds, and the map element tag.set_error_test.gotakes coverage to 95.5% andmake coverexits non-zero. (Removingset_map_test.gostill passes at 100%, correctly — those statements are redundantly covered.)go vet ./...,golangci-lint run,make test,make cover, and the suite underGOTOOLCHAIN=go1.21.13, all clean. 258 test cases, up from 179.Not in scope
The review also confirmed a crash that cannot be an ordinary test, since the test binary dies with it: a self-referential type whose default materializes an element recurses until the stack overflows. Filed as #71, with the pointer, slice and map spellings.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JjAoN3BBvc6dfT9pog3kMB
Generated by Claude Code