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
This commit is contained in:
@@ -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": "<repo-relative test path>", "reason": "<owning cleanup/quarantine task and rationale>", "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`.
|
||||
|
||||
@@ -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",
|
||||
|
||||
49
scripts/__tests__/check-no-test-timeout-appeasement.test.mjs
Normal file
49
scripts/__tests__/check-no-test-timeout-appeasement.test.mjs
Normal file
@@ -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/);
|
||||
});
|
||||
119
scripts/check-no-test-timeout-appeasement.mjs
Executable file
119
scripts/check-no-test-timeout-appeasement.mjs
Executable file
@@ -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();
|
||||
}
|
||||
10
scripts/lib/test-timeout-appeasement-allowlist.json
Normal file
10
scripts/lib/test-timeout-appeasement-allowlist.json
Normal file
@@ -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"
|
||||
}
|
||||
]
|
||||
}
|
||||
Reference in New Issue
Block a user