From 2b99b365de3fa6a402a7e1f1b9a9c841463c86bc Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 17 Aug 2026 05:25:17 -0700 Subject: [PATCH] FN-9141: rescue plugin-runner tests and enforce quarantine lockstep Rescue the plugin-runner suite before deletion while making quarantine records mechanically consistent. - preserve logger assertions across worker-reused mock cleanup with a stable hoisted logger - remove the rescued suite from the quarantine ledger and Vitest exclusion - enforce ledger-to-exclude lockstep and cover missing or dangling quarantine entries - document the reproduction evidence, rescue disposition, and strict checker behavior Files changed: .../suite-only-flakes-observed-register.md | 14 +- docs/testing.md | 17 +- .../engine/src/__tests__/plugin-runner.test.ts | 37 ++-- packages/engine/vitest.config.ts | 14 +- scripts/__tests__/check-quarantine-ledger.test.mjs | 217 +++++++++--------- scripts/__tests__/ci-test-shard-timings.test.mjs | 5 +- scripts/check-quarantine-ledger.mjs | 245 +++++++++++++-------- scripts/lib/test-quarantine.json | 10 +- 8 files changed, 314 insertions(+), 245 deletions(-) Fusion-Task-Id: FN-9141 Fusion-Task-Lineage: 5b0549bf-3cc6-495e-bf99-a30a2dffb029 Co-authored-by: Fusion (runfusion.ai) --- .../suite-only-flakes-observed-register.md | 14 +- docs/testing.md | 17 +- .../src/__tests__/plugin-runner.test.ts | 37 +-- packages/engine/vitest.config.ts | 14 +- .../check-quarantine-ledger.test.mjs | 217 ++++++++-------- .../__tests__/ci-test-shard-timings.test.mjs | 5 +- scripts/check-quarantine-ledger.mjs | 245 +++++++++++------- scripts/lib/test-quarantine.json | 10 +- 8 files changed, 314 insertions(+), 245 deletions(-) diff --git a/docs/solutions/test-failures/suite-only-flakes-observed-register.md b/docs/solutions/test-failures/suite-only-flakes-observed-register.md index aa6d87a7a5..19bdc382b1 100644 --- a/docs/solutions/test-failures/suite-only-flakes-observed-register.md +++ b/docs/solutions/test-failures/suite-only-flakes-observed-register.md @@ -80,7 +80,7 @@ DDL microbenchmarks of the pre-fix pristine shape measured `CREATE DATABASE` 44. ## 3. Plugin runner complete-lane lifecycle hook - **File:** `packages/engine/src/__tests__/plugin-runner.test.ts` -- **Exact test:** `PluginRunner > task lifecycle hooks > should invoke onTaskCompleted when the complete lane is RENAMED` +- **Historical exact test:** `PluginRunner > task lifecycle hooks > should invoke onTaskCompleted when the complete lane is RENAMED` - **Observed tree/SHA:** PR [#2799](https://github.com/Runfusion/Fusion/pull/2799) merged-with-main. | run | result | @@ -110,6 +110,16 @@ Seven tests failed in `plugin-runner.test.ts`, but only this one identity surviv | 8 workers, run 2 (188.61s) | pass | 39 failed / 795 passed files; unrelated baseline-red | | 2 workers, run 2 (590.97s) | pass | 39 failed / 795 passed files; unrelated baseline-red | +**Rescued 2026-08-17 (FN-9141):** FN-9141 completed the terminal new-strategy campaign with shuffled files/tests, worker reuse without per-file isolation, and a temporary byte-for-byte repeated subject. The completed two-worker lane reproduced a named test-fixture defect: a neighbouring worker-reused file can call `vi.clearAllMocks()` after `PluginRunner` has initialized, erasing `createLogger.mock.results` before the hot-reload warning assertion reads it. The assertion already catches the real `stopPlugin` rejection/warning contract; the repair keeps its logger instance in a hoisted stable reference and explicitly proves cleanup cannot erase that reference. The ledger row and default-lane exclusion remain removed together. No timeout, retry, assertion weakening, skip, polling, or permanent worker-policy change was used. + +| FN-9141 new-strategy reproduction | seed | workers | isolation | duration | subject result | whole-lane result | +|---|---:|---:|---:|---:|---|---| +| shuffled, worker-reuse loaded engine-default | 914141 | 2 | disabled / worker reuse | 900.1s (campaign bound) | 82 passed | terminated before summary; unrelated `project-engine` timeouts after subject completed | +| shuffled, repeated-subject loaded engine-default | 914142 | 8 | enabled; temporary byte-for-byte subject repeat | 222.66s | original 82 passed; repeat 82 passed | complete: 48 failed / 787 passed / 1 skipped files; 117 failed / 11230 passed / 14 skipped / 1 todo tests; no subject failure | +| shuffled, worker-reuse, repeated-subject loaded engine-default | 914143 | 2 | disabled / worker reuse; temporary byte-for-byte subject repeat | 1548.60s | reproduced one hot-reload warning fixture failure; repeat passed | complete: 224 failed / 611 passed / 1 skipped files; 1801 failed / 9534 passed / 26 skipped / 1 todo tests; unrelated loaded failures also present | + +The rescue retains plugin loading, contribution accessor, runtime compatibility, hot-reload, and lifecycle-hook assertions, including the renamed-complete-lane `onTaskCompleted` dispatch that would have been uniquely lost under deletion. The direct regression now covers the worker-reused cleanup sequence that caused the reproduced warning assertion failure. + ## 4. Planning Mode direct task handoff - **File:** `packages/dashboard/app/components/__tests__/PlanningModeModal.planning-flow.test.tsx` @@ -143,7 +153,7 @@ The failure exercises the pre-existing mobile tab transition, while the task-cre ## Common shape and investigated result -FN-9125 established that entry 3 is not PostgreSQL-suite-adjacent: `plugin-runner.test.ts` uses an in-memory mocked TaskStore and has no PostgreSQL/harness import. FN-9135's bounded reproduction campaign did not identify the root cause needed to rescue it, so the suite remains quarantined with its coverage intact until the ratchet deadline. Entries 1, 2, and 7 remain evidence-gathering-pending PostgreSQL observations: current full-output runs at six workers plus a twelve-worker PostgreSQL-directory run did not reproduce any subject identity, so FN-9125 cannot claim them superseded or resolved. The golden-template/advisory-lock lifecycle and schema-applier's inline baseline path are concrete architecture facts, not a demonstrated cause of these assertions. Core policy forbids inline PG quarantine: FN-9126 owns entry 1, FN-9128 exclusively owns entry 2, and FN-9127 owns entry 7 for CI/host-specific `pg_stat_activity`, lifecycle timing, and a complete loaded failure capture before any source or fan-out change. Entry 6 instead records a merge-gate eviction after a loaded-lane setup-hook timeout; `FNXC:PgTestTemplateDb 2026-07-19-17:20` and `FNXC:PgTestWorkerCap 2026-07-18-18:00` are already-landed mitigations for that mode, not new diagnoses to re-open. The Planning Mode entries are separate frontend timing observations. +FN-9125 established that former entry 3 was not PostgreSQL-suite-adjacent: `plugin-runner.test.ts` used an in-memory mocked TaskStore and had no PostgreSQL/harness import. FN-9135 did not identify a root cause, but FN-9141's completed shuffled worker-reuse campaign reproduced and structurally fixed the logger mock-history fixture defect; the suite and its renamed-complete-lane dispatch coverage remain active. Entries 1, 2, and 7 remain evidence-gathering-pending PostgreSQL observations: current full-output runs at six workers plus a twelve-worker PostgreSQL-directory run did not reproduce any subject identity, so FN-9125 cannot claim them superseded or resolved. The golden-template/advisory-lock lifecycle and schema-applier's inline baseline path are concrete architecture facts, not a demonstrated cause of these assertions. Core policy forbids inline PG quarantine: FN-9126 owns entry 1, FN-9128 exclusively owns entry 2, and FN-9127 owns entry 7 for CI/host-specific `pg_stat_activity`, lifecycle timing, and a complete loaded failure capture before any source or fan-out change. Entry 6 instead records a merge-gate eviction after a loaded-lane setup-hook timeout; `FNXC:PgTestTemplateDb 2026-07-19-17:20` and `FNXC:PgTestWorkerCap 2026-07-18-18:00` are already-landed mitigations for that mode, not new diagnoses to re-open. The Planning Mode entries are separate frontend timing observations. ## Policy and escalation diff --git a/docs/testing.md b/docs/testing.md index 85491638bb..d3cf554e5d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -414,7 +414,7 @@ The CI job uses `fetch-depth: 0` because these tests run real git operations. ## Quarantine ledger and the deletion ratchet -Flaky tests are quarantined ON SIGHT and deleted on a 2-week clock. This is written policy with minimal mechanics — deliberately no loader module, no automation (see the AGENTS.md standing rule "Flaky Tests Are Quarantined on Sight"). +Flaky tests are quarantined ON SIGHT and deleted on a 2-week clock. The ledger remains a simple JSON record, while `scripts/check-quarantine-ledger.mjs` mechanically checks every ledger/exclude pair (see the AGENTS.md standing rule "Flaky Tests Are Quarantined on Sight"). Quarantine is the default when a sighting is reproducible enough to justify evicting a file's coverage. The only exception is the narrow first-sighting record authority in AGENTS.md: a high-value file may be recorded in the [observed suite-only flakes register](solutions/test-failures/suite-only-flakes-observed-register.md) instead (`docs/solutions/test-failures/suite-only-flakes-observed-register.md`). A second sighting of a registered flake moves it to the ledger plus matching Vitest `exclude` in one lockstep commit. @@ -430,13 +430,15 @@ Quarantine is the default when a sighting is reproducible enough to justify evic ### Quarantine deadline visibility check -Run `pnpm check:quarantine-ledger` to print a soonest-deadline-first summary of `scripts/lib/test-quarantine.json`. The command uses the same 14-day deletion clock (`quarantinedAt + 14d`) as the velocity baseline and reports each entry as expired, near-deadline, healthy, or unknown when `quarantinedAt` is missing/invalid. It is a visibility aid only: default mode exits 0 even when entries are near or expired, preserving the deliberately-unwired policy and leaving rescue-or-delete decisions to maintainers. +Run `pnpm check:quarantine-ledger` to print a soonest-deadline-first summary of `scripts/lib/test-quarantine.json`. The command uses the same 14-day deletion clock (`quarantinedAt + 14d`) as the velocity baseline and reports each entry as expired, near-deadline, healthy, or unknown when `quarantinedAt` is missing/invalid. Default mode remains report-only for deadline status. + +The checker also enforces the quarantine lockstep. It reads only comment-stripped `exclude:` array literals in every `packages/*/vitest.config.ts`; include-shard lists and identifier/spread excludes are deliberately out of scope. It reports `missing-file` (a ledger entry names no file), `missing-exclude` (a ledger file lacks its package exclusion), and `dangling-exclude` (an exclusion names no file). `--strict` fails on any of those violations as well as near/expired deadlines. Flags: - `--warn-within=` changes the near-deadline window from the default 5 days. -- `--json` emits the computed rows plus summary counts for machine consumption. -- `--strict` exits 1 when any entry is expired or near-deadline, for opt-in local or project-specific gates only. Do not wire this into `pretest`, `test:gate`, or other default blocking lanes without an explicit policy change. +- `--json` emits the computed rows, lockstep violations, and summary counts for machine consumption. +- `--strict` exits 1 when an entry is expired/near-deadline or a lockstep violation exists; this is enforced by the PR check. **Rescue** (before the clock runs out) requires both: evidence the test catches real regressions, and a root-cause fix for the flake. Stabilization passes — widened timeouts, retries, loosened assertions — are appeasement, not rescue, and are banned (for agents especially). @@ -450,8 +452,11 @@ Flags: **2026-08-10 project-engine disposition (FN-8937):** Rescued `project-engine.test.ts` before its 2026-08-20 deadline. The suite's un-mocked `exec`-based integration-branch probe spawned real git, while the shared subprocess guard watchdog used fakeable timers; a duplicate-registration path could also orphan a watchdog handle. The resolver is now a deterministic suite seam, and watchdogs use captured real timers with owner-scoped failure draining, preserving sibling-test failure ownership. The ledger claim that runtime schedules 120s was a misread of `FUSION_TEST_SUBPROCESS_TIMEOUT_MS`: production retains its correct 60s ladder at `packages/engine/src/project-engine.ts:4551`, and the test correctly forbids its uncapped 120000ms rung. Thus the request to update the assertion to 120s is a documented deviation; no timeout was widened, retry added, or assertion weakened. The ledger/config exclusions were removed together, while the file remains outside `engine-core` pending separate gate-admission evidence. - -**2026-08-17 plugin-runner disposition (FN-9135):** Retained `packages/engine/src/__tests__/plugin-runner.test.ts`, its ledger row, and its paired default-lane exclusion after the 2/6/8-worker, two-runs-each engine-default campaign reproduced no subject failure and exposed no root cause. Passing reproduction runs do not qualify as the root-cause fix required for rescue, and the ratchet does not authorize deletion before the 2026-08-30 deadline. This preserves all 82 tests, including the remaining `onTaskCompleted` lifecycle-dispatch coverage, without changing timeouts, retries, assertions, skips, polling, or worker settings. + +**2026-08-17 plugin-runner disposition (FN-9141):** Rescued `packages/engine/src/__tests__/plugin-runner.test.ts` before the 2026-08-30 deletion-ratchet deadline. The seed-recorded completed lane combined shuffled order, no-isolation worker reuse, and a temporary byte-for-byte subject repeat. It reproduced one hot-reload warning assertion failure in the original subject because an unrelated worker-reused file cleared `createLogger.mock.results`; the repeat passed. The suite now keeps the initialized mock logger in a stable hoisted reference and explicitly proves cleanup cannot erase it, retaining the strict `stopPlugin` rejection/warning and renamed-complete-lane `onTaskCompleted` dispatch contracts. The ledger row and default-lane exclusion remain removed together. No timeout, retry, assertion weakening, skip, polling, or permanent worker-policy change was used. ### Validate before excluding and preserve timeout budgets diff --git a/packages/engine/src/__tests__/plugin-runner.test.ts b/packages/engine/src/__tests__/plugin-runner.test.ts index 8211e2f077..3aec05319b 100644 --- a/packages/engine/src/__tests__/plugin-runner.test.ts +++ b/packages/engine/src/__tests__/plugin-runner.test.ts @@ -17,15 +17,26 @@ import { type PluginInstallation, } from "@fusion/core"; import type { FusionPlugin, PluginToolDefinition } from "@fusion/core"; -import { createLogger } from "../logger.js"; -// Mock the logger to suppress output during tests -vi.mock("../logger.js", () => ({ - createLogger: vi.fn(() => ({ - log: vi.fn(), debug: vi.fn(), +/* +FNXC:PluginRunnerTests 2026-08-17-12:11: +The no-isolation, worker-reuse campaign showed unrelated files can call `vi.clearAllMocks()` +between this module's import and its lifecycle assertion. Keep the mocked logger instance in a +hoisted stable reference so the assertion continues to test the warning contract rather than +Vitest's erased mock-call history. +*/ +const { pluginRunnerLogger } = vi.hoisted(() => ({ + pluginRunnerLogger: { + log: vi.fn(), + debug: vi.fn(), warn: vi.fn(), error: vi.fn(), - })), + }, +})); + +// Mock the logger to suppress output during tests. +vi.mock("../logger.js", () => ({ + createLogger: vi.fn(() => pluginRunnerLogger), executorLog: { log: vi.fn(), debug: vi.fn(), warn: vi.fn(), @@ -99,17 +110,7 @@ describe("PluginRunner", () => { ...overrides, }); - const getPluginRunnerLogger = () => { - const logger = vi.mocked(createLogger).mock.results.at(-1)?.value as { - log: ReturnType; - warn: ReturnType; - error: ReturnType; - } | undefined; - if (!logger) { - throw new Error("Expected plugin-runner logger to be initialized"); - } - return logger; - }; + const getPluginRunnerLogger = () => pluginRunnerLogger; beforeEach(() => { // Create fresh mocks for each test @@ -1773,6 +1774,8 @@ describe("PluginRunner", () => { const unregisteredHandler = mockPluginStore.on.mock.calls.find( call => call[0] === "plugin:unregistered" )?.[1]; + // Mirror a worker-reused neighbour's mock cleanup after this module initialized. + vi.clearAllMocks(); const logger = getPluginRunnerLogger(); logger.warn.mockClear(); expect(unregisteredHandler).toBeTypeOf("function"); diff --git a/packages/engine/vitest.config.ts b/packages/engine/vitest.config.ts index cc038df584..b4a7fed0d1 100644 --- a/packages/engine/vitest.config.ts +++ b/packages/engine/vitest.config.ts @@ -336,15 +336,13 @@ export default defineConfig({ // / `test:all` invoked from the root `test:full` script. "src/**/*.slow.test.ts", /* - FNXC:PluginRunnerFlake 2026-08-16-17:30: - FN-9125 confirmed this in-memory mocked unit file has no PostgreSQL - dependency, but historical loaded-engine evidence captured seven - failures without sufficient identities for a structural fix. Quarantine - the whole file under the deletion ratchet instead of weakening its - lifecycle assertions; scripts/lib/test-quarantine.json is the paired - dated ledger record. + FNXC:PluginRunnerFlake 2026-08-17-12:11: + FN-9141 rescued the PluginRunner suite before the 2026-08-30 deletion + ratchet. A completed shuffled worker-reuse campaign reproduced a test-fixture + defect: cross-file `vi.clearAllMocks()` erased the logger mock-result history + used by the lifecycle warning assertion. The suite now keeps a stable hoisted + logger reference and directly proves that cleanup cannot erase that contract. */ - "src/__tests__/plugin-runner.test.ts", /* FNXC:FullSuiteBookkeeping 2026-08-09-03:49: All 11 engine-default entries from the 2026-08-05 full-suite quarantine wave (run 30982276306) were deleted under the deletion ratchet after operator directive. These tested pre-refactor APIs (getBuiltinWorkflow removed post-U10b), stale mock shapes, census/allowlist drift, and mock-hoist errors that no longer have a production path to exercise. diff --git a/scripts/__tests__/check-quarantine-ledger.test.mjs b/scripts/__tests__/check-quarantine-ledger.test.mjs index cf9b8a38ac..cab4dabf14 100644 --- a/scripts/__tests__/check-quarantine-ledger.test.mjs +++ b/scripts/__tests__/check-quarantine-ledger.test.mjs @@ -1,159 +1,160 @@ +/* +FNXC:QuarantineLockstep 2026-08-17-11:14: +Branch-B's real ledger is intentionally empty, so the real-tree assertion proves only that no dangling exclusion survives. Fixture negatives provide the non-vacuous proof that strict mode rejects each half of a broken quarantine decision. +*/ + import { test } from "node:test"; import assert from "node:assert/strict"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; -import { - computeDeadlines, - main, - readLedger, - renderReport, -} from "../check-quarantine-ledger.mjs"; +import { computeDeadlines, findLockstepViolations, main, readLedger, renderReport } from "../check-quarantine-ledger.mjs"; function captureStream() { let text = ""; - return { - stream: { write(chunk) { text += chunk; } }, - get text() { return text; }, - }; -} - -function tempRoot() { - return mkdtempSync(path.join(tmpdir(), "fusion-quarantine-ledger-")); + return { stream: { write(chunk) { text += chunk; } }, get text() { return text; } }; } +function tempRoot() { return mkdtempSync(path.join(tmpdir(), "fusion-quarantine-ledger-")); } function writeLedger(rootDir, ledger) { const ledgerPath = path.join(rootDir, "scripts/lib/test-quarantine.json"); mkdirSync(path.dirname(ledgerPath), { recursive: true }); writeFileSync(ledgerPath, `${JSON.stringify(ledger, null, 2)}\n`, "utf8"); return ledgerPath; } +function writeFile(rootDir, relativePath, content = "") { + const target = path.join(rootDir, relativePath); + mkdirSync(path.dirname(target), { recursive: true }); + writeFileSync(target, content, "utf8"); +} +function writeConfig(rootDir, packageName, content) { writeFile(rootDir, `packages/${packageName}/vitest.config.ts`, content); } +function healthyEntry(file = "packages/engine/src/healthy.test.ts") { return { file, reason: "fresh quarantine", quarantinedAt: "2026-07-12" }; } +function ledgerFixture(rootDir, entry = healthyEntry(), config = 'exclude: ["src/healthy.test.ts"]') { + writeFile(rootDir, entry.file); + writeConfig(rootDir, "engine", `export default { test: { ${config} } };`); + return writeLedger(rootDir, { entries: [entry] }); +} const fixedNow = new Date("2026-07-12T12:00:00.000Z"); +const fixtureLedger = { entries: [ + { file: "packages/engine/src/healthy.test.ts", reason: "fresh quarantine", quarantinedAt: "2026-07-12" }, + { file: "packages/engine/src/near.test.ts", reason: "approaching deletion deadline", quarantinedAt: "2026-07-04" }, + { file: "packages/engine/src/expired.test.ts", reason: "past deletion deadline", quarantinedAt: "2026-06-27" }, + { file: "packages/engine/src/unknown.test.ts", reason: "missing quarantine date" }, +] }; -const fixtureLedger = { - entries: [ - { - file: "healthy.test.ts", - reason: "fresh quarantine", - quarantinedAt: "2026-07-12", - }, - { - file: "near.test.ts", - reason: "approaching deletion deadline", - quarantinedAt: "2026-07-04", - }, - { - file: "expired.test.ts", - reason: "past deletion deadline", - quarantinedAt: "2026-06-27", - }, - { - file: "unknown.test.ts", - reason: "missing quarantine date", - }, - ], -}; +function materializeDeadlineFixture(rootDir) { + for (const entry of fixtureLedger.entries) writeFile(rootDir, entry.file); + writeConfig(rootDir, "engine", 'export default { test: { exclude: ["src/healthy.test.ts", "src/near.test.ts", "src/expired.test.ts", "src/unknown.test.ts"] } };'); + return writeLedger(rootDir, fixtureLedger); +} test("computeDeadlines buckets healthy, near, expired, and unknown entries", () => { const rows = computeDeadlines(fixtureLedger, { now: fixedNow, warnWithinDays: 6 }); - const byFile = Object.fromEntries(rows.map((row) => [row.file, row])); - + const byFile = Object.fromEntries(rows.map((row) => [path.basename(row.file), row])); assert.equal(byFile["healthy.test.ts"].status, "healthy"); assert.equal(byFile["healthy.test.ts"].daysRemaining, 14); - assert.equal(byFile["healthy.test.ts"].deadline, "2026-07-26"); - assert.equal(byFile["near.test.ts"].status, "near"); - assert.equal(byFile["near.test.ts"].daysRemaining, 6); - assert.equal(byFile["expired.test.ts"].status, "expired"); - assert.ok(byFile["expired.test.ts"].daysRemaining <= 0); - assert.equal(byFile["unknown.test.ts"].status, "unknown"); - assert.equal(byFile["unknown.test.ts"].daysRemaining, null); - assert.equal(byFile["unknown.test.ts"].deadline, null); -}); - -test("computeDeadlines sorts soonest deadline first with unknown entries last", () => { - const rows = computeDeadlines(fixtureLedger, { now: fixedNow, warnWithinDays: 6 }); - - assert.deepEqual(rows.map((row) => row.file), [ - "expired.test.ts", - "near.test.ts", - "healthy.test.ts", - "unknown.test.ts", - ]); }); test("renderReport handles an empty ledger without throwing", () => { - const rows = computeDeadlines({ entries: [] }, { now: fixedNow }); - const report = renderReport(rows); - - assert.deepEqual(rows, []); + const report = renderReport(computeDeadlines({ entries: [] }, { now: fixedNow })); assert.match(report, /Ledger is empty; nothing quarantined\./); - assert.match(report, /Summary: total=0 expired=0 near=0 healthy=0 unknown=0/); + assert.match(report, /lockstep=0/); }); test("readLedger tolerates a missing ledger and rejects non-array entries", () => { const rootDir = tempRoot(); try { assert.deepEqual(readLedger(path.join(rootDir, "missing.json")), { entries: [] }); - const ledgerPath = writeLedger(rootDir, { entries: {} }); - assert.throws( - () => readLedger(ledgerPath), - /quarantine ledger .* must have an "entries" array/, - ); - } finally { - rmSync(rootDir, { recursive: true, force: true }); - } + assert.throws(() => readLedger(writeLedger(rootDir, { entries: {} })), /must have an "entries" array/); + } finally { rmSync(rootDir, { recursive: true, force: true }); } }); test("main is report-only by default but --strict fails on near or expired entries", () => { const rootDir = tempRoot(); try { - const ledgerPath = writeLedger(rootDir, fixtureLedger); + const ledgerPath = materializeDeadlineFixture(rootDir); const stdout = captureStream(); const stderr = captureStream(); - assert.equal(main([], { rootDir, ledgerPath, stdout: stdout.stream, stderr: stderr.stream, now: fixedNow }), 0); - assert.match(stdout.text, /expired=1 near=0 healthy=2 unknown=1/); - assert.equal(stderr.text, ""); - - const strictStdout = captureStream(); - assert.equal(main(["--strict", "--warn-within=6"], { rootDir, ledgerPath, stdout: strictStdout.stream, stderr: stderr.stream, now: fixedNow }), 1); - - const healthyLedgerPath = writeLedger(rootDir, { entries: [{ file: "healthy.test.ts", quarantinedAt: "2026-07-12" }] }); - const healthyStdout = captureStream(); - assert.equal(main(["--strict"], { rootDir, ledgerPath: healthyLedgerPath, stdout: healthyStdout.stream, stderr: stderr.stream, now: fixedNow }), 0); - } finally { - rmSync(rootDir, { recursive: true, force: true }); - } + assert.match(stdout.text, /expired=1 near=0 healthy=2 unknown=1 lockstep=0/); + assert.equal(main(["--strict", "--warn-within=6"], { rootDir, ledgerPath, stdout: captureStream().stream, stderr: stderr.stream, now: fixedNow }), 1); + const healthyLedgerPath = ledgerFixture(rootDir); + assert.equal(main(["--strict"], { rootDir, ledgerPath: healthyLedgerPath, stdout: captureStream().stream, stderr: stderr.stream, now: fixedNow }), 0); + } finally { rmSync(rootDir, { recursive: true, force: true }); } }); -test("--json output parses and includes per-entry status and days remaining", () => { +test("missing ledger file is a strict missing-file violation", () => { const rootDir = tempRoot(); try { - const ledgerPath = writeLedger(rootDir, fixtureLedger); - const stdout = captureStream(); - const stderr = captureStream(); - - assert.equal(main(["--json", "--warn-within=6"], { rootDir, ledgerPath, stdout: stdout.stream, stderr: stderr.stream, now: fixedNow }), 0); - - const parsed = JSON.parse(stdout.text); - assert.equal(parsed.summary.expired, 1); - assert.equal(parsed.summary.near, 1); - assert.deepEqual( - parsed.rows.map((row) => ({ file: row.file, status: row.status, daysRemaining: row.daysRemaining })), - [ - { file: "expired.test.ts", status: "expired", daysRemaining: -1 }, - { file: "near.test.ts", status: "near", daysRemaining: 6 }, - { file: "healthy.test.ts", status: "healthy", daysRemaining: 14 }, - { file: "unknown.test.ts", status: "unknown", daysRemaining: null }, - ], - ); - assert.equal(stderr.text, ""); - } finally { - rmSync(rootDir, { recursive: true, force: true }); - } + writeConfig(rootDir, "engine", 'export default { test: { exclude: [] } };'); + const ledgerPath = writeLedger(rootDir, { entries: [healthyEntry("packages/engine/src/missing.test.ts")] }); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }).map((row) => row.kind), ["missing-file"]); + // A missing ledger file is enough to fail strict even when the deadline is healthy. + assert.equal(main(["--strict"], { rootDir, ledgerPath, stdout: captureStream().stream, stderr: captureStream().stream, now: fixedNow }), 1); + } finally { rmSync(rootDir, { recursive: true, force: true }); } +}); + +test("existing ledger file without an exclusion is missing-exclude", () => { + const rootDir = tempRoot(); + try { + const ledgerPath = ledgerFixture(rootDir, healthyEntry(), 'exclude: []'); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }).map((row) => row.kind), ["missing-exclude"]); + } finally { rmSync(rootDir, { recursive: true, force: true }); } +}); + +test("existing ledger file with its exclusion has no violations", () => { + const rootDir = tempRoot(); + try { + const ledgerPath = ledgerFixture(rootDir); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }), []); + assert.equal(main(["--strict"], { rootDir, ledgerPath, stdout: captureStream().stream, stderr: captureStream().stream, now: fixedNow }), 0); + } finally { rmSync(rootDir, { recursive: true, force: true }); } +}); + +test("exclude array naming a nonexistent test is dangling-exclude", () => { + const rootDir = tempRoot(); + try { + writeConfig(rootDir, "engine", 'export default { test: { exclude: ["src/deleted.test.ts"] } };'); + const ledgerPath = writeLedger(rootDir, { entries: [] }); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }).map((row) => row.kind), ["dangling-exclude"]); + } finally { rmSync(rootDir, { recursive: true, force: true }); } +}); + +test("include lists and non-array excludes are deliberately ignored", () => { + const rootDir = tempRoot(); + try { + writeConfig(rootDir, "dashboard", 'const skipListDashboardGlobs = []; export default { test: { include: ["app/missing.test.ts"], exclude: skipListDashboardGlobs } };'); + const ledgerPath = writeLedger(rootDir, { entries: [] }); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }), []); + } finally { rmSync(rootDir, { recursive: true, force: true }); } +}); + +test("comment-only ledger file mention cannot satisfy an exclusion", () => { + const rootDir = tempRoot(); + try { + const ledgerPath = ledgerFixture(rootDir, healthyEntry(), '/* FNXC: historical "src/healthy.test.ts" */ exclude: []'); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }).map((row) => row.kind), ["missing-exclude"]); + } finally { rmSync(rootDir, { recursive: true, force: true }); } +}); + +test("real repository has no lockstep violations", () => { + const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); + const ledgerPath = path.join(rootDir, "scripts/lib/test-quarantine.json"); + assert.deepEqual(findLockstepViolations({ rootDir, ledger: readLedger(ledgerPath) }), []); +}); + +test("--json output includes lockstep violations", () => { + const rootDir = tempRoot(); + try { + const ledgerPath = ledgerFixture(rootDir); + const stdout = captureStream(); + assert.equal(main(["--json"], { rootDir, ledgerPath, stdout: stdout.stream, stderr: captureStream().stream, now: fixedNow }), 0); + assert.deepEqual(JSON.parse(stdout.text).lockstep, []); + } finally { rmSync(rootDir, { recursive: true, force: true }); } }); diff --git a/scripts/__tests__/ci-test-shard-timings.test.mjs b/scripts/__tests__/ci-test-shard-timings.test.mjs index ce65bace6f..dd6102ef71 100644 --- a/scripts/__tests__/ci-test-shard-timings.test.mjs +++ b/scripts/__tests__/ci-test-shard-timings.test.mjs @@ -55,6 +55,7 @@ function tmpRoot() { * files. Stale or phantom-path entries silently degrade CI shard balancing and * can hide merge-gate regressions, so this guard fails before that drift lands. */ + test("committed timing snapshot is fresh, parseable, and references live test files", () => { const snapshotPath = path.join(REPO_ROOT, TIMINGS_SNAPSHOT_RELATIVE); const snapshot = JSON.parse(readFileSync(snapshotPath, "utf8")); @@ -70,7 +71,9 @@ test("committed timing snapshot is fresh, parseable, and references live test fi ); const recordedFiles = Object.values(snapshot.packages).flatMap((pkg) => Object.keys(pkg.files ?? {})); - const missingFiles = recordedFiles.filter((file) => !existsSync(path.join(REPO_ROOT, file))); + const missingFiles = recordedFiles.filter( + (file) => !existsSync(path.join(REPO_ROOT, file)), + ); assert.deepEqual(missingFiles, [], `timing snapshot references missing test files:\n${missingFiles.join("\n")}`); const timings = loadPlanningTimings({ projectRoot: REPO_ROOT }); diff --git a/scripts/check-quarantine-ledger.mjs b/scripts/check-quarantine-ledger.mjs index 1aca8c281a..0206c932cb 100644 --- a/scripts/check-quarantine-ledger.mjs +++ b/scripts/check-quarantine-ledger.mjs @@ -3,9 +3,12 @@ FNXC:TestQuarantine 2026-07-12-00:00: The flaky-test deletion ratchet had no visibility tool for entries approaching the `quarantinedAt + 14d` deletion deadline. This report surfaces near-deadline quarantines so maintainers can make deliberate rescue-or-expire decisions while preserving the policy's report-only default; `--strict` is the opt-in enforcement path. + +FNXC:QuarantineLockstep 2026-08-17-11:10: +A ledger row and its Vitest exclusion are one quarantine decision. Scan only comment-free `exclude:` array literals so stale FNXC prose and dashboard include shards cannot turn a disposition check into a false failure; strict mode rejects either half pointing at a missing test file. */ -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, readFileSync, readdirSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -59,11 +62,124 @@ function summarizeRows(rows) { ); } -export function readLedger(ledgerPath) { - if (!existsSync(ledgerPath)) { - return { entries: [] }; +function stripComments(source) { + return source + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/^\s*\/\/.*$/gm, ""); +} + +function extractBalancedArray(source, openingBracket) { + let depth = 0; + let quote = null; + let escaped = false; + + for (let index = openingBracket; index < source.length; index += 1) { + const character = source[index]; + if (quote) { + if (escaped) { + escaped = false; + } else if (character === "\\") { + escaped = true; + } else if (character === quote) { + quote = null; + } + continue; + } + if (character === "\"" || character === "'") { + quote = character; + } else if (character === "[") { + depth += 1; + } else if (character === "]") { + depth -= 1; + if (depth === 0) return source.slice(openingBracket, index + 1); + } + } + return null; +} + +function extractConcreteExcludes(source) { + const commentFree = stripComments(source); + const excludes = []; + const excludePattern = /\bexclude\s*:/g; + let match; + while ((match = excludePattern.exec(commentFree))) { + let index = match.index + match[0].length; + while (/\s/.test(commentFree[index] ?? "")) index += 1; + if (commentFree[index] !== "[") continue; + const array = extractBalancedArray(commentFree, index); + if (array == null) continue; + const strings = /"((?:\\.|[^"\\])*)"/g; + let stringMatch; + while ((stringMatch = strings.exec(array))) { + const value = JSON.parse(`"${stringMatch[1]}"`); + if (/\.test\.tsx?$/.test(value) && !/[*?{}]/.test(value)) excludes.push(value); + } + excludePattern.lastIndex = index + array.length; + } + return excludes; +} + +function discoverPackageConfigs(rootDir) { + const packagesRoot = path.join(rootDir, "packages"); + if (!existsSync(packagesRoot)) return []; + return readdirSync(packagesRoot, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => path.join("packages", entry.name, "vitest.config.ts")) + .filter((config) => existsSync(path.join(rootDir, config))); +} + +function normalizeConfigPath(rootDir, config) { + return path.isAbsolute(config) ? config : path.join(rootDir, config); +} + +/** + * Checks the two directions of the quarantine decision without evaluating Vitest configuration. + * `packageConfigs` accepts root-relative or absolute config paths to keep fixture tests narrow. + */ +export function findLockstepViolations({ rootDir, ledger, packageConfigs = discoverPackageConfigs(rootDir) }) { + const configExcludes = new Map(); + for (const config of packageConfigs) { + const configPath = normalizeConfigPath(rootDir, config); + if (!existsSync(configPath)) continue; + const relativeConfig = path.relative(rootDir, configPath); + configExcludes.set(relativeConfig, extractConcreteExcludes(readFileSync(configPath, "utf8"))); } + const violations = []; + for (const entry of Array.isArray(ledger?.entries) ? ledger.entries : []) { + const file = String(entry?.file ?? ""); + const filePath = path.join(rootDir, file); + if (!existsSync(filePath)) { + violations.push({ kind: "missing-file", file, detail: "ledger entry names no file on disk" }); + continue; + } + + const packageMatch = /^packages\/([^/]+)\/(.+)$/.exec(file); + const config = packageMatch ? path.join("packages", packageMatch[1], "vitest.config.ts") : null; + if (config == null || !configExcludes.has(config)) { + violations.push({ kind: "unmapped-entry", file, config: config ?? undefined, detail: "ledger file has no package Vitest config" }); + continue; + } + + const packageRelativeFile = packageMatch[2]; + if (!configExcludes.get(config).includes(packageRelativeFile)) { + violations.push({ kind: "missing-exclude", file, config, detail: "ledger file is absent from the package exclude array" }); + } + } + + for (const [config, excludes] of configExcludes) { + for (const excludedFile of excludes) { + const packageRoot = path.dirname(config); + if (!existsSync(path.join(rootDir, packageRoot, excludedFile))) { + violations.push({ kind: "dangling-exclude", file: path.join(packageRoot, excludedFile), config, detail: "exclude array names no file on disk" }); + } + } + } + return violations; +} + +export function readLedger(ledgerPath) { + if (!existsSync(ledgerPath)) return { entries: [] }; const json = JSON.parse(readFileSync(ledgerPath, "utf8")); if (json?.entries != null && !Array.isArray(json.entries)) { throw new Error(`quarantine ledger ${ledgerPath} must have an "entries" array`); @@ -77,32 +193,11 @@ export function computeDeadlines(json, { now = new Date(), warnWithinDays = DEFA const quarantinedAtDate = toDate(entry?.quarantinedAt); const age = ageDays(entry?.quarantinedAt, now); const daysRemaining = age == null ? null : DELETION_CLOCK_DAYS - age; - const deadlineDate = quarantinedAtDate == null - ? null - : new Date(quarantinedAtDate.getTime() + DELETION_CLOCK_DAYS * MS_PER_DAY); + const deadlineDate = quarantinedAtDate == null ? null : new Date(quarantinedAtDate.getTime() + DELETION_CLOCK_DAYS * MS_PER_DAY); let status = "unknown"; - if (daysRemaining != null) { - if (daysRemaining <= 0) { - status = "expired"; - } else if (daysRemaining <= warnWithinDays) { - status = "near"; - } else { - status = "healthy"; - } - } - - return { - index, - file: entry?.file ?? "unknown", - reason: entry?.reason ?? "", - quarantinedAt: entry?.quarantinedAt ?? null, - ageDays: age, - daysRemaining, - deadline: deadlineDate == null ? null : formatIsoDate(deadlineDate), - status, - }; + if (daysRemaining != null) status = daysRemaining <= 0 ? "expired" : daysRemaining <= warnWithinDays ? "near" : "healthy"; + return { index, file: entry?.file ?? "unknown", reason: entry?.reason ?? "", quarantinedAt: entry?.quarantinedAt ?? null, ageDays: age, daysRemaining, deadline: deadlineDate == null ? null : formatIsoDate(deadlineDate), status }; }); - return rows.sort((a, b) => { if (a.deadline == null && b.deadline == null) return a.index - b.index; if (a.deadline == null) return 1; @@ -111,92 +206,52 @@ export function computeDeadlines(json, { now = new Date(), warnWithinDays = DEFA }); } -export function renderReport(rows, { warnWithinDays = DEFAULT_WARN_WITHIN_DAYS } = {}) { +export function renderReport(rows, { warnWithinDays = DEFAULT_WARN_WITHIN_DAYS, violations = [] } = {}) { const summary = summarizeRows(rows); const lines = [ "Quarantine ledger deadline report", `Deletion clock: quarantinedAt + ${DELETION_CLOCK_DAYS} days; near-deadline window: ${warnWithinDays} days`, - `Summary: total=${summary.total} expired=${summary.expired} near=${summary.near} healthy=${summary.healthy} unknown=${summary.unknown}`, + `Summary: total=${summary.total} expired=${summary.expired} near=${summary.near} healthy=${summary.healthy} unknown=${summary.unknown} lockstep=${violations.length}`, ]; - - if (rows.length === 0) { - lines.push("Ledger is empty; nothing quarantined."); - return `${lines.join("\n")}\n`; + if (rows.length === 0) lines.push("Ledger is empty; nothing quarantined."); + else { + lines.push("Entries (soonest deadline first):"); + for (const row of rows) { + const timing = row.status === "expired" ? `EXPIRED (${Math.abs(row.daysRemaining)} day${Math.abs(row.daysRemaining) === 1 ? "" : "s"} overdue)` : row.daysRemaining == null ? "deadline unknown" : `${row.daysRemaining} day${row.daysRemaining === 1 ? "" : "s"} remaining`; + lines.push(`- [${row.status}] ${row.file} — ${timing}; deadline=${row.deadline ?? "unknown"}; reason=${truncateReason(row.reason) || "no reason recorded"}`); + } } - - lines.push("Entries (soonest deadline first):"); - for (const row of rows) { - const timing = row.status === "expired" - ? `EXPIRED (${Math.abs(row.daysRemaining)} day${Math.abs(row.daysRemaining) === 1 ? "" : "s"} overdue)` - : row.daysRemaining == null - ? "deadline unknown" - : `${row.daysRemaining} day${row.daysRemaining === 1 ? "" : "s"} remaining`; - const deadline = row.deadline == null ? "unknown" : row.deadline; - const reason = truncateReason(row.reason) || "no reason recorded"; - lines.push(`- [${row.status}] ${row.file} — ${timing}; deadline=${deadline}; reason=${reason}`); + if (violations.length > 0) { + lines.push("Lockstep violations:"); + for (const violation of violations) lines.push(`- [${violation.kind}] ${violation.file}${violation.config ? ` (${violation.config})` : ""}: ${violation.detail}`); } - return `${lines.join("\n")}\n`; } function parseArgs(argv) { - const args = { - warnWithinDays: DEFAULT_WARN_WITHIN_DAYS, - json: false, - strict: false, - help: false, - }; - + const args = { warnWithinDays: DEFAULT_WARN_WITHIN_DAYS, json: false, strict: false, help: false }; for (const arg of argv) { - if (arg === "--json") { - args.json = true; - } else if (arg === "--strict") { - args.strict = true; - } else if (arg === "--help" || arg === "-h") { - args.help = true; - } else if (arg.startsWith("--warn-within=")) { - args.warnWithinDays = normalizeWarnWithinDays(arg.slice("--warn-within=".length)); - } else { - throw new Error(`Unknown argument: ${arg}`); - } + if (arg === "--json") args.json = true; + else if (arg === "--strict") args.strict = true; + else if (arg === "--help" || arg === "-h") args.help = true; + else if (arg.startsWith("--warn-within=")) args.warnWithinDays = normalizeWarnWithinDays(arg.slice("--warn-within=".length)); + else throw new Error(`Unknown argument: ${arg}`); } - return args; } export function main(argv = process.argv.slice(2), { rootDir = repoRoot, stdout = process.stdout, stderr = process.stderr, now = new Date(), ledgerPath = path.join(rootDir, DEFAULT_QUARANTINE_PATH) } = {}) { let args; - try { - args = parseArgs(argv); - } catch (error) { - stderr.write(`${error.message}\n`); - return 1; - } - - if (args.help) { - stdout.write("Usage: node scripts/check-quarantine-ledger.mjs [--warn-within=] [--json] [--strict]\n"); - return 0; - } - + try { args = parseArgs(argv); } catch (error) { stderr.write(`${error.message}\n`); return 1; } + if (args.help) { stdout.write("Usage: node scripts/check-quarantine-ledger.mjs [--warn-within=] [--json] [--strict]\n"); return 0; } let ledger; - try { - ledger = readLedger(ledgerPath); - } catch (error) { - stderr.write(`Failed to read quarantine ledger: ${error.message}\n`); - return 1; - } - + try { ledger = readLedger(ledgerPath); } catch (error) { stderr.write(`Failed to read quarantine ledger: ${error.message}\n`); return 1; } const rows = computeDeadlines(ledger, { now, warnWithinDays: args.warnWithinDays }); - const summary = summarizeRows(rows); - if (args.json) { - stdout.write(`${JSON.stringify({ summary, rows }, null, 2)}\n`); - } else { - stdout.write(renderReport(rows, { warnWithinDays: args.warnWithinDays })); - } - - return args.strict && (summary.expired > 0 || summary.near > 0) ? 1 : 0; + const violations = findLockstepViolations({ rootDir, ledger }); + const summary = { ...summarizeRows(rows), lockstep: violations.length }; + if (args.json) stdout.write(`${JSON.stringify({ summary, rows, lockstep: violations }, null, 2)}\n`); + else stdout.write(renderReport(rows, { warnWithinDays: args.warnWithinDays, violations })); + return args.strict && (summary.expired > 0 || summary.near > 0 || violations.length > 0) ? 1 : 0; } -if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { - process.exitCode = main(); -} +if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) process.exitCode = main(); diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json index 5b6e25133d..2f340e17c1 100644 --- a/scripts/lib/test-quarantine.json +++ b/scripts/lib/test-quarantine.json @@ -1,10 +1,4 @@ { - "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config is the enforcement.", - "entries": [ - { - "file": "packages/engine/src/__tests__/plugin-runner.test.ts", - "reason": "FN-9125: first-sighting register entry 3; failure evidence https://github.com/Runfusion/Fusion/pull/2799#issuecomment-5134921769 captures seven loaded-engine failures on the PR #2799 merged-with-main tree, but only `PluginRunner > task lifecycle hooks > should invoke onTaskCompleted when the complete lane is RENAMED` survived because the original dot output was truncated. FN-9125 confirmed the file is in-memory/mocked and not PostgreSQL-backed; no current structural failure was reproducible. Quarantined under the deletion ratchet rather than weakening lifecycle assertions.", - "quarantinedAt": "2026-08-16" - } - ] + "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). scripts/check-quarantine-ledger.mjs mechanically verifies this ledger and concrete `exclude:` array entries stay in lockstep.", + "entries": [] }