FN-6382: unquarantine stabilized flaky tests
Restore quarantined tests by fixing their flaky harness seams instead of extending the deletion ratchet. - Mark active Vitest worker roots and skip live worker roots during prune cleanup. - Make bubblewrap backend coverage deterministic with an injectable runner and restore it to the engine gate. - Remove rescued core and bubblewrap tests from the quarantine ledger and Vitest excludes. Files changed: .../core/src/__test-utils__/vitest-teardown.ts | 10 +++++- packages/core/vitest.config.ts | 8 +---- .../__tests__/sandbox/bubblewrap-backend.test.ts | 29 +++++++++------ packages/engine/src/sandbox/bubblewrap-backend.ts | 9 +++-- packages/engine/vitest.config.ts | 1 - scripts/__tests__/test-changed.test.mjs | 23 ++++++++++++ scripts/lib/test-quarantine.json | 34 ++---------------- scripts/test-changed.mjs | 41 ++++++++++++++++++++++ 8 files changed, 102 insertions(+), 53 deletions(-) Fusion-Task-Id: FN-6382 Fusion-Task-Lineage: 018dc7ac-1ef1-495e-a5fd-96ea44fcd43b
This commit is contained in:
@@ -6,10 +6,12 @@
|
||||
* the run-local worker/home directories as leaks.
|
||||
*/
|
||||
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join, resolve } from "node:path";
|
||||
|
||||
export const WORKER_ROOT_OWNER_FILE = ".fusion-test-worker-root-owner";
|
||||
|
||||
let workerRootRmSync = rmSync;
|
||||
let workerRootSleepMsSync = sleepMsSync;
|
||||
|
||||
@@ -54,6 +56,12 @@ export default function setup(): () => Promise<void> {
|
||||
// setup-time redirect sweep proportional to stale directories left by every
|
||||
// prior interrupted run.
|
||||
const workerRoot = resolve(mkdtempSync(join(tmpdir(), "fusion-test-workers-")));
|
||||
try {
|
||||
writeFileSync(join(workerRoot, WORKER_ROOT_OWNER_FILE), `${process.pid}\n`);
|
||||
} catch {
|
||||
// Best effort only. The marker protects active roots from external orphan
|
||||
// pruning; teardown still owns this root by absolute path.
|
||||
}
|
||||
process.env.FUSION_TEST_WORKER_ROOT = workerRoot;
|
||||
|
||||
return async function teardown() {
|
||||
|
||||
@@ -14,13 +14,7 @@ export default defineConfig({
|
||||
},
|
||||
test: {
|
||||
include: ["src/**/*.test.ts"],
|
||||
exclude: [
|
||||
"src/__tests__/soft-delete-tasks.test.ts",
|
||||
"src/__tests__/store-get-task-columns.test.ts",
|
||||
"src/__tests__/store-create-summarize-deferred-hook.test.ts",
|
||||
"src/__tests__/task-dependency-mutation.test.ts",
|
||||
"src/__tests__/task-node-override.test.ts",
|
||||
],
|
||||
exclude: [],
|
||||
setupFiles: [
|
||||
"./src/__test-utils__/vitest-setup.ts",
|
||||
],
|
||||
|
||||
@@ -65,11 +65,17 @@ describe("BubblewrapBackend", () => {
|
||||
expect(nativeStub.run).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it(
|
||||
"attempts bwrap execution when available",
|
||||
async () => {
|
||||
detectMock.mockResolvedValue({ available: true, path: "bwrap" });
|
||||
const backend = new BubblewrapBackend();
|
||||
it("attempts bwrap execution when available", async () => {
|
||||
detectMock.mockResolvedValue({ available: true, path: "/usr/bin/test-bwrap" });
|
||||
const runBwrap = vi.fn(async (): Promise<SandboxRunResult> => ({
|
||||
stdout: "hello\n",
|
||||
stderr: "",
|
||||
exitCode: 0,
|
||||
signal: null,
|
||||
timedOut: false,
|
||||
bufferExceeded: false,
|
||||
}));
|
||||
const backend = new BubblewrapBackend(undefined, runBwrap);
|
||||
await backend.prepare({ allowNetwork: true });
|
||||
|
||||
const result = await backend.run("echo hello", {
|
||||
@@ -79,11 +85,14 @@ describe("BubblewrapBackend", () => {
|
||||
encoding: "utf-8",
|
||||
});
|
||||
|
||||
expect(result).toHaveProperty("stdout");
|
||||
expect(result).toHaveProperty("stderr");
|
||||
},
|
||||
10_000,
|
||||
);
|
||||
expect(result.stdout).toBe("hello\n");
|
||||
expect(runBwrap).toHaveBeenCalledOnce();
|
||||
const [command, args] = runBwrap.mock.calls[0];
|
||||
expect(command).toBe("/usr/bin/test-bwrap");
|
||||
expect(args.at(-3)).toBe("/bin/sh");
|
||||
expect(args.at(-2)).toBe("-lc");
|
||||
expect(args.at(-1)).toBe("echo hello");
|
||||
});
|
||||
|
||||
it.skipIf(process.platform !== "linux" || !hasBwrap)("runs real bubblewrap hello integration", async () => {
|
||||
vi.doUnmock("../../sandbox/bubblewrap-detect.js");
|
||||
|
||||
@@ -17,6 +17,7 @@ import type {
|
||||
const execAsync = promisify(exec);
|
||||
|
||||
type FailureMode = "fail-hard" | "fallback-native";
|
||||
type BubblewrapRunner = (command: string, args: string[], options: SandboxRunOptions) => Promise<SandboxRunResult>;
|
||||
|
||||
export class SandboxUnavailableError extends Error {
|
||||
constructor(message: string) {
|
||||
@@ -30,7 +31,10 @@ export class BubblewrapBackend implements SandboxBackend {
|
||||
private useNativeFallback = false;
|
||||
private pnpmStorePathByCwd = new Map<string, string>();
|
||||
|
||||
constructor(private readonly nativeBackend: SandboxBackend = new NativeSandboxBackend()) {}
|
||||
constructor(
|
||||
private readonly nativeBackend: SandboxBackend = new NativeSandboxBackend(),
|
||||
private readonly bwrapRunner?: BubblewrapRunner,
|
||||
) {}
|
||||
|
||||
capabilities(): SandboxCapabilities {
|
||||
return {
|
||||
@@ -87,7 +91,8 @@ export class BubblewrapBackend implements SandboxBackend {
|
||||
});
|
||||
|
||||
const bwrapPath = detect.path ?? "bwrap";
|
||||
return this.runBwrapSpawn(bwrapPath, [...policyArgs, "--", "/bin/sh", "-lc", command], options);
|
||||
const bwrapArgs = [...policyArgs, "--", "/bin/sh", "-lc", command];
|
||||
return (this.bwrapRunner ?? this.runBwrapSpawn.bind(this))(bwrapPath, bwrapArgs, options);
|
||||
}
|
||||
|
||||
async runStreaming(command: string, options: SandboxRunStreamingOptions): Promise<SandboxStreamingResult> {
|
||||
|
||||
@@ -105,7 +105,6 @@ export default defineConfig({
|
||||
"src/__tests__/merger-ai-cleanup-active-session.test.ts",
|
||||
"src/__tests__/merger-ai-cleanup.test.ts",
|
||||
"src/__tests__/merger-ai.test.ts",
|
||||
"src/__tests__/sandbox/bubblewrap-backend.test.ts",
|
||||
],
|
||||
},
|
||||
},
|
||||
|
||||
@@ -1035,6 +1035,29 @@ function withPersistentPruneFailure(root, pruneFn) {
|
||||
}
|
||||
}
|
||||
|
||||
test("pruneFusionTestWorkers: skips active per-invocation worker roots", () => {
|
||||
const root = createNonEmptyPruneRoot("fusion-test-workers-", "active");
|
||||
try {
|
||||
writeFileSync(path.join(root, ".fusion-test-worker-root-owner"), `${process.pid}\n`);
|
||||
pruneFusionTestWorkers(1024);
|
||||
assert.equal(existsSync(root), true, "active worker root must not be pruned");
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("pruneFusionTestWorkers: skips markerless roots with live redirect sinks", () => {
|
||||
const root = mkdtempSync(path.join(tmpdir(), `fusion-test-workers-active-redir-${process.pid}-`));
|
||||
try {
|
||||
mkdirSync(path.join(root, `redir-${process.pid}`), { recursive: true });
|
||||
writeFileSync(path.join(root, `redir-${process.pid}`, "payload.txt"), "active\n");
|
||||
pruneFusionTestWorkers(1024);
|
||||
assert.equal(existsSync(root), true, "live redir-pid root must not be pruned");
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("pruneFusionTestWorkers: reclaims non-empty root after transient ENOTEMPTY", () => {
|
||||
const root = createNonEmptyPruneRoot("fusion-test-workers-", "transient");
|
||||
withTransientPruneFailure(root, pruneFusionTestWorkers);
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
{
|
||||
"$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.",
|
||||
"$comment": "Flaky-test quarantine ledger (deletion ratchet \u2014 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 \u2014 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/engine/src/__tests__/merger-ai-cleanup-active-session.test.ts",
|
||||
"reason": "Flake: pruneExistingAiMergeWorktrees skips active-session paths — active-session temp AI merge dir was unexpectedly pruned during pnpm --filter @fusion/engine test in FN-6206 verification, while the same file passed standalone. Root cause suspected: realpathSync resolution mismatch or readdirSync mock interaction with activeSessionRegistry singleton under concurrent engine suite load. Discovered during FN-6206.",
|
||||
"reason": "Flake: pruneExistingAiMergeWorktrees skips active-session paths \u2014 active-session temp AI merge dir was unexpectedly pruned during pnpm --filter @fusion/engine test in FN-6206 verification, while the same file passed standalone. Root cause suspected: realpathSync resolution mismatch or readdirSync mock interaction with activeSessionRegistry singleton under concurrent engine suite load. Discovered during FN-6206.",
|
||||
"quarantinedAt": "2026-06-10"
|
||||
},
|
||||
{
|
||||
@@ -26,36 +26,6 @@
|
||||
"reason": "Flake observed during FN-6294 verification and reproduced during FN-6319 broad `pnpm --filter @fusion/engine test`: `clearStaleBlockedBy handles missed task:deleted event with soft-deleted-blocker reason` failed because the log entry was absent, while the same file passed standalone and the narrow three-file reproduction passed. Product-code cross-check: `clearStaleBlockedBy` still has the soft-deleted-blocker branch and soft-delete-deadlock-scan-exclusion.test.ts covers it via a deterministic store double, indicating suite-order/concurrency sensitivity in this reliability-interactions fixture rather than a confirmed product bug.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/engine/src/__tests__/sandbox/bubblewrap-backend.test.ts",
|
||||
"reason": "Flake observed during FN-6294 verification: `attempts bwrap execution when available` timed out in the broad and narrow engine runs, while the file passed standalone during FN-6319. The test mocks detectBwrap as available with path `bwrap` and then invokes real bwrap execution, making it host/environment sensitive when a real bwrap binary is unavailable or behaves differently under suite load.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/core/src/__tests__/soft-delete-tasks.test.ts",
|
||||
"reason": "Flake observed during FN-6324 verification: broad `pnpm test` failed with `ENOENT: no such file or directory, mkdtemp .../fusion-test-workers-.../redir-.../kb-store-test-XXXXXX`, alongside a leaked fusion-test-workers temp root. The task only changed agent role policy/settings/heartbeat routing, so this temp redirect failure is unrelated suite-order/concurrency sensitivity.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/core/src/__tests__/store-get-task-columns.test.ts",
|
||||
"reason": "Flake observed during FN-6324 verification: broad `pnpm test` failed with `ENOENT` renaming a task.json temp file under the redirected fusion-test-workers temp root after the temp tree disappeared. The task only changed agent role policy/settings/heartbeat routing, so this is unrelated temp redirect suite-order/concurrency sensitivity.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/core/src/__tests__/task-dependency-mutation.test.ts",
|
||||
"reason": "Flake observed during FN-6324 verification: broad `pnpm test` failed with `ENOENT` reading task.json under a redirected fusion-test-workers temp root that had disappeared. The task only changed agent role policy/settings/heartbeat routing, so this is unrelated temp redirect suite-order/concurrency sensitivity.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/core/src/__tests__/task-node-override.test.ts",
|
||||
"reason": "Flake observed during FN-6324 verification: broad `pnpm test` failed with `Task FN-001 not found` after temp-root disappearance symptoms in adjacent core tests, alongside a leaked fusion-test-workers temp root. The task only changed agent role policy/settings/heartbeat routing, so this is unrelated temp redirect suite-order/concurrency sensitivity.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/core/src/__tests__/store-create-summarize-deferred-hook.test.ts",
|
||||
"reason": "Flake observed during FN-6320 final broad `pnpm test`: `store-create.test.ts > TaskStore > createTask with title summarization > defers the task-created hook until store-managed summarize completes` timed out because the registered task-created hook had zero calls after the gated store-managed summarizer prompt was released. FN-6326 cross-check: the test passed twice standalone after FN-6313, and product code in `TaskStore.createTask` suppresses the synchronous hook only while `hasPendingSummarization` is true, then unconditionally refreshes the task and calls `invokeTaskCreatedHook(latestTask)` after `onSummarize` settles across success/null/throw branches. The broad/package load failure was therefore classified as suite-load/harness sensitivity rather than a confirmed product defect; the single flaky `it` was extracted so the rest of `store-create.test.ts` remains covered.",
|
||||
"quarantinedAt": "2026-06-12"
|
||||
},
|
||||
{
|
||||
"file": "packages/dashboard/src/__tests__/routes-settings.test.ts",
|
||||
"reason": "Flake observed during FN-6354 broad `pnpm test`: `GET /api/memory/audit > preserves extraction metadata across extract then audit requests` received HTTP 503 instead of 200 in the dashboard api:curated lane, while the same named test passed standalone immediately afterward. FN-6354 only changed the task-detail Chat composer UI/tests, so this is classified as unrelated suite-order/concurrency sensitivity in the dashboard API quality lane.",
|
||||
|
||||
@@ -173,11 +173,51 @@ let cleanupRmSync = rmSync;
|
||||
const PRUNE_REMOVE_RETRIES = 3;
|
||||
const PRUNE_REMOVE_DELAY_MS = 75;
|
||||
const PRUNE_DIAGNOSTIC_CHILD_LIMIT = 8;
|
||||
const FUSION_WORKER_ROOT_OWNER_FILE = ".fusion-test-worker-root-owner";
|
||||
|
||||
function isEnoentError(err) {
|
||||
return Boolean(err && typeof err === "object" && "code" in err && err.code === "ENOENT");
|
||||
}
|
||||
|
||||
function isProcessAlive(pid) {
|
||||
if (!Number.isInteger(pid) || pid <= 0) return false;
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true;
|
||||
} catch (error) {
|
||||
return error && typeof error === "object" && error.code === "EPERM";
|
||||
}
|
||||
}
|
||||
|
||||
function readWorkerRootOwnerPid(rootPath) {
|
||||
try {
|
||||
const raw = readFileSync(path.join(rootPath, FUSION_WORKER_ROOT_OWNER_FILE), "utf8").trim();
|
||||
const pid = Number.parseInt(raw, 10);
|
||||
return Number.isInteger(pid) && pid > 0 ? pid : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function isActiveFusionWorkerRoot(rootPath) {
|
||||
const ownerPid = readWorkerRootOwnerPid(rootPath);
|
||||
if (ownerPid !== null && isProcessAlive(ownerPid)) return true;
|
||||
|
||||
// Backward-compatible guard for worker roots created before the owner marker
|
||||
// landed, or marker writes that failed: an alive redir-<pid> child means a
|
||||
// Vitest worker still owns temp workspaces beneath this root.
|
||||
try {
|
||||
for (const child of readdirSync(rootPath, { withFileTypes: true })) {
|
||||
if (!child.isDirectory()) continue;
|
||||
const match = /^redir-(\d+)$/.exec(child.name);
|
||||
if (match && isProcessAlive(Number.parseInt(match[1], 10))) return true;
|
||||
}
|
||||
} catch {
|
||||
// If we cannot inspect it, fall through to normal best-effort pruning.
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function listImmediateChildrenForPruneWarning(rootPath) {
|
||||
try {
|
||||
const children = readdirSync(rootPath).slice(0, PRUNE_DIAGNOSTIC_CHILD_LIMIT);
|
||||
@@ -235,6 +275,7 @@ function pruneFusionTestRoots(prefix, maxEntries = PRUNE_MAX_ENTRIES, retryOptio
|
||||
} catch {
|
||||
// Keep raw path fallback.
|
||||
}
|
||||
if (isActiveFusionWorkerRoot(rawPath)) continue;
|
||||
removePrunedRootWithRetry(rawPath, retryOptions);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user