fix: address 8 tire-kick findings (panics, output race, dead fields, doc errors, MSI inconsistency)#18
Merged
Merged
Conversation
- Replace all panic(err) in worker goroutines (runBuiltinExec, runCustomExec) and main-path functions with clean stderr errors and appropriate exit/return codes; a bad --exec path now prints a single line to stderr rather than dumping a goroutine stack trace. - Move all diff and status printing out of interpretBuiltinExitCode (called outside the report mutex) and into recordMutantResult (called under the mutex), eliminating the output race that caused diff lines to interleave with PASS/FAIL lines from other workers. As a result interpretBuiltinExitCode became a trivial two-branch expression and was inlined. - --dry-run output and help text now note that the count is an upper bound; identical mutations across files are deduplicated during a real run, so the dry-run number is always >= the actual mutation count. - README blacklist section: corrected "run go-mutesting normally" to "run with --debug"; checksums are only emitted in debug mode. - README --exec example path: removed spurious /v2/ segment that made the path point to a non-existent directory, causing a panic. - Remove dead Stats fields TimeOutCount and MutationCodeCoverage (never written, always 0 in JSON output) and Report.Timeouted (never populated); update README and tests accordingly. - --logger-agentic-json now writes msi as a 0-1 ratio to match --logger-summary-json, eliminating the prior inconsistency where one output used percentage and the other used ratio. - Track skipped (non-compiling) mutants in Report.Skipped slice so that computeMutatorStats can include them in the per-mutator breakdown, making the breakdown consistent with the headline MSI which already counts skipped mutants in the numerator. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
A ruthless senior engineer tire-kicked the tool and found 8 confirmed real issues. This PR fixes all of them:
panic(err)calls inrunBuiltinExecandrunCustomExec(and main-path setup functions) replaced with clean stderr errors + appropriate exit/return codes. A bad--execpath now prints one line to stderr instead of dumping a goroutine stack trace.interpretBuiltinExitCode(outside the report mutex) intorecordMutantResult(under the mutex). Diffs no longer interleave with PASS/FAIL lines from concurrent workers.interpretBuiltinExitCodebecame a trivial two-branch expression and was inlined.--dry-runover-count undocumented: Help text and dry-run output now note the count is an upper bound — deduplication only runs during a real mutation pass.--debug.--execpath has spurious/v2/: The example path pointed to a non-existent directory; fixed.TimeOutCount,MutationCodeCoverage(never written, always 0), andTimeouted(never populated) removed fromStats/Report; README updated.--logger-agentic-jsonwas outputtingmsias 0–100 (percentage) while--logger-summary-jsonused 0–1 (ratio). Agentic JSON now uses 0–1 to match.report.Skipped []Mutantslice added;recordMutantResultpopulates it;computeMutatorStatsnow iterates it so the per-mutator table reflects skipped mutations consistently with the headline MSI.Test plan
go test ./...passes (all packages)--min-msi 75 --min-covered-msi 80on mutator and internal packages--execpath produces a single clean error line, no stack trace--logger-summary-jsonand--logger-agentic-jsonproduce the samemsivalue and scalego-mutesting --dry-run ./...output includes the upper-bound note🤖 Generated with Claude Code