diff --git a/AGENTS.md b/AGENTS.md index de7835bf36..792c835773 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,6 +176,10 @@ conflicting PR, i.e. after the cost was already paid. ### Standing Rule: Flaky Tests Are Quarantined on Sight (Deletion Ratchet) - A test observed failing without a corresponding real bug in the change is QUARANTINED ON SIGHT: add an entry to `scripts/lib/test-quarantine.json` (`file`, `reason` with a link to the failing run, `quarantinedAt`) AND a matching one-line `exclude` in that package's vitest config, in the same commit. + +- On a **first** sighting only, a flake in a file whose remaining coverage is substantial MAY be recorded in [`docs/solutions/test-failures/suite-only-flakes-observed-register.md`](docs/solutions/test-failures/suite-only-flakes-observed-register.md) instead of quarantined, because quarantine is file-level and would evict that coverage over a single observation. Recording is mandatory, not optional evasion: the register entry must include the file path, exact `suite > case`, reproduction data, and observed tree/SHA. A **second** sighting of the same test is an ordinary on-sight quarantine with no further discretion. This exception does not relax the anti-appeasement rule (no widened timeouts, retries, loosened/deleted assertions, or `.skip`) and does not apply to a merge-gate flake, which is still evicted under the gate rule below. - **Agents must never appease a flaky test.** No widened timeouts, no added retries, no loosened or deleted assertions to make a flake pass. Quarantine it instead. Appeasement drains the test's signal and is how the suite rotted last time. - A quarantined test is DELETED after 14 days (`quarantinedAt` + 2 weeks) unless rescued. Rescue requires evidence the test catches real regressions plus a root-cause fix — not stabilization passes. - A flake INSIDE the merge gate is evicted, not skipped: remove its line from the `engine-core` allow-list in `packages/engine/vitest.config.ts` (the eviction PR does not need the flaky test to pass). diff --git a/docs/solutions/test-failures/suite-only-flakes-observed-register.md b/docs/solutions/test-failures/suite-only-flakes-observed-register.md new file mode 100644 index 0000000000..a63e6cb34b --- /dev/null +++ b/docs/solutions/test-failures/suite-only-flakes-observed-register.md @@ -0,0 +1,74 @@ +--- +category: test-failures +module: testing +date: 2026-08-01 +problem_type: suite_only_flake +component: PostgreSQL test infrastructure +severity: medium +applies_when: + - "A test fails under full-suite parallelism but passes when run alone" + - "A first flake sighting is in a file whose remaining coverage is substantial" + - "Capturing evidence before a file-level quarantine decision" +tags: + - flake + - postgres + - full-suite + - quarantine +--- + +# Observed suite-only flakes register + +This register preserves first-sighting evidence under the narrow exception in [AGENTS.md](../../../AGENTS.md#standing-rule-flaky-tests-are-quarantined-on-sight-deletion-ratchet). It is not a quarantine: the normal default remains a ledger entry plus matching Vitest `exclude` in the same commit. + +## 1. Project identity returns no stored identity + +- **File:** `packages/core/src/__tests__/postgres/project-identity.test.ts` +- **Exact test:** `project-identity async (PostgreSQL integration) > returns null when no identity is stored` +- **Observed tree/SHA:** `origin/main` at `7927c7b58a` +- **Observed frequency:** 1-in-3 full-core-suite runs. + +| run | result | +|---|---| +| full core suite (1st) | **1 failed** / 4824 passed | +| full core suite (2nd) | 4825 passed | +| full core suite (3rd) | 4825 passed | +| file alone ×2 | 6 passed, 6 passed | + +## 2. Schema applier retains registered dependents + +- **File:** `packages/core/src/__tests__/postgres/schema-applier.test.ts` +- **Exact test:** `schema-applier: VAL-SCHEMA-001 final-schema parity (table counts) > retains unreplaced registered dependents for every delete action` +- **Observed tree/SHA:** PR [#2828](https://github.com/Runfusion/Fusion/pull/2828) merged-with-main. + +| run | result | +|---|---| +| full core suite on #2828 merged-with-main | **failed** | +| file alone ×2 on the same tree | 75 passed, 75 passed | +| file alone on `origin/main` | passed | + +## 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` +- **Observed tree/SHA:** PR [#2799](https://github.com/Runfusion/Fusion/pull/2799) merged-with-main. + +| run | result | +|---|---| +| full engine suite on #2799 merged-with-main (1st) | **8 failed** (7 in this file + 1 inherited) | +| full engine suite, same tree (2nd) | 1 failed (the inherited one only) | +| file alone | 80 passed | +| full engine suite on `origin/main` ×2 | clean | + +Seven tests failed in `plugin-runner.test.ts`, but only this one identity survived capture: `--reporter=dot | tail -3` truncated the `FAIL` lines and retained only the summary. + +## Common shape and unverified suspicion + +All three are PostgreSQL-backed or PostgreSQL-suite-adjacent, pass in isolation, and appear only under full-suite parallelism. This points at shared database state between test files rather than any of the three tests. It is **unverified and uninvestigated**, not a diagnosis; do not infer a root-cause fix from this record. + +## Policy and escalation + +Quarantine is file-level, while these files retain 6 / 75 / 80 passing tests. Under the first-sighting exception in AGENTS.md, recording preserves that valuable coverage while retaining the evidence needed for action. A **second sighting** of any registered test is an on-sight quarantine: add it to `scripts/lib/test-quarantine.json` and the matching Vitest `exclude` in one lockstep commit; this register entry is then evidence for the ledger `reason`. + +Capture **full runner output** before recording or quarantining a failure—for example, tee it to a file. Never pipe a dot reporter through `tail`: the summary survives while the `FAIL` identity lines needed for a quarantine entry are exactly what gets truncated. + +Source: [Runfusion/Fusion issue #2862](https://github.com/Runfusion/Fusion/issues/2862). diff --git a/docs/testing.md b/docs/testing.md index b56d8c525c..f71a21585f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -337,6 +337,8 @@ The CI job uses `fetch-depth: 0` because these tests run real git operations. 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"). +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. + **To quarantine a test** (a test that failed without a corresponding real bug in the change), in one commit: 1. Add an entry to `scripts/lib/test-quarantine.json`: @@ -359,6 +361,8 @@ Flags: ### Validate before excluding and preserve timeout budgets +Capture **full runner output** before recording or filing a ledger entry—for example, tee it to a file. Never pipe a dot reporter through `tail`: the summary remains but the `FAIL` identity lines needed for evidence are truncated. + Validate a quarantine-bound file **before** adding its exclusion. The dashboard quarantine array is spread into every dashboard project exclude, so even an explicitly named CLI file is suppressed afterward; no CLI flag removes a configured exclusion. The only local route back to validation is an uncommitted removal of both lockstep entries. Hoisting expensive real-dependency construction into a reusable per-file `beforeAll` is a valid structural rescue, but it inherits the hook timeout and does not by itself fix a duration-driven flake. Do not widen a timeout under a “deliberate budget” framing without an owner-approved policy exception; FN-8647 and [#3245](https://github.com/Runfusion/Fusion/issues/3245) document this distinction. When proving that a quarantine change did not alter the budget, inspect the **staged** diff before the final lockstep commit and fail nonzero on any added **or removed** config `testTimeout`, `hookTimeout`, or `teardownTimeout` line—removal falls back to a runner default. Then parse the resulting test source rather than applying a line-wise diff regex: any expression in a hook's second or case's third timeout position is forbidden regardless of its shape (`15 * 1000`, a bare identifier, or a cast all count). Resolve calls through a `vitest` import alias map and namespace bindings; reject local rebinding and computed access outright. Rebinding detection must scan the whole initializer/assignment RHS, not one root identifier, so container forms such as `const [h] = [beforeAll]`, object/conditional/sequence wrappers, and element access are caught while direct invocation callee positions are skipped. diff --git a/scripts/__tests__/observed-flake-register.test.mjs b/scripts/__tests__/observed-flake-register.test.mjs new file mode 100644 index 0000000000..17d2484bfd --- /dev/null +++ b/scripts/__tests__/observed-flake-register.test.mjs @@ -0,0 +1,61 @@ +/* +FNXC:TestFlakeRegister 2026-08-01-07:00: +Issue #2862 observed suite-only PostgreSQL-adjacent flakes in files with substantial remaining coverage, so the AGENTS.md first-sighting exception authorizes a record instead of a file-level quarantine. This test prevents dangling paths, suite-title drift, and silent removal of that narrow policy or its evidence requirements. +*/ +import assert from "node:assert/strict"; +import { existsSync, readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const rootDir = resolve(__dirname, "../.."); +const registerRelativePath = "docs/solutions/test-failures/suite-only-flakes-observed-register.md"; +const registerPath = resolve(rootDir, registerRelativePath); +const agentsPath = resolve(rootDir, "AGENTS.md"); +const testingPath = resolve(rootDir, "docs/testing.md"); + +function readRegisterEntries(register) { + const entries = [...register.matchAll(/- \*\*File:\*\* `([^`]+)`\n- \*\*Exact test:\*\* `([^`]+)`/g)].map( + ([, file, fullName]) => ({ file, fullName }), + ); + + assert.ok(entries.length > 0, "Expected the observed-flake register to name at least one test"); + return entries; +} + +test("observed-flake register frontmatter identifies test failures", () => { + assert.ok(existsSync(registerPath), `Missing register: ${registerRelativePath}`); + const register = readFileSync(registerPath, "utf8"); + const frontmatter = register.match(/^---\n([\s\S]*?)\n---/); + + assert.ok(frontmatter, "Expected YAML frontmatter in the observed-flake register"); + assert.match(frontmatter[1], /^category:\s*test-failures\s*$/m); +}); + +test("observed-flake register paths and every documented hierarchy segment remain valid", () => { + const register = readFileSync(registerPath, "utf8"); + + for (const { file, fullName } of readRegisterEntries(register)) { + const subjectPath = resolve(rootDir, file); + assert.ok(existsSync(subjectPath), `Registered test file no longer exists: ${file}`); + + const subject = readFileSync(subjectPath, "utf8"); + for (const segment of fullName.split(">").map((part) => part.trim())) { + assert.ok(segment, `Empty suite hierarchy segment in ${fullName}`); + assert.ok(subject.includes(segment), `Missing hierarchy segment "${segment}" in ${file}`); + } + } +}); + +test("testing guidance and the AGENTS.md exception retain record escalation evidence", () => { + const testing = readFileSync(testingPath, "utf8"); + const agents = readFileSync(agentsPath, "utf8"); + const register = readFileSync(registerPath, "utf8"); + + assert.ok(testing.includes(registerRelativePath), "docs/testing.md must link the observed-flake register"); + assert.ok(agents.includes("On a **first** sighting only"), "AGENTS.md must retain the first-sighting exception"); + assert.ok(agents.includes("A **second** sighting of the same test"), "AGENTS.md must retain second-sighting escalation"); + assert.ok(register.includes("A **second sighting**"), "Register must retain second-sighting escalation"); + assert.ok(register.includes("Capture **full runner output**"), "Register must retain full-output capture guidance"); +});