fail-pattern: abort when it matches the baseline, and ship known-good patterns per harness - #22
Conversation
… patterns Closes #13. Two independent defects hit `suite.fail-pattern` in one campaign, neither a property of the repo it happened in. TOO WIDE. `'\] (test\w+)\('` matches forge's `[PASS] testFoo(` as readily as `[FAIL: ...] testFoo(`, so mutants were credited to tests that pass under them — a pure-constant assertion named as the killer of a guard mutant. The green baseline is a free oracle for this: nothing failed there, so anything the pattern captures out of baseline output is a passing test. `mutation-probe` now aborts on that, with the offending captures in the message, the way a red baseline already aborts. TOO NARROW, AND SILENT. `'\[FAIL.*?\] (test\w+)\('` fixes the first and then names nobody, because `.` does not cross the newlines forge puts inside a multi-line assertion message. The verdict is unaffected (it comes from the tally), so the only symptom is an empty killer column. The baseline check cannot see this one — a pattern that matches nothing matches nothing at baseline either — so each such kill now prints "killer NOT NAMED". And `harness = "forge" | "cargo"` supplies both patterns, so a campaign stops authoring the field that carries both mistakes. Each shipped pattern is pinned by a test to real captured output of that harness under fixtures/, green and red, across two forge versions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cargo fixture had no FAILING doctest, so nothing discriminated `(.+)` from `(\S+)` in the cargo fail-pattern — the comment claimed a property the tests did not hold it to. And no end-to-end run killed a mutant with no fail-pattern configured at all, so the guard on the 'killer NOT NAMED' line was free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mutation pass survived a mutant that deletes `\([^\n]*\) \((?:gas|runs):` from the shipped forge fail-pattern: nothing in the fixtures discriminated a name on a RESULT line from any other `] name(` in forge's output. Real output has one, and campaigns hit it routinely: at `-vvv` forge prints call traces, and a reverting contract's custom error comes out as `└─ ← [Revert] Custom(1, 2)`. Without the anchor the pattern captures `Custom` and names it as the test that killed the mutant. Same red run re-captured at `-vvv` on both pinned forge versions, and the existing test strengthened to walk all four fixtures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 ai:vetter |
2 participants
Closes #13. Stacked on #10 —
mutation-probe-rs/only exists on that branch, so this targets2026-08-12-mutation-probe-bin.The killer column is the part of a mutation matrix a reader can check.
suite.fail-patternproduces it, is optional, has no default, and the only worked example in--helpwas cargo-shaped — so every Solidity campaign authored its own. It carries two opposite defects, and neither of them shows up as a failing run, because the VERDICT is read from the suite's pass/fail tally and never from this pattern. One campaign onrainlanguage/rain.sol.codegenhit both.Too wide.
'\] (test\w+)\('matches[PASS] testFoo(exactly as readily as[FAIL: …] testFoo(—captures_iterruns unanchored over the whole output and nothing there distinguishes a pass from a failure. A guard mutant insideapplyMigrationcame back killed bytestHeadGenesisIsNotZero, a pure constant assertion. The matrix read exactly like a correct one while naming tests that pass under the mutant.Too narrow, and silent.
'\[FAIL.*?\] (test\w+)\('fixes that and then names nobody:.does not cross newlines, and forge puts multi-line assertion messages inside the brackets. The issue records 87 of 95 kills unattributed in one pass and 7 of 18 in another — all correct KILLED verdicts, with an empty killer column.Three changes, one per direction plus the one that stops the field being authored at all:
baseline_defecthas already established that ZERO tests failed there, so anything the pattern captures out of baseline output is a PASSING test.mutation-probeaborts (exit 2) with the offending captures in the message, the way a red baseline already abortskiller NOT NAMEDinstead of shipping a blank cellharness = "forge" | "cargo"supplies a known-goodproofandfail-pattern, selected by the same signal that already picks the suite commandBoth patterns describe a harness's OUTPUT FORMAT — a property of forge or cargo, not of any repo — so authoring them per campaign re-derives the same two mistakes every time. Each shipped pattern is pinned by a test to real captured output of that harness under
fixtures/, green and red, across two forge versions.(?s)is not the fix, and that is measured rather than reasonedThe issue proposes
(?s)as the one-line fix for the narrow direction and flags it unverified. It was verified, against real forge output from two versions, and it is refuted —the_s_flag_fixes_multiline_messages_and_then_misattributes_invariantsholds both halves:(?s),testGeneratedSourceMatchesSnapshot— the multi-lineassertEqcase — is named again.] name(of its own: the name is printed on a later line, after a[Sequence]block.(?s)runs on past the end of that entry and stops at the next] name(in the output — which in both fixtures belongs to a[PASS]line.invariant_NeverIncrementsdrops out of the killers andtestCounterStartsAtZero, which passed under the mutant, is named in its place.Trading a blank cell for a wrong one is the worse half of the trade, so the shipped patterns are line-anchored instead of dot-all.
Decisions worth disagreeing with
prooforfail-patternoverrides the harness's, not the other way round. A repo whose suite wraps its harness — abuild.shthat reformats output — has to be able to say so, and silently preferring the shipped pattern over what the config asked for would be its own wrong matrix.[PASS] testFoo(and[FAIL: …] testFoo(differ by a few characters, and every artefact downstream of the wide pattern looks correct. A warning on a pass whose whole output is a matrix is a warning nobody reads.fail-patternwas configured. A config that never asked for attribution has no blank column to complain about, and nagging on every kill would train the reader to skip the line that matters when a pattern is configured.killedByis deduplicated, then capped. Forge prints every failing test twice — once inline, once underFailing tests:— so an undeduped list spends its cap of 5 on two names printed twice and drops the rest.assertEqof generated Solidity, a fuzz counterexample with]inside the message, an invariant name printed after a[Sequence]block, a-vvvtrace line) are not shapes anyone writes from memory correctly.What it does not do
The issue's stronger fix — capture the set of test names that PASS at baseline and at each mutant, take the difference, and delete the regex entirely — is not here. It needs structured output (
forge test --json) rather than scraped text, which is a different change to a different part of the bin: it replaces how the suite is run and read, not how its text is matched. Worth filing on its own; this PR closes the class the two shipped defects came from without pre-empting it.QA
Discriminating tests: 17 new — 12 unit (
shipped_forge_proof_reads_forges_own_tally,shipped_forge_fail_pattern_names_every_failure_and_no_passing_test,shipped_cargo_patterns_read_real_cargo_output,the_incidents_wide_pattern_is_caught_by_the_green_baseline,a_sound_fail_pattern_captures_nothing_at_a_green_baseline,the_s_flag_fixes_multiline_messages_and_then_misattributes_invariants,killed_by_is_distinct_and_capped,a_named_harness_supplies_both_patterns,explicit_patterns_override_the_harness,an_unknown_harness_names_the_ones_that_exist,without_a_harness_proof_is_still_required,every_shipped_pattern_compiles_with_the_groups_the_probe_reads) and 5 integration against the toy repo (a_fail_pattern_matching_the_baseline_aborts_before_probing,a_fail_pattern_that_names_nobody_says_so_per_kill,a_pass_with_no_fail_pattern_at_all_is_not_nagged,a_named_harness_replaces_the_hand_written_patterns,an_unknown_harness_is_a_config_abort). Suite is 39 green (27 unit + 12 integration), up from 22. The integration five drive the real bin end to end: the wide pattern aborts the pass at exit 2 with its baseline captures in the message and writes no report at all; the narrow one still scores KILLED and sayskiller NOT NAMED;harness = "cargo"alone, with neither regex written out, drives a full pass and names the killer.Mutations applied: 14 self-mutants over the new logic and the shipped patterns, run BY
mutation-probeitself (suite =nix develop -c cargo test --offline --no-fail-fast, baseline asserted green at 39 passed before probing). 14/14 killed, every one naming the test that kills it.tests::prefixes are unit tests, bare names integration:tests::a_sound_fail_pattern_captures_nothing_at_a_green_baseline,tests::the_incidents_wide_pattern_is_caught_by_the_green_baseline, +3a_fail_pattern_matching_the_baseline_aborts_before_probingtests::killed_by_is_distinct_and_capped,tests::shipped_forge_fail_pattern_names_every_failure_and_no_passing_test, +1tests::killed_by_is_distinct_and_cappedtests::a_named_harness_supplies_both_patterns,a_named_harness_replaces_the_hand_written_patternstests::a_named_harness_supplies_both_patterns,a_named_harness_replaces_the_hand_written_patternstests::explicit_patterns_override_the_harnesstests::an_unknown_harness_names_the_ones_that_exist,an_unknown_harness_is_a_config_abort, +2tests::shipped_forge_fail_pattern_names_every_failure_and_no_passing_test,tests::the_s_flag_fixes_multiline_messages_and_then_misattributes_invariantstests::shipped_forge_fail_pattern_names_every_failure_and_no_passing_test,tests::the_s_flag_fixes_multiline_messages_and_then_misattributes_invariantstests::shipped_forge_fail_pattern_names_every_failure_and_no_passing_testtests::shipped_forge_proof_reads_forges_own_tallytests::shipped_cargo_patterns_read_real_cargo_outputa_pass_with_no_fail_pattern_at_all_is_not_naggedThe survivor this pass found, and what closed it. M11 SURVIVED the first run: deleting the trailing
\([^\n]*\) \((?:gas|runs):from the shipped forge pattern changed nothing any fixture could see, so the anchor was undocumented and unpinned. The shape that discriminates it is one campaigns hit constantly — at-vvvforge prints call traces, and a reverting contract's custom error comes out as└─ ← [Revert] Custom(1, 2), which is a] name(with no metrics after it. Without the anchor the pattern capturesCustomand names a Solidity error as the killing test. Closed with the same red run re-captured at-vvvon both pinned forge versions (*-red-traces.txt),shipped_forge_fail_pattern_names_every_failure_and_no_passing_teststrengthened in place to walk all four fixtures, and the anchor's job written into the pattern's comment. Confirmed by re-running the whole 14 on the fix: 14/14.Oracle: issue fail-pattern silently misattributes killers: abort when it matches the baseline's own output, and ship known-good patterns #13's two observed defects and the harnesses' real output, never the code. Every shipped-pattern expectation is
captured_names(fixture) == the failures the run's own summary reports— forge says "4 tests passed, 7 failed" and the test asserts exactly those seven names, once each, from output where forge printed each of them twice. The(?s)hypothesis was treated the same way: run, not reasoned about, and recorded as refuted with the fixture that refutes it.Category check: covers both defects the issue names, not just the one that produced the incident, plus the authoring step that generates them (
harness). The issue's own(?s)suggestion was tested rather than adopted, and its "stronger fix" is left explicitly unbuilt rather than silently dropped. Noskills/change, so no version bump — version-hygiene CI requires one only whenskills/moves.