diff --git a/docs/testing.md b/docs/testing.md index ecdd039334..4da035186d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -162,6 +162,8 @@ Legitimate legacy exceptions must be recorded in `scripts/lib/test-timeout-appea **2026-06-15 rescue batch (FN-6486):** two same-day quarantines were rescued before their 2026-06-29 deletion deadline. `store-concurrent-writes.test.ts` kept its WAL/`transactionImmediate` regression value by making the external lock helper's timed release use synchronous `Atomics.wait` inside the child process, removing event-loop timer scheduling as the load-only flake source without widening retry windows. `extension-task-tools.test.ts` kept its worktree-root task-tool coverage by closing each real `TaskStore` fixture before temp-root removal and using non-hoisted mock cleanup. The reusable pattern is to remove scheduler/resource leaks in the helper or fixture seam, then prove the rescue with repeated exact-file runs plus package lanes, not with timeout bumps, retries, assertion loosening, or worker changes. +**2026-06-16 rescue (FN-6514):** `packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx` was rescued before its 2026-06-30 deletion deadline. The file still caught real quick-entry behavior regressions, but it leaked jsdom descriptors for `window.innerWidth`, `window.matchMedia`, `document.visibilityState`, `URL.createObjectURL`, and `URL.revokeObjectURL`; a mobile viewport helper could leave later tests in the same dashboard backfill shard observing `innerWidth=375` and mismatched responsive assertions. The rescue removed the ledger/config quarantine entries in lockstep, captured each original `PropertyDescriptor` at module load, restored those descriptors (or deleted own properties that were originally absent) in `afterEach`, and added a guard test that mutates all rescued globals before asserting they return to their original descriptors. Reusable pattern: any test file that changes jsdom globals with `Object.defineProperty` or spies on replaceable globals must snapshot the original descriptor at the top of the file, restore it in every `afterEach`, and prove the invariant with a guard test; do not use timeout bumps, retries, worker changes, or blanket `vi.restoreAllMocks()` when module mocks depend on stable implementations. + **Gate eviction:** a flake inside the merge gate cannot block all merges while red — it is evicted by removing its line from the `engine-core` allow-list (no quarantine entry needed unless it should also leave the non-blocking tier). **Gate admission:** the mirror operation — add the test's path to the `engine-core` `include` array in `packages/engine/vitest.config.ts`, citing the evidence of value (a real regression it caught) in the PR. Keep the project under its ~60s wall-clock budget. diff --git a/packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx b/packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx index d54d1bf12d..f1853f2c5a 100644 --- a/packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx +++ b/packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx @@ -28,6 +28,39 @@ const TEST_PROJECT_ID = "proj-123"; const QUICK_ENTRY_STORAGE_KEY = scopedKey("kb-quick-entry-text", TEST_PROJECT_ID); const QUICK_ENTRY_BOX_CSS = readFileSync("app/components/QuickEntryBox.css", "utf8"); +const originalWindowInnerWidthDescriptor = Object.getOwnPropertyDescriptor(window, "innerWidth"); +const originalWindowMatchMediaDescriptor = Object.getOwnPropertyDescriptor(window, "matchMedia"); +const originalDocumentVisibilityStateDescriptor = Object.getOwnPropertyDescriptor(document, "visibilityState"); +const originalCreateObjectURLDescriptor = Object.getOwnPropertyDescriptor(URL, "createObjectURL"); +const originalRevokeObjectURLDescriptor = Object.getOwnPropertyDescriptor(URL, "revokeObjectURL"); + +function restoreDescriptor(target: object, property: PropertyKey, descriptor: PropertyDescriptor | undefined) { + if (descriptor) { + Object.defineProperty(target, property, descriptor); + return; + } + + delete (target as Record)[property]; +} + +function restoreQuickEntryTestGlobals() { + restoreDescriptor(window, "innerWidth", originalWindowInnerWidthDescriptor); + restoreDescriptor(window, "matchMedia", originalWindowMatchMediaDescriptor); + restoreDescriptor(document, "visibilityState", originalDocumentVisibilityStateDescriptor); + restoreDescriptor(URL, "createObjectURL", originalCreateObjectURLDescriptor); + restoreDescriptor(URL, "revokeObjectURL", originalRevokeObjectURLDescriptor); +} + +function expectQuickEntryTestGlobalsRestored() { + expect(Object.getOwnPropertyDescriptor(window, "innerWidth")).toEqual(originalWindowInnerWidthDescriptor); + expect(Object.getOwnPropertyDescriptor(window, "matchMedia")).toEqual(originalWindowMatchMediaDescriptor); + expect(Object.getOwnPropertyDescriptor(document, "visibilityState")).toEqual( + originalDocumentVisibilityStateDescriptor, + ); + expect(Object.getOwnPropertyDescriptor(URL, "createObjectURL")).toEqual(originalCreateObjectURLDescriptor); + expect(Object.getOwnPropertyDescriptor(URL, "revokeObjectURL")).toEqual(originalRevokeObjectURLDescriptor); +} + function quickEntryMobileActionsTouchRule() { return ( QUICK_ENTRY_BOX_CSS.match( @@ -336,6 +369,23 @@ describe("QuickEntryBox", () => { }); vi.useRealTimers(); localStorage.clear(); + restoreQuickEntryTestGlobals(); + }); + + /* + FNXC:DashboardTestIsolation 2026-06-16-21:31: + QuickEntryBox runs in broad dashboard jsdom workers, so viewport, visibility, and object-URL mocks must restore their original descriptors after every test. + This keeps mobile `innerWidth`/`matchMedia` state from flipping later disclosure `aria-expanded` assertions under sibling-file load. + */ + it("restores jsdom globals mutated by viewport and URL helpers", () => { + mockMobileViewport(); + Object.defineProperty(document, "visibilityState", { configurable: true, value: "visible" }); + Object.defineProperty(URL, "createObjectURL", { configurable: true, writable: true, value: vi.fn() }); + Object.defineProperty(URL, "revokeObjectURL", { configurable: true, writable: true, value: vi.fn() }); + + restoreQuickEntryTestGlobals(); + + expectQuickEntryTestGlobalsRestored(); }); it("renders textarea with placeholder", () => { diff --git a/packages/dashboard/vitest.config.ts b/packages/dashboard/vitest.config.ts index 32d82c2088..16d0ec2aa8 100644 --- a/packages/dashboard/vitest.config.ts +++ b/packages/dashboard/vitest.config.ts @@ -236,16 +236,15 @@ FNXC:DashboardTestQuarantine 2026-06-14-17:01: FN-6454 applied the quarantine deletion ratchet to every dashboard test quarantined on 2026-06-14. Keep this list empty until a new flaky dashboard test is quarantined with a matching ledger entry. -FNXC:DashboardTestQuarantine 2026-06-16-18:59: -FN-6496 verification observed QuickEntryBox expanded-mode assertions fail only in the workspace gate while an isolated file rerun passed. -Quarantine the file under the deletion ratchet instead of appeasing timing/state leakage with retries or widened waits. +FNXC:DashboardTestQuarantine 2026-06-16-21:31: +FN-6514 rescued QuickEntryBox before the 2026-06-30 deletion deadline by restoring its mutated jsdom viewport, visibility, and object-URL globals in file teardown. +Keep it out of this exclude list so the broad app backfill lane exercises its aria-expanded regression coverage without quarantine drift. FNXC:DashboardTestQuarantine 2026-06-16-19:21: FN-6496 merge verification observed github-tracking-hook fail during the changed-test backfill shard with temp-directory cleanup ENOTEMPTY, then pass on isolated rerun. Quarantine the cleanup-flaky file under the deletion ratchet rather than changing production or test timing outside the chat-streaming scope. */ const quarantinedDashboardTests: string[] = [ - "app/components/__tests__/QuickEntryBox.test.tsx", "src/__tests__/github-tracking-hook.test.ts", ]; diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json index 5068e512b8..0d4c12f453 100644 --- a/scripts/lib/test-quarantine.json +++ b/scripts/lib/test-quarantine.json @@ -1,11 +1,6 @@ { "$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 exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.", "entries": [ - { - "file": "packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx", - "reason": "FN-6496 verification: pnpm test failed in QuickEntryBox expanded-mode tests (expected aria-expanded=false, received true) while FN-6496 only changed chat streaming hooks; direct isolated rerun of this file passed, so classify as unrelated flaky state leakage. Failing command: pnpm test; confirming command: pnpm --filter @fusion/dashboard exec vitest run app/components/__tests__/QuickEntryBox.test.tsx --reporter=dot --silent=passed-only.", - "quarantinedAt": "2026-06-16" - }, { "file": "packages/dashboard/src/__tests__/github-tracking-hook.test.ts", "reason": "FN-6496 merge verification: pnpm test failed in dashboard-api-quality-backfill with ENOTEMPTY while removing a temp task directory; isolated rerun of the file passed, so classify as unrelated cleanup flake. Failing command: pnpm test; confirming command: pnpm --filter @fusion/dashboard exec vitest run src/__tests__/github-tracking-hook.test.ts --reporter=dot --silent=passed-only.",