fix: run both mutation tools in place so their baselines see the real tree (fixes #1409) - #1410
Merged
Merged
Conversation
Run 34812562007 failed both mutation baselines before a single mutant ran, and the two failures are one class: each tool runs the suite in a copy of the project, while both suites read the real tree. cargo-mutants copies only src-tauri/. progress.test.rs reads ../src/export/PdfExportDialog.tsx, so the unmutated baseline failed with NotFound. #1210 hit the same wall and was patched with a per-test sandbox skip in command_error.test.rs. That could never close the class: on macOS the copy does not even compile, because six include_str! calls in the browser shims reach ../../../src/lib/browser/agent/ (reproduced locally, exit 4 at build). Stryker copies into .stryker-tmp/sandbox-* and symlinks node_modules in from outside it. Vite roots server.fs.allow at the sandbox, so every katex.min.css?raw import resolved to a path outside the allow list and failed with Denied ID. Running the related test set inside a sandbox shows three failing files (printDocument, copyAsHtml, pickPrintHtmlSource); CI named only the first, because the dry run bails, and Stryker's vitest runner reports only the wrapper message, not its cause. The fix removes the copy: - cargo mutants gets --in-place on the workflow invocation. 27.1.0 has no mutants.toml key for it, and the tool refuses it together with --jobs. - stryker.config.json gets inPlace, plus disableTypeChecks false so an in-place run rewrites only the mutated files instead of every src file. - mutation:ts names stryker.config.json, because with no argument Stryker discovers a config and tries stryker.conf.* first. - The #1210 skip is deleted, so the wire-fixture bond runs under mutation. Measured locally: the copied cargo-mutants baseline fails at build (exit 4); in place it passes (3117 tests, both frontend bonds included) and the one sampled mutant is caught with the source restored afterwards. Also corrects the documented narrowed local run in stryker.config.json: pnpm forwards a -- separator literally, and Stryker rejected the old command as too many arguments before doing anything.
A copy of the project is what broke both mutation baselines, and each of these edits would bring the copy back with every check still green until the next weekly run: - dropping --in-place from a cargo mutants invocation, which has no mutants.toml key to fall back on; - calling stryker run without stryker.config.json as its first argument, which lets Stryker discover a different config; - removing inPlace or disableTypeChecks false from stryker.config.json. scripts/check-mutation-in-place.test.mjs runs in the gate tier, inside check:static. It discovers every workflow and package.json script and tokenizes each script instead of grepping lines: quotes, escapes, continuations, comments, separators, redirections, command substitution, sh -c and eval. It then judges only the program each command actually runs, looking through assignments, env, time, sudo, npx, node and pnpm exec. So a flag in a comment, on a neighbouring command or in a redirection target does not count, and neither does an echoed mention. It also asserts that mutation.yml really runs the mutation:ts script it checks, and refuses to pass when it finds no invocation of either tool. Verified by mutation against the real tree: it fails on a workflow without --in-place, on mutation:ts without the config, and on a workflow whose pnpm mutation:ts became an echo or a comment. Codex found the parser gaps over three audit rounds; each one is now a failing fixture.
The vitest runner passes bail 1 to Vitest unless disableBail is set, so a red Stryker dry run names only the FIRST failing test. Run 34812562007 named one of the three test files that were failing. A dryRunOnly plus disableBail step now runs before the mutation run and names them all in one pass. disableBail stays out of stryker.config.json: it applies to every mutant run too, where stopping at the first kill is what keeps the sweep inside its timeout. The price is one extra green dry run a week, 8m04s in run 34089856980. The step names stryker.config.json explicitly, like mutation:ts, so config discovery cannot pick another file.
#1409) In place, the Stryker vitest runner writes stryker-setup-<worker>.js into the project root instead of a sandbox (measured: nine files, one per test runner process). The runner deletes them when it is disposed at the end of a run; a run that is killed can leave them behind as untracked files that are easy to commit by accident. They are ignored next to .stryker-tmp/ and reports/mutation/ now.
2 tasks
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.
Summary
Fixes #1409.
Run 34812562007 failed both mutation jobs at the baseline, before a single mutant ran. The two failures are one defect class: each mutation tool runs the test suite in a copy of the project, and both suites read the real tree. This PR removes the copy, so both tools mutate the checkout in place. A gate-tier test keeps it that way, and a non-bailing Stryker dry run now names every failing test in one pass.
Root causes
The class: a mutation tool runs the suite from a partial, relocated copy of the repository, while the tests depend on files that copy leaves out. This is the second time it has broken the Rust baseline (#1210 was the first). The first fix was a per-test skip, which could not close the class.
mutantssrc-tauri/(the cargo workspace root)pdf_export::renderer::progress::tests::every_stage_is_spelled_the_way_the_dialog_expectsreads../src/export/PdfExportDialog.tsx: NotFound, soFAILED Unmutated baseline. #1210 hit the same wall withcommand_error.test.rs. On macOS the copy does not even compile, because sixinclude_str!("../../../src/lib/browser/agent/*.src.js")in the browser shims reach outside the crate. No runtime skip or read helper can fix that.ts-mutants.stryker-tmp/sandbox-*, withnode_modulessymlinked in from outsideserver.fs.allowat the sandbox, because the copiedpnpm-workspace.yamlmarks it as the workspace root.katex/dist/katex.min.css?rawresolves to a real path outside that allow list, andisServerAccessDeniedForTransformrejects it:Denied ID …. Vitest wraps this as "There was an error when mocking a module". Stryker's vitest runner keeps onlyerrors[0].message, so the cause never reached the log.The Stryker cause was found by running the failing file inside a real sandbox. Running the whole related test set there shows three failing files:
printDocument.test.ts(7 tests), pluscopyAsHtml.test.tsandpickPrintHtmlSource.test.ts, which fail at import. CI named only the first because the runner passesbail: 1. The last green run (34089856980, 75e4c5d) predates 74f77b7. That commit addedprintDocument.tsandexportToHtmlFolder.tsand rewireduseExportOperations.ts, and all three failing files import that code. The Vitest bump in the same window (4.1.10 → 4.1.11) touched no module-loading path.What changed
.github/workflows/mutation.yml:cargo mutants --in-place ….--in-placeis CLI-only; 27.1.0'sConfighas no key for it, and the tool itself refuses it together with--jobsorCARGO_MUTANTS_JOBS. A new first step,pnpm exec stryker run stryker.config.json --dryRunOnly --disableBail, lists every failing test.disableBailstays out of the config because it would also slow every mutant run. The header records the incident, the class and the fix.stryker.config.json:"inPlace": trueand"disableTypeChecks": false. Without the second setting, an in-place run would prepend// @ts-nocheckto everysrc/**script file in the working tree, and Vitest never type-checks anyway. Comments cover recovery after a hard kill (copy.stryker-tmp/backup-*/back) and whyignorePatternsstill matter (they bound the project crawl).package.json:mutation:tsis nowstryker run stryker.config.json. With no argument, Stryker discovers a config and triesstryker.conf.*beforestryker.config.*.src-tauri/src/command_error.test.rs: deleted the Mutation testing workflow failed #1210in_mutants_sandboxskip, so the wire-fixture bond now runs under mutation too.src-tauri/.cargo/mutants.toml: the documented local command now uses--in-place, with the reason..gitignore:stryker-setup-*.js. In place, the vitest runner writes these into the project root and deletes them when a run ends; a killed run can leave them behind.scripts/check-mutation-in-place.test.mjs(new, gate tier, runs insidecheck:static): discovers every workflow andpackage.jsonscript and tokenizes each one: quotes, escapes, continuations, comments, separators, redirections,$(…)/backticks,sh -c,eval. It then requires--in-placeon every cargo-mutants invocation andstryker.config.jsonas the first argument of everystryker run. It also asserts thatmutation.ymlreally runspnpm mutation:ts, and that the config setsinPlace: trueanddisableTypeChecks: false. It refuses to pass vacuously.Audit
Codex CLI (
codex exec --sandbox read-only, reasoning effort high), three rounds.cargo mutants # --in-place,… & …--in-placeandcargo +stable mutantsthrough; replaced with a tokenizer. Stryker config discovery prefersstryker.conf.*, so both entry points now namestryker.config.json. Thegit checkout -- srcrecovery advice would discard uncommitted work; it now says to copy.stryker-tmp/backup-*/back. Refuted my own claim: I had written that--jobssilently copies the tree under--in-place. Codex showed the tool rejects the combination (verified: exit 1, clap conflict), so that claim and the redundant jobs checks were removed.--ignorePatterns stryker.config.json) now fails, because the config must be the first argument. Redirection targets (> --in-place,<<< --in-place) are no longer arguments. The "CI runs mutation:ts" check now parses commands, so a comment cannot satisfy it. Prose inside quotes is no longer rescanned as a command.env/time/sudo/npx/node/pnpm exec, soecho pnpm mutation:tsno longer passes.pnpm --dir . mutation:tsno longer false-fails. A pre-existing wrong doc line (pnpm mutation:ts -- --mutate …) was corrected: pnpm forwards--literally and Stryker exits 1 with "too many arguments", verified.Every parser and entry-point finding is a failing fixture in the gate test. The gate was also mutation-tested against the real tree (dropping
--in-place, dropping the explicit config, turningpnpm mutation:tsinto an echo or a comment), and each edit fails it. Codex did not run a fourth round, so the round-3 fixes are verified by those fixtures and mutations, not by Codex. Its round-3 verdict on the runtime fix: "The root-cause fix is sound".Validation
Local runs were on a shared 10-core machine, with load averages between 20 and 145 from two other agents.
Rust baseline, before and after (cargo-mutants 27.1.0, the version CI installs):
cargo mutants --manifest-path src-tauri/Cargo.toml -F '<one mutant>'FAILED Unmutated baselineat build, exit 4: sixcouldn't read src/browser/../../../src/lib/browser/agent/*.src.jserrorscargo mutants --in-place --manifest-path src-tauri/Cargo.toml -F '<one mutant>'ok Unmutated baseline in 148s build + 192s test,1 mutant tested: 1 caught, exit 0. The baseline ran 3117 tests, includingfrontend_wire_fixture_stays_in_syncandevery_stage_is_spelled_the_way_the_dialog_expects. Afterwardsgit statuswas clean, so the mutant was reverted.cargo-mutants --in-place --jobs 2andCARGO_MUTANTS_JOBS=2 … --in-placeboth exit 1:the argument '--in-place' cannot be used with '--jobs <JOBS>'.Stryker, before and after (9.6.1):
.stryker-tmp/sandbox-*printDocument7 tests;copyAsHtml,pickPrintHtmlSourceat import), allError: Denied ID …/katex@0.18.4/…/katex.min.css?raw. 480 files passed.stryker run stryker.config.json --dryRunOnly --disableBail, in a separate worktree of the branchInitial test run succeeded. Ran 13880 tests in 21 minutes and 51 seconds, exit 0. All three files were in the related set. Exactly 36 files were backed up and rewritten, and afterwardsgit statuswas clean with nostryker-setup-*.jsleft behind.A full sandbox-mode Stryker dry run could not finish inside its 20-minute
dryRunTimeoutMinutesunder this load, which is why the related set was run with Vitest directly inside a real Stryker sandbox.Gates:
cargo test --manifest-path src-tauri/Cargo.tomlcargo clippy --manifest-path src-tauri/Cargo.toml --all-targets -- -D warningscargo fmt --checkscripts/check-mutation-in-place.test.mjspnpm check:predeltapnpm check:allscripts/check-cross-target.shwas not run, because notauri::testmock-runtime test was added.Confirming CI run on this branch: 34915550487 (workflow_dispatch, commit 110a583). Job logs are not readable through the API until a job finishes, so the evidence here is step timing.
ts-mutants→ Dry run, listing EVERY failing test (Stryker):completed successin 10m10s (01:01:28 → 01:11:38Z). This is the in-place, non-bailing dry run, green on Linux. The mutation step that follows was still running at 01:58Z, 46 minutes in and well past its own dry run, which would have ended the step on failure.mutants→ Run scoped mutation testing: stillin_progressat 01:58Z, 56 minutes after it started. The failing baseline in 34812562007 ended that step 4.5 minutes in (186s build + 81s test), and cargo-mutants exits as soon as a baseline fails.The mutation sweeps themselves take hours; the final scores were not waited for.