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