From 28f9b1948be8604e1950b4a2a53ed710e3cb08a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 22 Oct 2026 05:33:09 +0200 Subject: [PATCH] test: a flaky-test quarantine with an expiry the guard enforces tests/quarantine/quarantine.json is the only way to take a flaky unit or integration test out of a run. An entry names the file, the full test name, the issue, the reason, the day it was added and the day it expires, at most 14 days later. Both setup files skip a listed test while its entry is live, reported as skipped with its issue rather than retried into green. Past its expiry date the test runs again, and quarantine-guard.test.ts fails until the entry is removed; it also fails on an entry naming a file or test that does not exist. The list starts empty. --- tests/integration/environment-setup.ts | 11 +- tests/quarantine/quarantine-guard.test.ts | 128 ++++++++++++++++++++++ tests/quarantine/quarantine.json | 4 + tests/quarantine/quarantine.ts | 78 +++++++++++++ vitest.setup.ts | 14 ++- 5 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 tests/quarantine/quarantine-guard.test.ts create mode 100644 tests/quarantine/quarantine.json create mode 100644 tests/quarantine/quarantine.ts diff --git a/tests/integration/environment-setup.ts b/tests/integration/environment-setup.ts index 9adf0e3c7..601b15715 100644 --- a/tests/integration/environment-setup.ts +++ b/tests/integration/environment-setup.ts @@ -1,5 +1,6 @@ -import { afterEach, inject, vi } from "vitest"; +import { afterEach, beforeEach, inject, vi } from "vitest"; +import { quarantineEntryFor } from "../quarantine/quarantine"; import { prepareWorkerDatabase } from "./worker-database"; // Each worker runs against its own clone of the migrated template (see @@ -35,6 +36,14 @@ afterEach(async () => { await settleBackgroundTasks(); }); +/** The flaky-test quarantine, as in the unit setup (`vitest.setup.ts`). */ +beforeEach((context) => { + const entry = quarantineEntryFor(context.task); + if (entry) { + context.skip(`quarantined until ${entry.expires} (${entry.issue})`); + } +}); + /** * Route handlers read cookies and headers through `next/headers`, which only * works inside a Next.js request scope. Every file gets the same stand-in, diff --git a/tests/quarantine/quarantine-guard.test.ts b/tests/quarantine/quarantine-guard.test.ts new file mode 100644 index 000000000..f29559468 --- /dev/null +++ b/tests/quarantine/quarantine-guard.test.ts @@ -0,0 +1,128 @@ +/** + * The quarantine list cannot outlive its excuses. + * + * Every entry names an issue, a reason, the day it was added, and a day it + * expires, at most two weeks later. Past that day this guard fails, and the + * test runs again in both suites whether or not the entry is removed, so a + * flake that nobody fixed becomes a red build instead of a permanent skip. + * The file and the test the entry names must exist, so an entry cannot + * quietly cover a test that was renamed. + */ +import { existsSync, readFileSync } from "node:fs"; +import { resolve } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { + MAX_QUARANTINE_DAYS, + fullName, + isActive, + readQuarantine, + type QuarantineEntry, + type TaskLike, +} from "./quarantine"; + +const ROOT = resolve(import.meta.dirname, "..", ".."); +const ISO_DATE = /^\d{4}-\d{2}-\d{2}$/; + +function problems(entry: QuarantineEntry, today: Date): string[] { + const found: string[] = []; + for (const field of [ + "file", + "test", + "issue", + "reason", + "added", + "expires", + ] as const) { + if (typeof entry[field] !== "string" || entry[field].trim() === "") { + found.push(`missing ${field}`); + } + } + if (found.length > 0) return found; + if (!/^#\d+$|^https:\/\//.test(entry.issue)) { + found.push(`issue must be "#" or a URL, got "${entry.issue}"`); + } + if (!ISO_DATE.test(entry.added) || !ISO_DATE.test(entry.expires)) { + found.push("added and expires must be YYYY-MM-DD"); + return found; + } + const days = + (Date.parse(`${entry.expires}T00:00:00Z`) - + Date.parse(`${entry.added}T00:00:00Z`)) / + 86_400_000; + if (days < 0 || days > MAX_QUARANTINE_DAYS) { + found.push( + `expires ${days} days after it was added; the limit is ${MAX_QUARANTINE_DAYS}`, + ); + } + if (!isActive(entry, today)) { + found.push( + `expired on ${entry.expires}: fix the test or delete it, then remove the entry`, + ); + } + const path = resolve(ROOT, entry.file); + if (!existsSync(path)) { + found.push(`${entry.file} does not exist`); + } else { + const leaf = entry.test.split(" > ").pop() ?? ""; + if (!readFileSync(path, "utf8").includes(leaf)) { + found.push(`${entry.file} has no test named "${leaf}"`); + } + } + return found; +} + +describe("the flaky-test quarantine", () => { + it("holds only live, complete, bounded entries", () => { + const today = new Date(); + const report = readQuarantine().flatMap((entry) => + problems(entry, today).map((p) => `${entry.file} > ${entry.test}: ${p}`), + ); + expect(report).toEqual([]); + }); + + it("names each test once", () => { + const keys = readQuarantine().map((e) => `${e.file}::${e.test}`); + expect(new Set(keys).size).toBe(keys.length); + }); + + it("refuses the entries it exists to refuse", () => { + const today = new Date("2026-10-22T09:00:00Z"); + const base: QuarantineEntry = { + file: "tests/quarantine/quarantine-guard.test.ts", + test: "the flaky-test quarantine > names each test once", + issue: "#1", + reason: "fails one run in twenty", + added: "2026-10-20", + expires: "2026-10-30", + }; + expect(problems(base, today)).toEqual([]); + expect(problems({ ...base, expires: "2026-10-21" }, today)).toEqual([ + expect.stringMatching(/^expired on 2026-10-21/), + ]); + expect(problems({ ...base, expires: "2026-11-20" }, today)).toEqual([ + expect.stringMatching(/the limit is 14/), + ]); + expect(problems({ ...base, issue: "soon" }, today)).toHaveLength(1); + // Built at run time, so the name cannot occur in this file's own source. + const gone = ["renamed", "away"].join("-"); + expect(problems({ ...base, test: `a > ${gone}` }, today)).toEqual([ + expect.stringContaining(`has no test named "${gone}"`), + ]); + // The expiry day itself still counts as quarantined. + expect(isActive(base, new Date("2026-10-30T23:00:00Z"))).toBe(true); + expect(isActive(base, new Date("2026-10-31T00:00:00Z"))).toBe(false); + }); + + it("builds the full name a test is listed under", () => { + const file: TaskLike & { filepath: string } = { + name: "tests/x.test.ts", + filepath: "/x", + }; + file.file = file; + const outer: TaskLike = { name: "outer", suite: file, file }; + const test: TaskLike = { name: "does a thing", suite: outer, file }; + expect(fullName(test)).toBe("outer > does a thing"); + }); +}); diff --git a/tests/quarantine/quarantine.json b/tests/quarantine/quarantine.json new file mode 100644 index 000000000..a85d8d02a --- /dev/null +++ b/tests/quarantine/quarantine.json @@ -0,0 +1,4 @@ +{ + "$comment": "Tests taken out of the unit and integration runs while a flake is fixed. Policy and format: docs/contributing/testing.md#flaky-tests. Every entry expires at most 14 days after it was added; tests/quarantine/quarantine-guard.test.ts fails once one has.", + "entries": [] +} diff --git a/tests/quarantine/quarantine.ts b/tests/quarantine/quarantine.ts new file mode 100644 index 000000000..11f1b7a89 --- /dev/null +++ b/tests/quarantine/quarantine.ts @@ -0,0 +1,78 @@ +/** + * The flaky-test quarantine: a list of tests skipped while their flake is + * fixed, each with an issue and an expiry date. + * + * A quarantined test is reported as skipped rather than retried into green, + * so the run stays honest about what it did not check, and the entry cannot + * outlive its excuse: once `expires` has passed the test runs again (and + * `quarantine-guard.test.ts` goes red until the entry is removed). Both + * suites apply the list from their setup files. + */ +import { readFileSync } from "node:fs"; +import { relative, resolve } from "node:path"; + +export interface QuarantineEntry { + /** Path from the repository root, e.g. `tests/integration/foo.test.ts`. */ + file: string; + /** Full name: describe blocks and the test, joined with " > ". */ + test: string; + /** Where the flake is tracked: `#123` or a URL. */ + issue: string; + /** What fails and how often, in a sentence. */ + reason: string; + /** ISO date the entry was added. */ + added: string; + /** ISO date after which the test runs again; at most 14 days after `added`. */ + expires: string; +} + +export const QUARANTINE_FILE = resolve(import.meta.dirname, "quarantine.json"); +export const MAX_QUARANTINE_DAYS = 14; +const ROOT = resolve(import.meta.dirname, "..", ".."); + +export function readQuarantine(path = QUARANTINE_FILE): QuarantineEntry[] { + const parsed = JSON.parse(readFileSync(path, "utf8")) as { + entries?: QuarantineEntry[]; + }; + return parsed.entries ?? []; +} + +/** True while the entry's expiry date (inclusive) has not passed. */ +export function isActive( + entry: QuarantineEntry, + today: Date = new Date(), +): boolean { + return today.toISOString().slice(0, 10) <= entry.expires; +} + +let cache: QuarantineEntry[] | null = null; + +export interface TaskLike { + name: string; + suite?: TaskLike | null; + file?: (TaskLike & { filepath?: string }) | null; +} + +/** Describe blocks and the test, outermost first; the file task is the root. */ +export function fullName(task: TaskLike): string { + const names: string[] = []; + let current: TaskLike | null | undefined = task; + while (current && current !== current.file) { + names.unshift(current.name); + current = current.suite; + } + return names.join(" > "); +} + +/** The active entry naming this test, if any. */ +export function quarantineEntryFor(task: TaskLike): QuarantineEntry | null { + cache ??= readQuarantine(); + if (cache.length === 0) return null; + const filepath = task.file?.filepath; + if (!filepath) return null; + const file = relative(ROOT, filepath).split("\\").join("/"); + const name = fullName(task); + return ( + cache.find((e) => e.file === file && e.test === name && isActive(e)) ?? null + ); +} diff --git a/vitest.setup.ts b/vitest.setup.ts index 5412a6d6d..7b397bdaa 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -18,8 +18,9 @@ * Server-side translation (`getServerTranslator`, `shared-resolve`) holds * every catalog on its own and needs no priming. */ -import { afterEach, vi } from "vitest"; +import { afterEach, beforeEach, vi } from "vitest"; import { settleBackgroundTasks } from "@/lib/logging/background-tasks"; +import { quarantineEntryFor } from "./tests/quarantine/quarantine"; import { primeMessages } from "@/lib/i18n/load-locale"; import enMessages from "./messages/en.json"; @@ -44,6 +45,17 @@ afterEach(async () => { await settleBackgroundTasks(); }); +/** + * Skip a test on the flaky-test quarantine list until its entry expires. See + * `tests/quarantine/quarantine.ts` and docs/contributing/testing.md. + */ +beforeEach((context) => { + const entry = quarantineEntryFor(context.task); + if (entry) { + context.skip(`quarantined until ${entry.expires} (${entry.issue})`); + } +}); + /** * The shared write ceiling is charged by every identity resolver on a write * (`src/lib/auth/write-meter.ts`), which is a database round-trip most route