From 1f660f3e8a7aa766c7e259084b0f915038476308 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 14 Jun 2026 03:38:44 -0700 Subject: [PATCH] FN-6434: block Vitest timeout appeasement Add a fast guard that rejects Vitest timeout bumps in tracked test files. - Add a test-timeout appeasement scanner with a temporary allowlist for legacy exemptions. - Run the scanner in pretest, pretest:full, and test:gate so merge gates catch timeout bumps. - Cover the scanner behavior with node:test cases and document the policy/remediation path. Files changed: docs/testing.md | 6 ++ package.json | 6 +- .../check-no-test-timeout-appeasement.test.mjs | 49 +++++++++ scripts/check-no-test-timeout-appeasement.mjs | 119 +++++++++++++++++++++ .../lib/test-timeout-appeasement-allowlist.json | 10 ++ 5 files changed, 187 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-6434 Fusion-Task-Lineage: deb46a27-b0c9-4644-b8bb-34ce98e7acde --- docs/testing.md | 6 + package.json | 6 +- ...check-no-test-timeout-appeasement.test.mjs | 49 ++++++++ scripts/check-no-test-timeout-appeasement.mjs | 119 ++++++++++++++++++ .../test-timeout-appeasement-allowlist.json | 10 ++ 5 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 scripts/__tests__/check-no-test-timeout-appeasement.test.mjs create mode 100755 scripts/check-no-test-timeout-appeasement.mjs create mode 100644 scripts/lib/test-timeout-appeasement-allowlist.json diff --git a/docs/testing.md b/docs/testing.md index 8fa7babb6f..b6e64e1ca7 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -148,6 +148,12 @@ Flaky tests are quarantined ON SIGHT and deleted on a 2-week clock. This is writ **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). +### Vitest timeout-appeasement guard + +`scripts/check-no-test-timeout-appeasement.mjs` runs in the fast `pretest`, `pretest:full`, and `test:gate` paths. It scans tracked `packages/**/*.test.*` and `plugins/**/*.test.*` files for per-file or suite-level Vitest timeout bumps, including `vi.setConfig({ testTimeout: ... })`, `vi.setConfig({ hookTimeout: ... })`, and bare `testTimeout:` / `hookTimeout:` properties in test files. It deliberately ignores global `vitest.config.*` timeouts. + +Legitimate legacy exceptions must be recorded in `scripts/lib/test-timeout-appeasement-allowlist.json` as `{ "file": "", "reason": "", "allowlistedAt": "YYYY-MM-DD" }`. Allowlisting is temporary: the real fix is to quarantine the flaky test or narrow the slow seam, then remove both the timeout bump and the allowlist entry. + **CLI shared-fixture rescue pattern (FN-6430):** the 2026-06-14 `@runfusion/fusion` quarantine batch passed direct runs but timed out or bled state only under package/workspace load. The rescue fixed the shared isolation seam, not the timeout: sweep stale top-level `fn-test-home-*` roots with a bounded one-level prefix scan, reject inherited `HOME` values that do not live under the current `fusion-test-workers-*` root, recreate/remark the worker root before each `mkdtemp`, reset module/singleton fixture state in the affected suites, close real stores created by research helpers, and narrow slow real-store seams by moving package imports out of timed test bodies. When rescuing a similar CLI batch, prove it with repeated rescued-file runs plus `pnpm --filter @runfusion/fusion test`, audit rescued files for `vi.setConfig`/`testTimeout`/`hookTimeout` appeasement, and keep ledger/config removals in the same commit. **Non-CLI quarantine sweep pattern (FN-6433):** for engine/core/dashboard batches, first remove quarantine excludes only in temporary local configs and run the exact quarantined files together so suite-load coupling is visible before editing the ledger. Rescue is valid when the grouped package lane proves the invariant now holds (for example, FN-6433 fixed engine cross-file interference by replacing broad `activeSessionRegistry.clear()` cleanup with path-scoped unregistering) or when a prior shared-fixture fix is demonstrated under package load. Delete duplicate/low-value files under the ratchet when another deterministic suite owns the same invariant. Finish by making `scripts/lib/test-quarantine.json` and every package Vitest exclude array converge in one commit, then prove the empty/non-empty state with package lanes, `pnpm test:gate`, `pnpm test`, `pnpm build`, and the bounded temp-leak output from `pnpm test`. diff --git a/package.json b/package.json index 0059835066..05ea337259 100644 --- a/package.json +++ b/package.json @@ -14,9 +14,9 @@ "type": "module", "packageManager": "pnpm@10.33.0", "scripts": { - "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", - "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", - "test:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs && pnpm --filter @fusion/engine test:core && pnpm --filter @runfusion/fusion test:ci-shape", + "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-test-timeout-appeasement.mjs", + "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-test-timeout-appeasement.mjs", + "test:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-test-timeout-appeasement.mjs && pnpm --filter @fusion/engine test:core && pnpm --filter @runfusion/fusion test:ci-shape", "smoke:boot": "node scripts/boot-smoke.mjs", "local": "node scripts/start-local.mjs", "dev": "node scripts/dev-with-memory.mjs", diff --git a/scripts/__tests__/check-no-test-timeout-appeasement.test.mjs b/scripts/__tests__/check-no-test-timeout-appeasement.test.mjs new file mode 100644 index 0000000000..b9bc4c9bac --- /dev/null +++ b/scripts/__tests__/check-no-test-timeout-appeasement.test.mjs @@ -0,0 +1,49 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { formatFailureMessage, scanFileContent } from "../check-no-test-timeout-appeasement.mjs"; + +const emptyAllowlist = { allowlistEntries: [] }; + +test("scanFileContent reports vi.setConfig testTimeout bumps", () => { + const source = ["import { vi } from 'vitest';", "vi.setConfig({ testTimeout: 30000 });"].join("\n"); + const matches = scanFileContent(source, "packages/x/src/a.test.ts", emptyAllowlist); + assert.equal(matches.length, 1); + assert.equal(matches[0].lineNumber, 2); + assert.match(matches[0].line, /testTimeout/); +}); + +test("scanFileContent reports hookTimeout bumps", () => { + const matches = scanFileContent("vi.setConfig({ hookTimeout: 30000 });", "packages/x/src/a.test.ts", emptyAllowlist); + assert.equal(matches.length, 1); + assert.equal(matches[0].lineNumber, 1); + assert.match(matches[0].line, /hookTimeout/); +}); + +test("scanFileContent ignores allowlisted files with a rationale", () => { + const matches = scanFileContent("vi.setConfig({ testTimeout: 30000 });", "packages/x/src/a.test.ts", { + allowlistEntries: [ + { + file: "packages/x/src/a.test.ts", + reason: "legacy timeout pending FN-0000 removal", + }, + ], + }); + assert.equal(matches.length, 0); +}); + +test("scanFileContent ignores global vitest config timeouts and non-test paths", () => { + const configMatches = scanFileContent("testTimeout: 30_000,", "packages/x/vitest.config.ts", emptyAllowlist); + const sourceMatches = scanFileContent("testTimeout: 30_000,", "packages/x/src/config.ts", emptyAllowlist); + assert.equal(configMatches.length, 0); + assert.equal(sourceMatches.length, 0); +}); + +test("formatFailureMessage cites file, line, quarantine remediation, and allowlist", () => { + const message = formatFailureMessage([ + { filePath: "packages/x/src/a.test.ts", lineNumber: 3, line: "vi.setConfig({ testTimeout: 30000 });" }, + ]); + assert.match(message, /packages\/x\/src\/a\.test\.ts:3/); + assert.match(message, /scripts\/lib\/test-quarantine\.json/); + assert.match(message, /Do Not Add Slow Tests/); + assert.match(message, /scripts\/lib\/test-timeout-appeasement-allowlist\.json/); +}); diff --git a/scripts/check-no-test-timeout-appeasement.mjs b/scripts/check-no-test-timeout-appeasement.mjs new file mode 100755 index 0000000000..e12af463b7 --- /dev/null +++ b/scripts/check-no-test-timeout-appeasement.mjs @@ -0,0 +1,119 @@ +#!/usr/bin/env node +/* +FNXC:TestHygiene 2026-06-14-03:15: +Repo policy forbids hiding slow or flaky Vitest suites with file-level or suite-level timeout bumps. +This guard blocks new `testTimeout` and `hookTimeout` appeasement in tracked test files, while a dated allowlist records temporary legacy exemptions that must link to the owning cleanup or quarantine work. +*/ +import { readFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +export const ALLOWLIST_PATH = "scripts/lib/test-timeout-appeasement-allowlist.json"; +const SCAN_ROOTS = ["packages", "plugins"]; +const TEST_FILE_PATTERN = /\.test\.(?:ts|tsx|mts|cts|mjs|cjs|js|jsx)$/; +const VITEST_CONFIG_PATTERN = /(?:^|\/)vitest\.config\.[mc]?[jt]s$/; +const TIMEOUT_PROPERTY_PATTERN = /\b(?:testTimeout|hookTimeout)\s*:/; + +function isTestFile(filePath) { + return TEST_FILE_PATTERN.test(filePath) && !VITEST_CONFIG_PATTERN.test(filePath); +} + +function listTrackedTargets() { + const result = spawnSync("git", ["ls-files", "--", ...SCAN_ROOTS], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) { + throw new Error(result.stderr?.trim() || "git ls-files failed"); + } + return result.stdout + .split("\n") + .map((line) => line.trim()) + .filter(Boolean) + .filter(isTestFile); +} + +function loadAllowlistEntries(allowlistPath = ALLOWLIST_PATH) { + let parsed; + try { + parsed = JSON.parse(readFileSync(allowlistPath, "utf8")); + } catch (error) { + throw new Error(`Failed to read ${allowlistPath}: ${error instanceof Error ? error.message : String(error)}`); + } + + if (!Array.isArray(parsed.entries)) { + throw new Error(`${allowlistPath} must contain an entries array`); + } + + return parsed.entries; +} + +function buildAllowlistedFiles(entries) { + const files = new Set(); + for (const [index, entry] of entries.entries()) { + if (!entry || typeof entry.file !== "string" || entry.file.trim() === "") { + throw new Error(`${ALLOWLIST_PATH} entries[${index}] must include a non-empty file`); + } + if (typeof entry.reason !== "string" || entry.reason.trim() === "") { + throw new Error(`${ALLOWLIST_PATH} entries[${index}] for ${entry.file} must include a non-empty reason`); + } + files.add(entry.file); + } + return files; +} + +export function scanFileContent(content, filePath, options = {}) { + if (!isTestFile(filePath)) return []; + + const allowlistedFiles = options.allowlistedFiles ?? buildAllowlistedFiles(options.allowlistEntries ?? []); + if (allowlistedFiles.has(filePath)) return []; + + const matches = []; + const lines = content.split(/\r?\n/); + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]; + if (TIMEOUT_PROPERTY_PATTERN.test(line)) { + matches.push({ filePath, lineNumber: index + 1, line }); + } + } + return matches; +} + +export function scanTrackedFiles(files = listTrackedTargets(), options = {}) { + const allowlistedFiles = options.allowlistedFiles ?? buildAllowlistedFiles(options.allowlistEntries ?? loadAllowlistEntries()); + const matches = []; + for (const filePath of files) { + if (!isTestFile(filePath) || allowlistedFiles.has(filePath)) continue; + let content; + try { + content = readFileSync(filePath, "utf8"); + } catch { + continue; + } + matches.push(...scanFileContent(content, filePath, { allowlistedFiles })); + } + return matches; +} + +export function formatFailureMessage(matches) { + const lines = matches.map( + ({ filePath, lineNumber, line }) => `${filePath}:${lineNumber}: ${line.trim()}`, + ); + return [ + "[check-no-test-timeout-appeasement] found Vitest timeout appeasement in tracked test files.", + "Do not raise per-file/suite timeouts to mask slow/flaky tests — quarantine via `scripts/lib/test-quarantine.json` or narrow the seam; see AGENTS.md 'Do Not Add Slow Tests'.", + `For legitimately exempt legacy cases, add a dated rationale to ${ALLOWLIST_PATH}; exemptions are temporary and should point at the owning cleanup task.`, + ...lines, + ].join("\n"); +} + +export function main() { + const matches = scanTrackedFiles(); + if (matches.length === 0) return 0; + console.error(formatFailureMessage(matches)); + return 1; +} + +if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { + process.exitCode = main(); +} diff --git a/scripts/lib/test-timeout-appeasement-allowlist.json b/scripts/lib/test-timeout-appeasement-allowlist.json new file mode 100644 index 0000000000..c13335713e --- /dev/null +++ b/scripts/lib/test-timeout-appeasement-allowlist.json @@ -0,0 +1,10 @@ +{ + "$comment": "Vitest timeout-appeasement allowlist (temporary exemption ledger — see AGENTS.md 'Do Not Add Slow Tests' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). The guard blocks new per-file or suite-level `testTimeout` / `hookTimeout` bumps in tracked test files. Every entry needs a repo-relative `file`, non-empty `reason` linking the owning cleanup/quarantine work, and `allowlistedAt` date. The goal is removal, not permanence: quarantine the flaky test or narrow the slow seam, then delete the timeout bump and this entry.", + "entries": [ + { + "file": "packages/cli/src/__tests__/extension-integration.test.ts", + "reason": "Pre-existing file-wide Vitest timeout appeasement identified during the FN-6430/FN-6434 CLI quarantine sweep; temporarily exempt while a follow-up removes or narrows this integration seam.", + "allowlistedAt": "2026-06-14" + } + ] +}