fix: keep the mutant --filter under the kernel's per-argument limit - #43
Open
yeapea wants to merge 1 commit into
Open
fix: keep the mutant --filter under the kernel's per-argument limit#43yeapea wants to merge 1 commit into
yeapea wants to merge 1 commit into
Conversation
A mutant's covering tests are emitted one regex fragment per test and joined into a SINGLE `--filter=` argv element. That element is unbounded, and on a class most of a suite reaches it runs past MAX_ARG_STRLEN — PAGE_SIZE * 32, 131072 bytes on x86_64, a kernel constant `ulimit` does not move. The child then dies with `posix_spawn() failed: Argument list too long` before running a single test, so every mutation in that class is lost. Measured at 172,779 bytes for one mutation in a class with 2,076 covering tests. macOS has no per-argument cap, so it reproduces only on Linux — in practice on CI, on the run whose score people publish. Support/FilterArgument applies two encodings in order: 1. Factor the class prefix: `Cls::(.*)a|Cls::(.*)b` becomes `Cls::(.*)(a|b)`. Lossless — both forms select exactly the same tests — and 34% shorter on a four-class set. The inner parentheses are load-bearing: without them the alternation binds to the whole pattern rather than the tail, and the filter selects tests it was never given. 2. If it still does not fit, collapse a class to its bare `Cls::` prefix, heaviest first, stopping the moment it fits. That is a strict superset of the original selection, so it can only run MORE tests and can never turn a killed mutant into a survivor. A collapse is never silent: `widened` names each class and how many extra tests it now selects, and MutationTest reports it once per process rather than once per mutation. A filter that quietly widened would make the score certify more than the run measured. If even a fully collapsed filter does not fit it throws, because dropping the filter would run the whole suite per mutant and read as a fast green. Closes #1771 (pestphp/pest).
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.
Closes pestphp/pest#1771.
The defect
A mutant's covering tests are emitted one regex fragment per test and joined into a single
--filter=argv element:That element is unbounded. Linux caps one argv element at
MAX_ARG_STRLEN = PAGE_SIZE * 32(include/uapi/linux/binfmts.h) — 131,072 bytes on x86_64, and it is not tunable:ulimit -smoves the totalARG_MAX, never the per-element cap. Past it the child dies withbefore running a single test, so every mutation in that class is lost. Measured at 172,779 bytes for one mutation in a class with 2,076 covering tests.
macOS has no per-argument cap at all, which is why this reproduces only on Linux — in practice on CI, on the run whose score people actually publish.
The fix
Pest\Mutate\Support\FilterArgumentapplies two encodings, in order.1 — Factor the class prefix.
Cls::(.*)a|Cls::(.*)bbecomesCls::(.*)(a|b). Lossless, and measured on synthetic sets shaped like the real ones:The inner parentheses are load-bearing.
Cls::(.*)a|bparses as(Cls::(.*)a)|(b), so a barebmatches anywhere and the filter runs tests it was never given. There is a test for exactly that.2 — If it still does not fit, collapse a class to its bare
Cls::prefix, heaviest contributor first, stopping the moment it fits.That is a strict superset of the original selection, which is precisely why it is safe here: widening can only ever run more tests, so it can never turn a killed mutant into a survivor.
That argument has one precondition worth stating: it holds while the ordinary suite is green. A test already failing for unrelated reasons counts as a kill, so a widened filter can inherit an unrelated red — the same fabrication class as running mutation in parallel.
So a collapse is never silent.
widenednames each class and how many extra tests it now selects;MutationTestreports it once per process rather than once per mutation in the class. A filter that quietly widened would make the score certify more than the run measured.And if even a fully collapsed filter does not fit, it throws. Dropping the filter would run the whole suite per mutant and read as a fast green, which is the one outcome worse than the crash.
Why a value object rather than a method
FilterArgument::for()returnsargumentpluswidened, so the encoding stays free of any output concern and is unit-testable without spawning a process. Thefwrite(STDERR, …)at the call site is the smallest honest channel for the notice — if you would rather it went throughFacade::instance()->emitter(), say so and I will move it; it needs an event class and listener wiring, so I did not add that unasked.BUDGET_BYTESis 98,304 rather than the kernel's full 131,072: the cap is per element and a suite only grows, so a filter sized to the exact limit is one new test away from failing again.Proof
pest(whole suite)pint --testphpstan analyserector --dry-runpest --type-coverageThree behaviours were red-proved by reverting the code that guards them:
throwon a still-oversized filterit fails loudly rather than returning a filter that still does not fitPrior art
This has been running in production in two repositories for weeks — one as a
cweagans/composer-patchespatch, one as an anchored vendor rewrite — against real mutation runs. This PR is that algorithm, upstreamed, so neither has to re-derive it on every release.