Skip to content

fix(scripts): confine the mutation-test run and name a crashed one - #653

Merged
JeroenSoeters merged 7 commits into
mainfrom
clanker/PLA-699
Aug 21, 2026
Merged

fix(scripts): confine the mutation-test run and name a crashed one#653
JeroenSoeters merged 7 commits into
mainfrom
clanker/PLA-699

Conversation

@clanker-pel

@clanker-pel clanker-pel Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Summary

gremlins is scoped to a directory subtree, not a package. mutation-test-changed.sh passes one changed package per invocation, but gremlins walks that directory recursively: internal/cli holds a single non-test Go file and 135 beneath it, and gremlins prints paths relative to its walk root — which is why a run that never touched the login code reported login/ledger.go. The testability filter was recursive too, admitting a package because some descendant had unit-tagged tests.

(The outright job deaths originally described on this PR turned out to be a separate defect — a runaway mutant allocating without bound and OOMing the runner — root-caused and fixed independently in #665. This PR is the selection-scope and legibility half: it makes runs small and their failures nameable.)

Three changes to the script:

  • Confine the run to the invoked package. --exclude-files '/' drops every walked path naming a descendant directory. Walked paths come from a filesystem rooted at the invoked directory and are always slash-separated, so a path holding a separator is by construction in a sub-directory — a different package.
  • Tell a crashed run from a cancelled one. With no report, gremlins' exit status is all there is to distinguish a cancelled run (exits 0, writes nothing) from one that ended before it could write. The row now names the status instead of calling both no output. Classification is on exit status only, never on text in gremlins' log.
  • Scope the testability filter to the package's own test files. Implemented by enumerating the directory's own *_test.go rather than by dropping grep -r.

Tagging the tests that carried no build constraint

Scoping the filter exposed ten test files — seven in internal/metastructure, one in internal/schema, two in internal/cli/tui — carrying no //go:build line at all. Such a file builds under every tag set, so its tests do run under go test -tags unit, but the scoped filter would no longer select its package. They are tagged unit so those packages stay selected; this is load-bearing for the scoping, not incidental cleanup.

Rebased over #667

The branch is updated over main and now carries the module-root path fix (#667) inside this PR's rewrite of the script: a changed package that is itself a module root is addressed as ., with the regression test preserved in this suite.

clanker-pel Bot added 5 commits August 16, 2026 05:39
gremlins mutates the whole directory subtree below the package it is
invoked on, not just that package. The PR mutation job runs it once per
changed package, so an invocation for a shallow package near the top of a
tree pulls in everything beneath it: internal/cli holds one non-test Go
file and mutating it walks 135. The job spent most of its time on code the
PR never touched, and reported surviving mutants against packages that were
not part of the change.

Pass --exclude-files '/' so gremlins drops every walked path that names a
descendant directory. Walked paths come from a filesystem rooted at the
invoked directory and are always slash-separated, so a path holding a
separator is by construction in a sub-directory, which for Go source means
a different package.

This narrows what is mutated, not what is covered — coverage is still
gathered over the whole subtree, and gremlins offers no flag to narrow it.
A changed package that produced no gremlins report was always reported as
"no output", which reads as a cancelled run — gremlins exits 0 and writes
nothing when it is cancelled. A run that died before writing its report
exits non-zero, and the summary table gave a reader no way to tell the two
apart.

classify_result now takes the run's exit status and names it in the reason
when there is no report, leaving the cancelled shape and the missing-tool
case as they were. The status is the only discriminator: gremlins carries
the coverage run's own output when coverage fails, so a unit test that
panics puts panic text in gremlins' output without gremlins having died.
The PR mutation job asks whether a changed package has unit-tagged tests
with a recursive grep, so a package is selected whenever anything beneath
it carries a `//go:build unit` test. The repository root holds one non-test
Go file and no unit-tagged test of its own, yet it is selected: a change to
that file puts the root package in front of gremlins, whose coverage run
then covers the whole root module.

List the package's own test files with a glob and hand those to grep by
name, so tests belonging to a sub-package no longer make their parent look
mutable. A directory with no test files is answered before grep is reached,
because a grep given a pattern and no file to read would take the run's
stdin, and the glob runs in a subshell so nullglob stays out of the rest of
the script.

The matching is unchanged — it still looks for the literal `//go:build
unit` — only the set of files it reads is narrower.
The pin already noted that the mutation-test classifier reads this tool's
failure semantics. Two more facts belong beside it, both otherwise a
source-reading exercise for the next person: the script depends on
--exclude-files matching an unanchored Go regex against walk-relative
slash-separated paths, and v0.6.0 is the newest release, so the pin is
not liftable by bumping. Note the shutdown defect the version carries as
well, so a crashed job with no report is recognisable rather than
puzzling.
Ten test files in internal/metastructure, internal/schema and
internal/cli/tui carried no //go:build line, so they built under every
tag set instead of the unit tag their tests actually run under.

The mutation-test package filter selects a package on its own
unit-tagged tests, so these three packages read as having no unit tests
at all — internal/metastructure most notably, whose own tests cover the
correctness-critical core. Tagging the files brings them into line with
the convention the rest of the suite already follows and restores the
selection: 65 packages to 68.
@JeroenSoeters

Copy link
Copy Markdown
Collaborator

Root-caused the check's failures with a local reproduction and fresh runner evidence; this PR's confinement turns out to be the fix for the actual root cause, not a symptom patch.

The chain: gremlins mutates the whole directory subtree below the package it is invoked on (a local run on ./internal/cli immediately mutates agent/, app/, apply/, ...), and the script invokes it once per changed package directory, so a CLI-heavy PR mutates the whole CLI subtree several times over. That sustained multi-worker workload kills the hosted runner VM at ~25 minutes: the newest failing run carries the annotation "The runner has received a shutdown signal", with the step cancelled and no timeout annotation. The famous panic: send on closed channel is gremlins v0.6.0 mishandling the SIGTERM of a VM that is already dying, i.e. noise during the death, not its cause (worth an upstream report, tracked separately).

Quantified: with this PR's --exclude-files '/', gremlins on ./internal/cli goes from 677+ mutants and still running after 6+ minutes locally (CI died at ~25 minutes) to 38 mutants in 5.7 seconds.

Two things worth noting for the record: the coverage phase still runs over the whole subtree per invocation, so ancestor/descendant selections do redundant coverage work (cheap, follow-up material); and the PR title undersells the change, since confining the run is what stops the crashes from happening at all.

@JeroenSoeters

Copy link
Copy Markdown
Collaborator

Found while triaging the mutation-check reds after the memory-cap merge: when a changed package is itself a module root (e.g. pkg/plugin), the relative-path computation in run_package breaks. rel_pkg is built as ${REPO_ROOT}/${pkg#/} and then stripped with ${rel_pkg#"$module_root"/}; for a module-root package the string equals $module_root exactly, the trailing-slash pattern never matches, and the absolute path survives. gremlins is then invoked as ./home/runner/work/formae/formae/pkg/plugin and fails with [setup failed] ... impossible to executeCoverage. This is what keeps #621's check red (it touches pkg/plugin), and this branch carries the same computation. The fix is to run gremlins with . when the package equals the module root.

@JeroenSoeters
JeroenSoeters merged commit ba68bac into main Aug 21, 2026
30 of 31 checks passed
@JeroenSoeters
JeroenSoeters deleted the clanker/PLA-699 branch August 21, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant