fix(gate-19): read the test file with a parser, not three regexes (#234, #239, #244) - #249
Merged
Merged
Conversation
…, #239, #244) Gate-19 is the highest-volume gate in the fleet. Its three open false-positive issues were three symptoms of one decision — reading JavaScript with regular expressions — and all three surfaced as the same sentence, "referenced only by a test that never runs", about tests that ran and PASSED in the same CI run. #234 A TRAILING COMMA before the closing paren. The body was located by stepping back from `)` over whitespace and requiring a `}`. Prettier's default and ESLint's `comma-dangle: always-multiline` put a `,` at exactly that index, so the body read as "" and the empty-body rule fired on a real, asserting test. #239 A CONDITIONAL `test.skip(true, reason)` inside an `if` guard. The discriminator was the ARGUMENT alone, but `true` is just Playwright's "skip from this point" shape — the CALL SITE carries the condition. 111 guarded call sites in the fleet against 4 genuinely unconditional ones. Worse, the remedy the gate prints is "replace the tag with @e2e exclude", so complying DELETED a true coverage claim. #244 A TAG WRITTEN INSIDE THE `test(` ARGUMENT LIST. Tag resolution only ever searched FORWARD, so a tag between the open paren and the title bound to the NEXT test in the file. On nldesign that mis-binding then met #234 on whichever test it landed on, and 34 of 190 findings came out. Two defects, one symptom — which is why the fixture asserts the BINDING and not only the count. So the file is tokenised once (comments, string contents, template contents and regex literals blanked; string delimiters kept, because "is the first argument a string literal" is the whole difference between `test.skip('t', fn)` and `test.skip(cond, 'reason')`), and a real tree of test/describe calls is built with header and body ranges. Structure questions are answered from that tree. Everything the old regexes had earned is kept and re-asserted: `rx.test(` is not Playwright, `latest(` merely ends in a name, `test.describe.skip(` must match (#212 — NOT undone), `.only`/`.serial` are not switched-off markers, and `test.beforeEach(`/`test.use(`/`test.step(`/ `test.describe.configure(` are not declarations at all. SIGNALLING. This gate returned its finding COUNT as an exit status — a byte — so 266 findings left as 10 and 256 would have left as 0, i.e. PASS (#209). The clamp that fixed the wrap made the byte carry NEITHER: a 404-finding run exited 255 while stdout said 404 (#242). The byte is now a status and nothing else — 0 pass, 1 fail, 2 error — and the count is on stdout, where the runner already reads it. A crash now reports SKIPPED (wiring), visible to --require-full-coverage, instead of a fabricated verdict; the runner also stops discarding the helper's stderr. NOT TOUCHED: the empty-diff `_pass` branch, which is #242's subject and is being fixed separately. MEASURED, root-commit-scoped, across 24 local checkouts: 8790 -> 8698 findings, -92, and every one of the 92 is a false positive removed. Not one finding was added anywhere. nldesign 190 -> 156 (exactly the 34 in #244); decidesk 991 -> 984; procest 1181 -> 1166; softwarecatalog 312 -> 291; openregister 799 -> 794; opencatalogi 51 -> 46; shillinq 284 -> 279. Unchanged where the dead findings are genuine: openconnector 412 (6 real `test.describe.skip`), openbuild 187 (36 real `test.skip('title', …)`), larpingapp 101 (23 real `test.fixme`), scholiq 102. PLANTED TRUE POSITIVES, against nldesign's real spec + real e2e suite after the fix: a scenario with no test at all, a scenario tagged only by a skipped test, and a scenario tagged only by an empty-bodied test are all still caught (156 -> 159), while a fourth planted scenario tagged by a real test in the nldesign trailing-comma layout is correctly not flagged. TESTS: 75 -> 107, all green, plus the 27 other helper suites and the 59 entry-point tests. Mutation-checked: reinstating the trailing-comma bug, the argument-only skip rule, the forward-only tag resolution, the header branch, and the count-as-exit-status each turn the right tests red — and a mutant that calls every ref live turns 25 tests red, which is the control that this fix did not simply widen the gate. One earlier mutant SURVIVED (deleting the header branch), proving that fixture could not see the branch it was meant to cover; a describe-header case was added that kills it.
Contributor
Author
Correction — the test count in this PR's description is wrongThe description and the commit message both say 75 → 107. The true number is 75 → 105. I wrote 107 from memory of an intermediate run (103) plus the two tests I added afterwards, without re-reading the runner's own line — the exact habit this gate's history is about. Nothing else in the measurements is affected: the suite is green, every other figure in the description came from a captured run, and the fleet numbers were re-verified against the merged Everything else stands: 27 other helper suites pass, 59 entry-point tests pass, and all six mutants are killed. |
This was referenced Aug 8, 2026
rubenvdlinde
pushed a commit
that referenced
this pull request
Aug 8, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…-19 liveness gap Peer review asked for a mutation standard rather than a single positive control. Seven mutants, each reintroducing one specific defect, plus an anti-widening control that reworks a log string nothing asserts on and which the suite must NOT notice — without it a suite that failed on any edit would score a perfect kill rate while being worthless. It earned its keep on the first run: 'exclude-directive-read-as-reference' SURVIVED. The fixture used '@e2e exclude <slug>' space-separated, and under that form the guarded and unguarded regexes are indistinguishable — both capture 'exclude', which contains no '#' or '::' and so resolves to no slug. The assertion had been passing while proving nothing. The guard is load-bearing only for '@e2e exclude::<slug>' and '@e2e exclude#<slug>', where the unguarded regex marks the named scenario COVERED; the fixture now uses those forms and the mutant dies. 7 of 7 killed, control survives. An unapplied mutant is reported as SKIPPED (wiring) and FAILS the run rather than counting as a kill — an anchor that has drifted means the battery measures less than it claims. Also documents a real limitation rather than leaving it to be discovered: #249 rewrote gate-19 to parse test files with a real JS parser, so it will not count an @e2e reference inside a describe.skip or an empty test body. This step reads the annotation as text and will, so its number is an UPPER BOUND on real coverage. A passing threshold here is not evidence that gate-19 would pass.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…e-19's leaked `set -e` Adopts the gate-19 / #249 signalling convention, and fixes two ways the PHP arm could have gone falsely green. 1. THE EXIT BYTE WAS THE ANSWER, AND A CRASH SHARES ITS VALUE. `--owns-document` returned 0=owns / 1=fragment. A helper that CRASHES also exits 1. Every template would have classified as a fragment, the whole PHP arm would have evaporated, and gate-38 would have reported PASS having inspected nothing. The answer now comes from STDOUT — one `--classify` call for the whole set, printing `<path>: page-root|fragment` — and a non-zero exit is a wiring failure, reported as SKIPPED with stderr KEPT rather than discarded. Also one python process instead of one per template. 2. A LATENT BUG IN main(), found by the assertion above. gate-19's block (line ~1901) turns `set -e` ON and leaves it on for every gate after it, though this script's header sets only `set -u`. The first run of the crash test did not report a falsely-green gate — it reported NO GATE AT ALL: the non-zero helper killed the entire runner mid-sweep, 21 later gates silently unreported, and the run ended on the abort guard. The call is now wrapped in `set +e` with the caller's flag restored. Tests: 2 new wiring assertions (classifier MISSING -> SKIPPED; classifier CRASHING -> SKIPPED), 18/18 green. The crash assertion is the one that found the `set -e` leak; without it that failure mode is invisible, because an aborted run's PASS lines read exactly like a clean run's. Merged origin/main rather than rebasing: the branch is shared and the fleet force-push guard is right to refuse a history rewrite.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
… not take the run down Adopts the gate-19 / #249 signalling convention for both new helpers. Both call sites started as `>> log 2>/dev/null || true`, which discards the traceback AND the failure. A crashed helper leaves an empty findings log, and an empty findings log is how these gates spell PASS — the #147 defect exactly. Exit code is now a STATUS, findings are STDOUT, stderr is KEPT in <log>.err, and a non-zero exit reports SKIPPED (wiring). Also wrapped in `set +e` with the caller's flag restored. gate-19's block turns errexit ON and leaves it on for every gate after it, though this script's header sets only `set -u`; with errexit live a failing helper never reaches its own `_skip` — it kills the whole runner mid-sweep. Measured on gate-38: 21 later gates silently unreported, the run ending on the abort guard, and the PASS lines above it reading exactly like a clean run. New suite scripts/lib/test_gate_a11y_helper_wiring.sh — 10 assertions: * POSITIVE CONTROL first: with both helpers intact, a fixture app built to fail both gates does fail both. Everything else is only meaningful because these fire. * helper MISSING -> SKIPPED, for each gate * helper CRASHING -> SKIPPED, for each gate * and, separately each time, that the run still reached its COVERAGE summary — "did not abort" cannot be folded into "said SKIPPED", because an aborted run's PASS lines are indistinguishable from a clean run's.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…e threshold never gated (#189) (#253) * fix(ci): a security failure DELETED the test tier (#194); the coverage threshold never gated (#189) #194 — gating the test tier on the security tier silently deletes all test evidence. phpunit, newman, playwright and journeydoc-capture each carried `&& needs.security.result != 'failure'`. Because composer audit queries the LIVE Packagist feed, CVE-2026-67434 against squizlabs/php_codesniffer — a code formatter that never runs in production — turned the whole fleet's test tier into 'skipped' on 2026-08-06 with no commit anywhere. A skipped job is a grey tick, not a red X, so nothing counted it and a review filed a false 'fully green' report. Option E of the issue: decouple AND render loudly. * the four test jobs no longer read needs.security.result; 'needs:' is kept for ordering only, suppressed by the existing !cancelled(). * security still blocks the merge unchanged, via the required 'quality / Quality Report' check. Nothing is weakened at the merge gate. * Quality Report gains a third gate: an ENABLED test job in state 'skipped' is the ABSENCE OF A VERDICT — it hard-fails and says so in words, and distinguishes 'tests passed, security failed' from 'tests never ran'. * scripts/assert-no-producer-deletes-a-verdict.py makes it an invariant, and closes the direction #229 left open: #229 asserted every job can REACH the required check, this asserts no job can be DELETED before it gets there. Proved live, not argued: run 31259774225 on fixture/issue-194-evidence-deletion. Identical failing test job under the two conditions — gated on security -> skipped (evidence gone, old tally GREEN) decoupled -> failure (verdict exists) new invariant -> failure ('TEST TIER NOT EXECUTED — NO VERDICT EXISTS') #189 — playwright-coverage-threshold has never gated. Three defects, all fixed: 1. below-threshold emitted ::warning:: and exited 0, so the knob was decorative. It now ::error::s and exits non-zero. 2. the metric was count(test() calls) / count(scenario headings) — two independent totals never compared to each other, which ten unrelated tests raised as much as covering ten scenarios did, and which could exceed 100% while covering nothing. Replaced with real per-scenario matching against @e2e references in gate-19's dialect. The old ratio is kept and reported as testsPerScenarioPercent, never gated on. 3. zero scenarios scored 100%. Zero enforceable scenarios is now NOT MEASURABLE and fails — a measurement that could not be taken is not a pass. Default threshold 75 -> 0 to bound the blast radius: measured across all 31 fleet callers, exactly one repo (pipelinq) enables this, and it sets its own value. Gating is now opt-in by setting a number, which is what a threshold input should mean. scripts/test-spec-coverage-gate.py extracts the shipped program out of quality.yml and runs it against fixtures — including the one #189 says cannot currently exist: coverage below threshold turning the job red. 24 assertions; the positive control neuters the gate to warning-only and 5 of them flip to FAIL, so its clean pass is a verdict. Both new scripts are wired into quality-resolve-probe.yml. A checker with no callers is not a checker. * test(ci): mutation battery for the spec-coverage gate; state the gate-19 liveness gap Peer review asked for a mutation standard rather than a single positive control. Seven mutants, each reintroducing one specific defect, plus an anti-widening control that reworks a log string nothing asserts on and which the suite must NOT notice — without it a suite that failed on any edit would score a perfect kill rate while being worthless. It earned its keep on the first run: 'exclude-directive-read-as-reference' SURVIVED. The fixture used '@e2e exclude <slug>' space-separated, and under that form the guarded and unguarded regexes are indistinguishable — both capture 'exclude', which contains no '#' or '::' and so resolves to no slug. The assertion had been passing while proving nothing. The guard is load-bearing only for '@e2e exclude::<slug>' and '@e2e exclude#<slug>', where the unguarded regex marks the named scenario COVERED; the fixture now uses those forms and the mutant dies. 7 of 7 killed, control survives. An unapplied mutant is reported as SKIPPED (wiring) and FAILS the run rather than counting as a kill — an anchor that has drifted means the battery measures less than it claims. Also documents a real limitation rather than leaving it to be discovered: #249 rewrote gate-19 to parse test files with a real JS parser, so it will not count an @e2e reference inside a describe.skip or an empty test body. This step reads the annotation as text and will, so its number is an UPPER BOUND on real coverage. A passing threshold here is not evidence that gate-19 would pass. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This was referenced Aug 8, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…224, #226, #230, #235, #236, #266) (#269) * fix(gates): nine checkers matched prose, not code — one shared scope, nine gates Every gate below decided a question about CODE by grepping the raw bytes of a file. Prose is made of the same bytes, so each one failed in BOTH directions at once — the shape first written down in #184: "a checker that greps a STRING LITERAL misses every constant and matches every comment." #191 gate-48 a REMOVED COMMENT naming `#[NoCSRFRequired]` read as a removed attribute. nldesign red for one rewritten docblock sentence. #196 gate-5 a docblock saying `#[NoAdminRequired]` is deliberately NOT used SATISFIED the auth gate. A false NEGATIVE on a security gate, and a pass leaves no log. #220 gate-31 an `<img>` in a JSDoc comment in <script> (launchpad). #235 gate-31 the same, 3 of 3 findings on openbuild. #224 gate-34 false RED on a comment AND false GREEN on window['confirm'](). #226 gate-3 a run() delegating to one helper read as a stub, and the gate was closable by an inert `$unused = 1;`. #230 gate-58 a comment WARNING AGAINST networkidle counted as a use of it. #236 gate-12 `<NcSelect[^>]*>` truncated at the `>` of `option =>`. #236 gate-32 a comment describing the `<div @click>` an element replaced scored as that `<div @click>`. #266 gate-41 a PHP comment mentioning `<html>` made a mount point a page root. ONE SCOPE, NOT NINE ------------------- scripts/lib/source_scope.py generalises the two precedents that already got this right — #184's PHP stripper (which knows `#` opens a comment but `#[` opens an attribute) and #249's gate-19 tokeniser (blank once, PRESERVE OFFSETS, keep string delimiters). Every mask returns a same-length string, so a gate can report a line number computed on the mask and read a suppression marker out of the ORIGINAL at that line — which matters because every suppression marker in this package lives in a comment. Gate-19 keeps its own copy of the JS tokeniser; a drift test asserts the two byte-identical over a corpus and over this package's own .js sources, and asserts the keyword sets equal — the corpus alone SURVIVED deleting "await" from one set, so the corpus alone was not enough. #196 SHIPS WITH A DECLARATION, NOT JUST A TIGHTENING ----------------------------------------------------- Admin-only is expressed in Nextcloud by the ABSENCE of an attribute, and absence is the only thing gate-5 reports. Closing the false negative alone would have converted it into a PERMANENT false positive on correct code, with no legitimate way to satisfy the gate. So `@auth admin-only <reason>` joins the `@spec exclude` family. Making bare absence sufficient was considered and rejected: it would empty the gate completely. MEASURED, NOT ASSUMED --------------------- - 3 fixtures from #226's table, the 4 arms from #224, the nldesign line from #191 and the larpingapp line from #230, all verbatim. - Every relaxation is paired with the true positive it must not swallow, and every wiring is covered both ways: a MISSING helper and a CRASHING helper must report SKIPPED, never PASS (#147, #245, #249). gate-5 additionally runs a positive control on the mask itself, because a mask that silently returns its input is invisible to `[ -f helper ]` and puts the gate straight back into the false negative. - A nested `<template #default>` slot regression was caught by measurement before landing: a lazy `(.*?)` ended the SFC template at the first slot close and deleted a real finding at openconnector EditMapping.vue:376. Boundaries are found by depth now, and there is a test. Closes #191, #196, #220, #224, #226, #230, #235, #266 Refs #236 (parts 1 and 2; part 3 was already fixed by #247) Supersedes #219, whose gate-12 helper is carried here with its 17 tests. * fix(gate-34,gate-48): a guard is not a second dialog, and an FQCN attribute is one Both found by MEASURING the fix rather than by reading the issues. gate-34 — 7 defects reported as 14 findings ------------------------------------------ The first cut accepted any `window.confirm` REFERENCE, called or not, so on openbuild every native dialog was reported twice: const ok = typeof window !== 'undefined' && window.confirm <- guard ? window.confirm(t('openbuild', 'Delete this automation?')) <- call A feature-detection guard is a truthiness test, not a second native dialog, and inflating a security-adjacent count is its own false report (#254: a count is not a defect count). A reference now counts only when it is an ALIAS — a binding whose call site is elsewhere and therefore invisible: const c = window.confirm counts const { confirm } = window counts x && window.confirm ? … : … does not openbuild: 7 before, 7 after, same seven lines. The anchor also lost a character it should never have had. Written `=\s*window\s*[.\[]` it CONSUMED the `window` that follows, and `finditer` returns non-overlapping matches — so `const r = window.confirm('x')` matched only the alias rule, failed it because a `(` follows, and reported NOTHING. A real call dropped by an anchor one character too greedy. It is a lookahead now, and there is a test. gate-48 — the old regex could not see a fully-qualified attribute ----------------------------------------------------------------- Running #191's arm 2 end-to-end through the runner reported PASS on a genuine removal of - #[\OCP\AppFramework\Http\Attribute\NoCSRFRequired] because the pre-fix pattern alternated on the literal `#[NoCSRFRequired]`. A false NEGATIVE hiding behind the false positive #191 reported — the same both-ways failure as every other gate in this change. The new bracket-bounded rule matches it. Refs #191, #224 * fix(source_scope): `</script bar>` ends a script, and the mask must know it CodeQL raised py/bad-tag-filter (HIGH) against this branch, and it is right. r'<script(\s[^>]*)?>(.*?)</script\s*>' does not match `</script bar>` or `</script\t\n foo>`, both of which an HTML parser treats as the end of the element. When the close is spelled that way the block regex fails to match AT ALL, the script body is never comment-masked, and a JSDoc `<img>` inside it is scanned as markup — #235 reintroduced by the mask written to fix it. `</style …>` had the same hole.⚠️ THE FIRST TEST FOR THIS SURVIVED THE MUTANT. It exercised `vue_markup_mask`, which keeps `<template>` spans and never goes through `_SCRIPT_BLOCK` at all, so reverting the regex changed nothing and the suite stayed green. The assertion now runs through `html_markup_mask` and `script_mask`, the two functions that actually use it, and the reverted regex kills both. A mutation test that does not kill is not evidence — it is a second thing to check. Refs #235
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…o full of markup, and three reported PASS over a crashed checker (#272) * fix(gates 35,40,42,44): four a11y gates excused themselves from a repo full of markup, and three reported PASS over a crashed checker Measured at package sha cdfbd7a against opencatalogi (93 .vue) and nldesign (zero .vue, one PHP template), one textbook true positive planted per gate in BOTH — the asymmetry that made #225/#261 possible. All 11 gates in the 34-44 band fired and named the plant in both arms, and all returned to their exact prior verdict on removal. Two defects survive that. 1. FOUR GATES GO `na` ON A TEMPLATES-ONLY REPO Gates 35, 40, 42 and 44 still guarded on `[ -d src ]` while 34/36/37/39/43 had moved to `_a11y_has_markup_dir`, and the central applicability table listed the whole family under `[ -d src ]`. On a repo with a `templates/` full of markup and no `src/`, same run, same files: gate-34/36/37/38/39/41/43 ran; four of them FAILED on the plants gate-35/40/42/44 NOT APPLICABLE — "this repo ships no frontend, so there is no .vue/.js/.ts source for this gate to inspect" `na` is the one verdict that removes a gate from coverage accounting, and the reason was contradicted by the same run's own output three lines above it. No fleet app is templates-only today; nldesign is one `rm` away, since its `src/` holds a single `manifest.json` — the exact shape that made twelve gates pass over nothing in #225. The guards now call `_a11y_has_markup_dir`, and the applicability declaration calls THE SAME FUNCTION rather than restating it, so the two cannot drift again. No third scope definition was added. 2. A CRASHED CHECKER REPORTED PASS (#147 / #249) — gates 40, 42, 44 With a `python3` on PATH that exits 1 on every call, run against opencatalogi: gate-40 PASS gate-42 PASS gate-44 PASS <- the three inline ones gate-34/37/38/39/41/43 SKIPPED (wiring) <- the six behind a helper gate-40 printed PASS over the 13 real findings it had reported one run earlier. gate-40 discarded its status with `2>/dev/null || true`; 42 and 44 ran per-file inline heredocs and never had one. 42 and 44 move to scripts/lib/check_link_text.py and scripts/lib/check_autocomplete.py — one interpreter for the whole file set, findings on stdout, exit code as a status — and 40 gains the same return-code guard. FOUND WHILE WRITING THE TESTS * gate-44 judged an input on the FIRST of name/id/v-model and stopped, so `<input id="e" type="text" name="email">` — the plainest textbook case this gate has — passed. Fleet effect, measured across 15 repos: openregister 0 -> 1 (an OpenAI Organization ID field), pipelinq 4 -> 5 (a "Colleague email" field). Both genuine, nothing lost. * gates 35, 36 and 44 read attribute values out of DOUBLE QUOTES ONLY. `tabindex='5'`, `alt=''` and `name='telephone'` render identically and reported PASS in both arms. Zero occurrences in the fleet today, which is why they could sit there indefinitely. * `[^>]*` in gates 42 and 44: a `>` inside an attribute value is not the end of a tag — the parse that hid 19 buttons from gate-39 (#259, #198, #236). * gates 42 and 44 scanned RAW text, so a commented-out `<a>click here</a>` or `<input name="email">` counted. That is gate-64's defect (#184), the one gate-38 (#247) and gate-41 (#266) each shipped a fix for. MEASURED AFTER, NOT ONLY BEFORE * 15 repos, gates 34-44, before vs after: every verdict and every finding count identical except the two new gate-44 true positives above. The rewrites of 42 and 44 removed nothing. * opencatalogi and nldesign return to their exact pre-plant baselines. * ARM 4 of test_gate_a11y_markup_scope.sh was mutation-checked: reverting gate-42's guard to `[ -d src ]` turns it red with the finding it was written for. TESTS * scripts/lib/test_check_link_text.py, test_check_autocomplete.py — 32 assertions; every relaxation ships with the true positive it must not swallow, comment/script exclusions ship with their positive control, and each ends with the whole PRE-FIX checker replayed as the mutant, asserting it answers DIFFERENTLY on every fixture. * test_gate_a11y_helper_wiring.sh gains gates 39, 40, 42, 44 (39 was wired correctly but never listed, so nothing held it to that) — 70 assertions. * test_gate_a11y_markup_scope.sh gains ARM 4, the templates-only repo. * Full discovered suite: 49 passed, 0 failed, 2 pre-existing quarantines. tests/test-hydra-gates-bin.sh: 59 passed, 0 failed. * fix(test): SC2194 — the case word was the constant, not the subject `case " 38 45 " in *" ${_g} "*)` matches a constant against a pattern built from the variable, which is the comparison written backwards. It happened to work, and ShellCheck is right that it reads as a mistake. Verified with shellcheck 0.10.0 at full severity: clean.
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 #234. Closes #239. Closes #244.
The three issues were one decision
Gate-19 reads JavaScript. It was reading it with regular expressions and a
hand-rolled paren walk, and all three open false-positive issues came out of
that — reported as the same sentence, "referenced only by a test that never
runs", about tests that ran and passed in the same CI run.
)over whitespace and requiring a}. Prettier's default and ESLint'scomma-dangle: always-multilineput a,at exactly that index, sobodystayed""and the empty-body rule fired on a real, asserting test.trueis just Playwright's "skip from this point" shape — the call site carries the condition. 111 guarded call sites in the fleet against 4 genuinely unconditional ones.What #244 actually was
The issue guessed "the search runs forward, so it either finds the next
test's declaration or runs off the end". The first half is exactly right and
it is the whole mechanism. nldesign writes every tag between the open paren
and the title:
A forward-only search binds each tag to the next test in the file. That
mis-binding then met #234 on whichever test it landed on — every one of
those declarations ends
},\n)— so the wrong test also read as an emptybody. Two defects, one symptom, which is why the fixture asserts the
binding directly and not only the count.
Why #239 is worse than an ordinary false positive
The remedy the gate prints is "replace the tag with a reason-bearing
@e2e exclude". Complying with a false #239 finding therefore deletes atrue coverage claim and permanently marks a genuinely-tested scenario as
untestable. The gate was pushing the codebase in the wrong direction.
The fix: parse, don't grep
Tokenise the file once — comments, string contents, template contents and
regex literals blanked, offsets and newlines preserved so tags found in the
original text locate into the structure. String delimiters are kept,
because "is the first argument a string literal" is the whole difference
between
test.skip('title', fn)(a switched-off declaration) andtest.skip(cond, 'reason')(a statement in a running test).Then build the real tree of
test/describecalls with header and bodyranges, and answer structure questions from it.
Everything the old regexes had earned is kept and re-asserted:
rx.test(is not Playwright'stest();latest(/submit(merely end in aname;
test.describe.skip(must match — #212 is NOT undone;.only/.serial/.parallelare not switched-off markers; andtest.beforeEach(/test.use(/test.step(/test.setTimeout(/test.describe.configure(are not declarations at all.Signalling
This gate returned its finding count as an exit status — a byte — so 266
findings left as 10, and 256 would have left as 0, read as PASS (#209).
The clamp that fixed the wrap made the byte carry neither: a 404-finding
run exited 255 while stdout said 404 (#242). Two numbers for one
measurement means one came through a lossy channel.
The byte is a status now and nothing else — 0 pass, 1 fail, 2 error — and
the count is on stdout where the runner already reads it. A crash reports
SKIPPED (wiring), visible to--require-full-coverage, instead of afabricated verdict; the runner also stops discarding the helper's stderr.
Verified end to end through the real runner:
FAIL — 159 scenario(s)withexit 1, and a planted crash yields
SKIPPED (wiring) — check_e2e_coverage.py exited 2 (error).Not touched
The empty-diff
_passbranch. That is #242's subject and is being fixedseparately — I only renamed its literal
0toEXIT_PASS.Measured — 24 local checkouts, root-commit-scoped
--scope-to-diff --base <root-commit>. 8790 → 8698 findings, −92. Everyone of the 92 is a false positive removed; not one finding was added
anywhere.
test.describe.skiptest.skip('title', …)test.fixmePlanted true positives — the gate still catches real gaps
Against nldesign's real spec and real e2e suite, after the fix (baseline
156):
missing @e2e)trailing-comma layout → correctly not flagged
156 → 159. The fix narrows nothing.
Tests
75 → 107, all green, plus the 27 other helper suites and the 59
entry-point tests.
Mutation-checked — reinstating each defect turns the right tests red:
_ref_is_livereturns True always (the anti-widening control)suite green, because a
test()header has no children so the fallbackreturned the same node. That fixture could not see the branch it was meant to
cover. A describe-header case was added, and it kills the mutant.
🤖 Generated with Claude Code