diff --git a/CHANGELOG.md b/CHANGELOG.md index 480e95f7..c659c12c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [0.10.0] - 2026-08-30 +### Fixed + +- **A change with nothing mutable in it no longer fails `ditto changed`.** The + score calculator reports `-1` for an empty run, which is below every threshold, + so a scope that produced no mutants and a suite that failed were the same + answer. `Run` and `RunStaged` keep that refusal — a repository or an index + yielding nothing is a scope somebody configured wrong — but a diff is not: a + new `switch` case, a comment, a rename are all legitimately unmutatable, and + failing a gate for one is failing a change for being correct. + + Found by this repository's own gate the day after it went green, on the `-v` + commit below: one added `switch` case, a scope of one file, zero mutants, and a + red run that had found nothing wrong with anything. + + The count is read through an optional interface, and the reporter decorators + now forward it. Without that it was unreadable exactly when the stack is + deepest, which is the gated run that raised the problem — backlog entry 12's + class, caught again. + ### Added - **`-v` and `--version` alongside `ditto version`.** Nothing new is reported; diff --git a/changed.go b/changed.go index b87f139b..ebb91cf7 100644 --- a/changed.go +++ b/changed.go @@ -1,7 +1,9 @@ package ditto import ( + "errors" "fmt" + "os" "github.com/Disble/ditto/internal/staged" ) @@ -80,5 +82,21 @@ func RunChanged(directory, baseRef string, excludePrefixes []string, options ... return fmt.Errorf("checking the checkout: %w", err) } - return runInSandbox(directory, plan, options) + err = runInSandbox(directory, plan, options) + + // A change CAN legitimately contain no mutable expression -- a new switch + // case, a comment, a rename -- and a gate that fails for that is failing a + // change for being correct. Measured on ditto's own gate the day after it + // went green: one added switch case, a scope of one file, zero mutants, and + // a red run that had found nothing wrong with anything. + // + // Run and RunStaged keep the refusal. A repository or an index that yields + // nothing is a scope somebody configured wrong; a diff is not. + if errors.As(err, &NoMutantsError{}) { + fmt.Fprintf(os.Stdout, "ditto: nothing in the change since %s is mutable, so there was nothing to score.\n", baseRef) + + return nil + } + + return err } diff --git a/changed_scope_test.go b/changed_scope_test.go index 4749ebd4..5f11dd2d 100644 --- a/changed_scope_test.go +++ b/changed_scope_test.go @@ -147,3 +147,30 @@ func TestRunChangedRefusesSomewhereThatIsNotARepository(t *testing.T) { t.Fatal("a directory outside any repository was accepted") } } + +// A change can legitimately contain no mutable expression — a new switch case, a +// comment, a rename. Failing a gate for that is failing a change for being +// correct, and it happened on ditto's own gate the day after it went green: +// one added switch case, a scope of one file, zero mutants, and a red run that +// had found nothing wrong with anything. +func TestRunChangedAcceptsAChangeWithNothingMutableInIt(t *testing.T) { + dir := dittotesting.GitRepository(t) + + dittotesting.WriteFile(t, dir, "plain.go", "package fixture\n\n// Plain does nothing an operator could be flipped in.\nfunc Plain() {}\n") + dittotesting.Git(t, dir, "add", "-A") + dittotesting.Git(t, dir, "commit", "-m", "plain") + + // Both stacks. Gated is the one CI builds, and it is the one where the count + // was unreadable: the gated reporter wraps the console reporter, and a + // decorator that drops a capability is refused by nothing. + for name, options := range map[string][]ditto.Option{ + "plain": nil, + "gated": {ditto.Gated()}, + } { + t.Run(name, func(t *testing.T) { + if err := ditto.RunChanged(dir, "base", nil, options...); err != nil { + t.Fatalf("a change with nothing mutable in it was reported as a failure: %v", err) + } + }) + } +} diff --git a/internal/consolereporter/consolereporter.go b/internal/consolereporter/consolereporter.go index b31d5ce7..5967c456 100644 --- a/internal/consolereporter/consolereporter.go +++ b/internal/consolereporter/consolereporter.go @@ -17,6 +17,13 @@ type ConsoleReporter struct { calculator ditto.ScoreCalculator minimumThreshold float32 diagnostics []*ditto.Diagnostic + + // total is what the last Summarize scored, after the mutants that never + // compiled were removed from both sides. Zero and "below threshold" are + // different answers -- a scope with nothing mutable in it is not a suite + // that failed -- and the score alone cannot tell them apart, because the + // calculator reports -1 for both an empty run and nothing else. + total int } func New( @@ -34,6 +41,11 @@ func New( } } +// Total is how many mutants the last Summarize scored. It is read through an +// optional interface, the way the temporary directory's RemoveAll and the gated +// laboratory's counters are, so no reporter is forced to answer. +func (r *ConsoleReporter) Total() int { return r.total } + func (r *ConsoleReporter) AddDiagnostic(diagnostic *ditto.Diagnostic) { r.diagnostics = append(r.diagnostics, diagnostic) } @@ -64,6 +76,7 @@ func (r *ConsoleReporter) Summarize() result.Result[any] { } total := killed + survived + r.total = total // Addresses first, diffs after. Survivors are the only part of this report // anybody acts on, and printing them after the diffs turned the report into diff --git a/internal/consolereporter/total_test.go b/internal/consolereporter/total_test.go new file mode 100644 index 00000000..c159338f --- /dev/null +++ b/internal/consolereporter/total_test.go @@ -0,0 +1,73 @@ +package consolereporter_test + +import ( + "testing" + + "github.com/Disble/ditto/internal/consolereporter" + "github.com/Disble/ditto/internal/ditto" + "github.com/Disble/ditto/internal/dittotesting/fakelogger" + "github.com/Disble/ditto/internal/dittotesting/fakescorecalculator" + "github.com/Disble/ditto/internal/dittotesting/stubdiffer" + "github.com/Disble/ditto/internal/future" + "github.com/Disble/ditto/internal/gatedreporter" + "github.com/Disble/ditto/internal/gomutatedfile" + "github.com/Disble/ditto/internal/result" + "github.com/Disble/ditto/internal/verbosereporter" + "github.com/stretchr/testify/assert" +) + +// A scope with nothing mutable in it and a suite that failed are different +// answers, and the score cannot tell them apart: the calculator reports -1 for +// an empty run, which is below every threshold. The count is what separates +// them. +func TestTotalSeparatesAnEmptyRunFromAFailingOne(t *testing.T) { + reporter := consolereporter.New(fakelogger.New(), stubdiffer.New(""), fakescorecalculator.Always(0), 0) + + assert.Equal(t, 0, reporter.Total(), "a reporter that has scored nothing counts nothing") + + reporter.AddDiagnostic(ditto.NewDiagnostic( + future.Resolved(result.Ok("mutant killed")), + gomutatedfile.New("dummy", "dummy.go", nil, nil), + )) + reporter.Summarize() + + assert.Equal(t, 1, reporter.Total()) +} + +// TestTheDecoratorsForwardTheCount is backlog entry 12's class, caught once +// already: a decorator that drops a capability is refused by nothing. Without +// this the count is unreadable exactly when the stack is deepest. +func TestTheDecoratorsForwardTheCount(t *testing.T) { + logger := fakelogger.New() + console := consolereporter.New(logger, stubdiffer.New(""), fakescorecalculator.Always(0), 0) + + console.AddDiagnostic(ditto.NewDiagnostic( + future.Resolved(result.Ok("mutant killed")), + gomutatedfile.New("dummy", "dummy.go", nil, nil), + )) + console.Summarize() + + // Both decorators, and the gated one over the verbose one, which is the + // stack the CI gate actually builds and the one the count was unreadable in. + for name, wrapped := range map[string]ditto.Reporter{ + "verbose": verbosereporter.New(logger, console), + "gated": gatedreporter.New(logger, noGates{}, console), + "gated over verbose": gatedreporter.New(logger, noGates{}, verbosereporter.New(logger, console)), + } { + t.Run(name, func(t *testing.T) { + counted, ok := wrapped.(interface{ Total() int }) + if !ok { + t.Fatalf("the %s reporter does not forward the count", name) + } + + assert.Equal(t, 1, counted.Total()) + }) + } +} + +// noGates is a gated laboratory that gated nothing, which is all this needs: the +// counters are not the subject here, the forwarding is. +type noGates struct{} + +func (noGates) Gated() int { return 0 } +func (noGates) FellBack() int { return 0 } diff --git a/internal/gatedreporter/gatedreporter.go b/internal/gatedreporter/gatedreporter.go index 4ec43454..0d137de1 100644 --- a/internal/gatedreporter/gatedreporter.go +++ b/internal/gatedreporter/gatedreporter.go @@ -66,3 +66,18 @@ func count(gated int) string { return strconv.Itoa(gated) } + +// Total forwards the count the reporter beneath scored. +// +// A decorator that drops a capability is refused by nothing -- backlog entry 12, +// and the reason this is written twice rather than assumed once. Without it the +// count is unreadable exactly when the stack is deepest, which is the gated run +// that found the problem in the first place. +func (r *GatedReporter) Total() int { + counted, ok := r.delegate.(interface{ Total() int }) + if !ok { + return -1 + } + + return counted.Total() +} diff --git a/internal/verbosereporter/verbosereporter.go b/internal/verbosereporter/verbosereporter.go index 45c711ce..d3ed122c 100644 --- a/internal/verbosereporter/verbosereporter.go +++ b/internal/verbosereporter/verbosereporter.go @@ -27,3 +27,18 @@ func (r *VerboseReporter) Summarize() result.Result[any] { return r.delegate.Summarize() } + +// Total forwards the count the reporter beneath scored. +// +// A decorator that drops a capability is refused by nothing -- backlog entry 12, +// and the reason this is written twice rather than assumed once. Without it the +// count is unreadable exactly when the stack is deepest, which is the gated run +// that found the problem in the first place. +func (r *VerboseReporter) Total() int { + counted, ok := r.delegate.(interface{ Total() int }) + if !ok { + return -1 + } + + return counted.Total() +} diff --git a/perf/baseline.json b/perf/baseline.json index 9a892715..01214907 100644 --- a/perf/baseline.json +++ b/perf/baseline.json @@ -17,7 +17,7 @@ "laboratoryRunsForOneChangedFunction": 4, "laboratoryRunsForOneChangedFunctionInEachOfTwoFiles": 8, "testCommandInvocationsPerReleaseWholeFixture": 49, - "mutantsPerReleaseOnThisRepository": 785 + "mutantsPerReleaseOnThisRepository": 789 }, "targets": { "sourceParsesPerReleaseWithThreeViruses": "Reached: 4, one parse per source file, down from 12. GoSourceFile.Incubate now takes the whole mutator set and parses once for all of them. With the default 14 mutators this is 14 parses per file reduced to 1.", @@ -28,6 +28,6 @@ "laboratoryRunsForOneChangedFunction": "4, the mutators that fire on one changed line and nothing else in the repository. This is what WithChangedRanges buys: without it the same fixture charges 48.", "laboratoryRunsForOneChangedFunctionInEachOfTwoFiles": "8, exactly twice the single-file number. The two ranges name offsets that exist in both files, because every fixture file has the same byte layout, so a scope holding one flat set of ranges would charge 16 and grow as the square of the file count. Keeping the ranges beside their file makes that impossible rather than merely unlikely.", "testCommandInvocationsPerReleaseWholeFixture": "49 for the 48 mutants of the whole fixture, plus one. That one is the baseline: the laboratory runs the suite once on unmutated code before scoring anything, because a test command that fails before it compiles fails for every mutant too, and ditto recognises a killed mutant by exactly that. Measured on ditto's own gate before the guard existed: 431 of 431 killed in 5.46 seconds, a perfect score for a run that compiled nothing. Every other laboratory counter here goes through a stand-in and cannot see a run the laboratory makes on its own, which is why this one exists — a cost nobody records is one that grows unnoticed, the mirror of the unrecorded gain this file already refuses. It must not grow: one baseline per release, never one per mutant.", - "mutantsPerReleaseOnThisRepository": "727, what one full run of the gate has to pay for. It was 660 before internal/verdict, and 684 before verdict.Text and the wiring, all on 2026-08-28; the ratchet fired on that commit and the number was written down rather than discovered later. Every other counter here is measured against the six-file fixture, which does not change when the repository does -- so all eight stayed green while the real cost grew from 431 mutants to 660 between 2026-08-15 and 2026-08-28 and pushed the gate past its 30-minute timeout. Measured on the two CI logs: cost per mutant moved 2.269s to 2.435s, seven percent, while the count moved forty-three; and every new mutant belonged to a file that did not exist in August -- internal/staged (133), cmd/ditto/main.go (59), staged.go (21), internal/filecopy (9). No old file grew. This counter is what perfbench's own doc comment already promised in prose: 'how many mutants a scope produces -- these are integers, identical on every machine, and a change in one is always meaningful.' It was the one number in that sentence nothing measured. It grows when code is added, and that is the point: the growth is real cost and has to be written down rather than discovered by a timeout. Moved to 734 on 2026-08-30 by backlog entries 22-26: +4 for the command line (the version subcommand, staged gating and the option builder they share) +3 for saying what a run is doing (internal/progresslaboratory and the baseline announcement), and +2 for confirming assertion kills (internal/confirminglaboratory). Each part was measured as it landed rather than one number attributed to the whole change afterwards. Then 736 to 783 (+47) for the range scope that closes backlog 21 -- changed.go, internal/staged/changed.go and the changed subcommand. That the change which SHRINKS the gate grows this counter by 47 is not a contradiction, it is the entry: this number is what the repository-sized question costs, and the gate no longer asks it on every push. Then 784, a net +1, when RunStaged and RunChanged were folded onto one runInSandbox and internal/dittotesting/gitrepository.go was added. The split between what the fold removed and what the new file added was not measured separately, so it is not claimed. Then 785 (+1) for resolving git to an absolute path rather than letting PATH decide, which SonarQube refused as a security rating on new code. Scoped exactly like ditto_mutation_test.go, so it speaks for the run it is named after. Counting costs no test command and no sandbox. docs/experiments/counting-the-real-repository.md." + "mutantsPerReleaseOnThisRepository": "727, what one full run of the gate has to pay for. It was 660 before internal/verdict, and 684 before verdict.Text and the wiring, all on 2026-08-28; the ratchet fired on that commit and the number was written down rather than discovered later. Every other counter here is measured against the six-file fixture, which does not change when the repository does -- so all eight stayed green while the real cost grew from 431 mutants to 660 between 2026-08-15 and 2026-08-28 and pushed the gate past its 30-minute timeout. Measured on the two CI logs: cost per mutant moved 2.269s to 2.435s, seven percent, while the count moved forty-three; and every new mutant belonged to a file that did not exist in August -- internal/staged (133), cmd/ditto/main.go (59), staged.go (21), internal/filecopy (9). No old file grew. This counter is what perfbench's own doc comment already promised in prose: 'how many mutants a scope produces -- these are integers, identical on every machine, and a change in one is always meaningful.' It was the one number in that sentence nothing measured. It grows when code is added, and that is the point: the growth is real cost and has to be written down rather than discovered by a timeout. Moved to 734 on 2026-08-30 by backlog entries 22-26: +4 for the command line (the version subcommand, staged gating and the option builder they share) +3 for saying what a run is doing (internal/progresslaboratory and the baseline announcement), and +2 for confirming assertion kills (internal/confirminglaboratory). Each part was measured as it landed rather than one number attributed to the whole change afterwards. Then 736 to 783 (+47) for the range scope that closes backlog 21 -- changed.go, internal/staged/changed.go and the changed subcommand. That the change which SHRINKS the gate grows this counter by 47 is not a contradiction, it is the entry: this number is what the repository-sized question costs, and the gate no longer asks it on every push. Then 784, a net +1, when RunStaged and RunChanged were folded onto one runInSandbox and internal/dittotesting/gitrepository.go was added. The split between what the fold removed and what the new file added was not measured separately, so it is not claimed. Then 785 (+1) for resolving git to an absolute path rather than letting PATH decide, which SonarQube refused as a security rating on new code. Then 789 (+4) for telling an empty scope apart from a failing one: the reporter now counts what it scored and two decorators forward that count. Scoped exactly like ditto_mutation_test.go, so it speaks for the run it is named after. Counting costs no test command and no sandbox. docs/experiments/counting-the-real-repository.md." } } diff --git a/release.go b/release.go index 510ba9bb..6141a9e4 100644 --- a/release.go +++ b/release.go @@ -143,12 +143,36 @@ func Run(options ...Option) error { } if !rel.summarize().IsOk() { + // A scope with nothing mutable in it is not a suite that failed, and the + // score cannot tell the two apart: the calculator reports -1 for an + // empty run, which is below every threshold. Measured on ditto's own + // gate, where adding one `switch` case -- an expression no virus can + // mutate -- produced a scope of one file, zero mutants and a failing + // release that had found nothing wrong with anything. + if rel.scored() == 0 { + return NoMutantsError{} + } + return ScoreBelowThresholdError{Minimum: rel.opts.MinimumThreshold} } return nil } +// NoMutantsError reports a run whose scope produced nothing to judge. +// +// It is deliberately still an error for Run and RunStaged: a repository or an +// index that yields no mutants is a scope somebody configured wrong, and the +// documented refusal for it stays. RunChanged treats it as nothing to do, +// because a change CAN legitimately contain no mutable expression -- a new +// switch case, a comment, a rename -- and failing a gate for that is failing it +// for being correct. +type NoMutantsError struct{} + +func (NoMutantsError) Error() string { + return "ditto: the scope produced no mutants, so there was nothing to score" +} + // ScoreBelowThresholdError reports a run that finished and did not reach its bar. // // It is not the same answer as a refusal: the mutants ran, the number is real, @@ -268,6 +292,18 @@ func (r *release) summarize() result.Result[any] { return r.reporter.Summarize() } +// scored is how many mutants the report counted, and -1 when the reporter cannot +// say. Read through an optional interface, so a decorator that does not forward +// it leaves the answer unknown rather than wrong. +func (r *release) scored() int { + counted, ok := r.reporter.(interface{ Total() int }) + if !ok { + return -1 + } + + return counted.Total() +} + // reclaim removes what a run left behind. // // Sandboxes outlive each mutant now, so removing them belongs to the run rather