Public: anyone can read this; the crew can change it.
Repo-wide rule tests (raw ms budgets, migration pins) skip a pull's own check, so rule breakers go green and sink merge batches (5 pulls today) #7143
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/7138-repo-wide-rule-tests-raw-ms-budgets-migr → main 2 commits fork 2 threads · 2 unresolved
8 files
changed +169
−22
docs/DEVELOP.md +8
| @@ -94,6 +94,14 @@ map has never seen fall back to the import-hop guess, and a change to | ||
| 94 | 94 | `forge.json`, `package.json`, `scripts/**` or `src/kernel/**` runs |
| 95 | 95 | everything. With no map yet the line says `by imports (no impact map)`. |
| 96 | 96 | |
| 97 | A test that reads the whole tree — every test file, every migration, the | |
| 98 | README — is reached by no diff, yet any file can break it. Those are | |
| 99 | `RULE_TESTS` in `src/checks/tiers.ts`, and every fast tier runs them | |
| 100 | whatever else the diff holds (#7138) — a diff of only the timing or impact table breaks no rule and runs none; a new rule of that kind goes on the list. Write | |
| 101 | the rule with `breaches()` from `test/helpers/rule.ts`: one red test per | |
| 102 | breach, named `path:line: …`, so the check's summary names the rule and | |
| 103 | the line. | |
| 104 | ||
| 97 | 105 | Before you push, ask what the fast tier would run for your change: |
| 98 | 106 | `files=$(bun scripts/fast.ts) && bun test $files`. `bun scripts/fast.ts |
| 99 | 107 | [--base origin/main] [--why | --null | --json]` lists the picked test files |
docs/OPERATING.md +1 −1
| @@ -181,7 +181,7 @@ A check, test or app cell asks for memory in `forge.json` (`check.limits.memory` | ||
| 181 | 181 | |
| 182 | 182 | ## Check tiers |
| 183 | 183 | |
| 184 | `forge.json`'s `check` may declare `fast` beside `run` (the full gate; `full` names it explicitly). `bun scripts/test-shards.ts --tier fast --diff <base>..<head>` runs the tests that touch the diff (changed, named for a changed source file, or importing one within two hops), every test the times table records under 1 s, and any named by `--learned a,b`. `src/checks/tiers.ts` decides; it adds no dependency. | |
| 184 | `forge.json`'s `check` may declare `fast` beside `run` (the full gate; `full` names it explicitly). `bun scripts/test-shards.ts --tier fast --diff <base>..<head>` runs the tests that touch the diff (changed, named for a changed source file, or importing one within two hops), every repo-wide rule test (`RULE_TESTS`, #7138), every test the times table records under 1 s, and any named by `--learned a,b`. `src/checks/tiers.ts` decides; it adds no dependency. | |
| 185 | 185 | |
| 186 | 186 | To see what the fast tier would run for your own change before you push: `files=$(bun scripts/fast.ts) && bun test $files`. `bun scripts/fast.ts [--base origin/main] [--why | --null | --json]` lists, one per line on stdout and nothing else there, the test files `fastSet` picks for the files changed between the base's merge-base and the working tree (committed, uncommitted and untracked), by the same times table and `scripts/test-impact.json` as the check; `--why` adds a tab and the reason per file, `--null` ends each path with NUL (`| xargs -0 -r bun test`), `--json` prints `{full, files:[{file, why}]}`. Exit 0 lists files; exit 3 means the change reaches no test (stdout empty); exit 2 means the diff is one the map cannot bound (`package.json`, `forge.json`, a gate change that is logic…; stderr says `full tier: … changed`) or the arguments or git failed. The `&&` guard matters: `bun test $(…)` alone ignores the exit code and, with nothing listed, runs the whole suite. It reads the diff with `git` called by argument list, never a shell; `scripts/tier-input.ts` is the reading it shares with `test-shards.ts`. |
| 187 | 187 |
scripts/flake.ts +1 −1
| @@ -308,7 +308,7 @@ export function rawBudgets(name: string, source: string): string[] { | ||
| 308 | 308 | ) |
| 309 | 309 | raw = true; |
| 310 | 310 | if (raw && !/expectBudget/.test(line)) |
| 311 | found.push(`${name}:${i + 1}: ${line.trim()} — bound speed with expectBudget() from test/helpers/budget.ts`); | |
| 311 | found.push(`${name}:${i + 1}: ${line.trim()} — bound speed with expectBudget() from test/helpers/budget.ts (#4429)`); | |
| 312 | 312 | }); |
| 313 | 313 | return found; |
| 314 | 314 | } |
src/checks/tiers.ts +25 −2
| @@ -19,6 +19,26 @@ export const QUICK_SECONDS = 1; | ||
| 19 | 19 | /** The fast tier's budget, in seconds; a plan over it is said, not hidden. */ |
| 20 | 20 | export const FAST_BUDGET_SECONDS = 5 * 60; |
| 21 | 21 | |
| 22 | /** | |
| 23 | * The repo-wide rule tests (#7138): each reads every test file, every | |
| 24 | * migration or the whole README, so no diff "touches" them by coverage, | |
| 25 | * import or name, yet any file can break them. They run in every fast | |
| 26 | * tier, whatever else the diff holds (a diff of only the check's own | |
| 27 | * timing and impact tables breaks no rule), so a rule breaker is red on its own pull and | |
| 28 | * not in a merge batch. Each is a second or two. | |
| 29 | */ | |
| 30 | export const RULE_TESTS: readonly string[] = [ | |
| 31 | // No raw wall-clock budget (#4429), no tight wall-clock bound, no retry. | |
| 32 | "test/flake.test.ts", | |
| 33 | // Every migration is pinned by its own pull, and no pinned one is edited. | |
| 34 | "test/schema-migrations.test.ts", | |
| 35 | // The README budget: its size, its line cap, and its index into docs/. | |
| 36 | "test/readme-front-door.test.ts", | |
| 37 | "test/readme-cap.test.ts", | |
| 38 | // The page audit: docs/design/audit.md names every top-level page. | |
| 39 | "test/design-audit.test.ts", | |
| 40 | ]; | |
| 41 | ||
| 22 | 42 | /** What a test the times table does not name is assumed to take. */ |
| 23 | 43 | const UNKNOWN_SECONDS = 5; |
| 24 | 44 | |
| @@ -99,8 +119,8 @@ export interface FastPlan { | ||
| 99 | 119 | /** |
| 100 | 120 | * The fast tier for a diff: a changed test; a test importing a changed file |
| 101 | 121 | * directly or through up to two hops of source; a test whose name matches a |
| 102 | * changed source path; whatever the pull learned; and every test known to | |
| 103 | * take under a second. A test with no recorded time is not known to be quick, so it runs only when | |
| 122 | * changed source path; every repo-wide rule test; whatever the pull | |
| 123 | * learned; and every test known to take under a second. A test with no recorded time is not known to be quick, so it runs only when | |
| 104 | 124 | * something above picks it. |
| 105 | 125 | */ |
| 106 | 126 | export function fastSet(input: FastInput): FastPlan { |
| @@ -147,6 +167,9 @@ export function fastSet(input: FastInput): FastPlan { | ||
| 147 | 167 | }; |
| 148 | 168 | const changed = new Set(diffed); |
| 149 | 169 | for (const file of changed) take(file, "changed"); |
| 170 | // The check's own tables can break no rule: a diff of only those runs none (#3579). | |
| 171 | if (diffed.some((f) => !f.endsWith("test-times.json") && !f.endsWith("test-impact.json"))) | |
| 172 | for (const test of RULE_TESTS) take(test, "repo-wide rule"); | |
Matching the table names by endsWith would also match a file named like test-times.json in another directory. Match the exact scripts/ paths. |
||
| 150 | 173 | // What a guess looks for. A test the map has measured already answered |
| 151 | 174 | // for every change the map can see, by what it executes; it is guessed |
| 152 | 175 | // only against the changes the map cannot see, so one unreached file |
test/flake.test.ts +13 −14
| @@ -33,9 +33,14 @@ import { | ||
| 33 | 33 | rawBudgets, |
| 34 | 34 | WALL_CLOCK_FLOOR_MS, |
| 35 | 35 | } from "../scripts/flake.ts"; |
| 36 | import { breaches } from "./helpers/rule.ts"; | |
| 36 | 37 | |
| 37 | 38 | const root = new URL("..", import.meta.url).pathname; |
| 38 | 39 | const read = (path: string): string => readFileSync(`${root}${path}`, "utf8"); |
| 40 | // Every test file the wall-clock rules read; this file holds the detectors' own examples. | |
| 41 | const scanned = readdirSync(`${root}test`) | |
| 42 | .filter((name) => name.endsWith(".test.ts") && name !== "flake.test.ts") | |
| 43 | .map((name) => `test/${name}`); | |
| 39 | 44 | |
| 40 | 45 | const dirs: string[] = []; |
| 41 | 46 | const scratch = async (): Promise<string> => { |
| @@ -341,14 +346,10 @@ describe("no tight wall-clock bound in a test", () => { | ||
| 341 | 346 | ); |
| 342 | 347 | }); |
| 343 | 348 | |
| 344 | test("no test file bounds a request or a spawn below the floor", () => { | |
| 345 | const files = readdirSync(`${root}test`) | |
| 346 | .filter((name) => name.endsWith(".test.ts")) | |
| 347 | // This file holds the detector's own examples. | |
| 348 | .filter((name) => name !== "flake.test.ts") | |
| 349 | .map((name) => `test/${name}`); | |
| 350 | expect(files.flatMap((path) => tightBounds(path, read(path)))).toEqual([]); | |
| 351 | }, 120_000); // reads every test file: past the 5 s default on a loaded machine | |
| 349 | breaches( | |
| 350 | "no test file bounds a request or a spawn below the floor", | |
| 351 | scanned.flatMap((path) => tightBounds(path, read(path))), | |
| 352 | ); | |
| 352 | 353 | }); |
| 353 | 354 | |
| 354 | 355 | describe("no raw wall-clock budget in a test (#4429)", () => { |
| @@ -362,12 +363,10 @@ describe("no raw wall-clock budget in a test (#4429)", () => { | ||
| 362 | 363 | expect(rawBudgets("x", "expect(lines.length).toBeLessThan(20);")).toEqual([]); |
| 363 | 364 | }); |
| 364 | 365 | |
| 365 | test("no test file holds a speed to a raw ms number", () => { | |
| 366 | const files = readdirSync(`${root}test`) | |
| 367 | .filter((name) => name.endsWith(".test.ts") && name !== "flake.test.ts") | |
| 368 | .map((name) => `test/${name}`); | |
| 369 | expect(files.flatMap((path) => rawBudgets(path, read(path)))).toEqual([]); | |
| 370 | }, 120_000); | |
| 366 | breaches( | |
| 367 | "no test file holds a speed to a raw ms number", | |
| 368 | scanned.flatMap((path) => rawBudgets(path, read(path))), | |
| 369 | ); | |
| 371 | 370 | }); |
| 372 | 371 | |
| 373 | 372 | describe("the scheduled run of the files outside the gate", () => { |
test/helpers/rule.ts added +20
| @@ -0,0 +1,20 @@ | ||
| 1 | /** | |
| 2 | * A repo-wide rule as tests (#7138): one red test per breach, named for | |
| 3 | * the breach itself (`test/x.test.ts:5: expect(…)…`), so a red check's | |
| 4 | * summary — which quotes the failing tests' names — says the rule (the | |
| 5 | * describe around it) and the file and line, and the builder fixes it in | |
| 6 | * one run. A tree with no breach is the one green test named `clean`. | |
| 7 | */ | |
| 8 | ||
| 9 | import { test } from "bun:test"; | |
| 10 | ||
| 11 | export function breaches(clean: string, found: readonly string[]): void { | |
| 12 | if (found.length === 0) { | |
| 13 | test(clean, () => {}); | |
| 14 | return; | |
| 15 | } | |
| 16 | for (const breach of found) | |
| 17 | test(breach, () => { | |
| 18 | throw new Error(`${clean}: ${breach}`); | |
| 19 | }); | |
| 20 | } |
test/rule-tests-always.test.ts added +94
| @@ -0,0 +1,94 @@ | ||
| 1 | /** | |
| 2 | * The repo-wide rule tests run in every pull's fast tier (#7138): a pull | |
| 3 | * that only adds a test file holding a raw ms budget is red on its own | |
| 4 | * check, and the check's summary names the rule and the line. | |
| 5 | */ | |
| 6 | ||
| 7 | import { afterAll, describe, expect, test } from "bun:test"; | |
| 8 | import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; | |
| 9 | import { tmpdir } from "node:os"; | |
| 10 | import { join } from "node:path"; | |
| 11 | import { gateFiles } from "../scripts/gate.ts"; | |
| 12 | import { rawBudgets } from "../scripts/flake.ts"; | |
| 13 | import { fastSet, RULE_TESTS } from "../src/checks/tiers.ts"; | |
| 14 | import { summariseCheckFailure } from "../src/checks/summary.ts"; | |
| 15 | ||
| 16 | const breaker = [ | |
| 17 | 'import { expect, test } from "bun:test";', | |
| 18 | 'test("x", () => {', | |
| 19 | " const t0 = performance.now();", | |
| 20 | // Split here so the rule, reading this file, does not find it. | |
| 21 | " expect(performance.now() - t0)" + | |
| 22 | ".toBeLessThan(5);", | |
| 23 | "});", | |
| 24 | ].join("\n"); | |
| 25 | ||
| 26 | const scratch = mkdtempSync(join(tmpdir(), "forge-rule-")); | |
| 27 | afterAll(() => rmSync(scratch, { recursive: true, force: true })); | |
| 28 | ||
| 29 | describe("the repo-wide rule tests in the fast tier", () => { | |
| 30 | const gate = gateFiles(); | |
| 31 | // A measured map that knows every test but the new one, as on a pull. | |
| 32 | const impact = new Map(gate.map((t) => [t, ["src/elsewhere.ts"]] as const)); | |
| 33 | const plan = (changed: string[]) => | |
| 34 | fastSet({ | |
| 35 | changed, | |
| 36 | tests: [...gate, "test/x.test.ts"], | |
| 37 | imports: () => [], | |
| 38 | times: new Map(), | |
| 39 | impact, | |
| 40 | }); | |
| 41 | ||
| 42 | test("every rule test is a gate file, so none is listed and never run", () => { | |
| 43 | for (const rule of RULE_TESTS) expect(gate).toContain(rule); | |
| 44 | }); | |
| 45 | ||
| 46 | test("a pull adding only test/x.test.ts runs every rule test", () => { | |
| 47 | const fast = plan(["test/x.test.ts"]); | |
| 48 | for (const rule of RULE_TESTS) { | |
| 49 | expect(fast.files).toContain(rule); | |
| 50 | expect(fast.why.get(rule)).toBe("repo-wide rule"); | |
| 51 | } | |
| 52 | expect(fast.files).toContain("test/x.test.ts"); | |
| 53 | }); | |
| 54 | ||
| 55 | test("a doc-only pull runs them too: the README budget is one", () => { | |
| 56 | expect(plan(["README.md"]).files).toEqual(expect.arrayContaining([...RULE_TESTS])); | |
| 57 | }); | |
| 58 | ||
| 59 | test("a diff of only the check's own tables runs none: it can break no rule (#3579)", () => { | |
| 60 | expect(plan(["scripts/test-times.json"]).files).toEqual([]); | |
| 61 | }); | |
| 62 | ||
| 63 | test("its raw ms budget is a red check whose summary names #4429 and the line", async () => { | |
| 64 | const found = rawBudgets("test/x.test.ts", breaker); | |
| 65 | expect(found).toHaveLength(1); | |
| 66 | // The rule as test/flake.test.ts states it, run by a real bun test. | |
| 67 | const file = join(scratch, "rule.test.ts"); | |
| 68 | writeFileSync( | |
| 69 | file, | |
| 70 | [ | |
| 71 | 'import { describe } from "bun:test";', | |
| 72 | `import { breaches } from ${JSON.stringify(join(import.meta.dir, "helpers/rule.ts"))};`, | |
| 73 | 'describe("no raw wall-clock budget in a test (#4429)", () => {', | |
| 74 | ` breaches("no test file holds a speed to a raw ms number", ${JSON.stringify(found)});`, | |
| 75 | "});", | |
| 76 | ].join("\n"), | |
| 77 | ); | |
| 78 | const run = Bun.spawn([process.execPath, "test", file], { | |
| 79 | cwd: scratch, | |
| 80 | stdout: "pipe", | |
| 81 | stderr: "pipe", | |
| 82 | env: { ...process.env, CI: "1" }, | |
| 83 | }); | |
| 84 | const [out, err, code] = await Promise.all([ | |
| 85 | new Response(run.stdout).text(), | |
| 86 | new Response(run.stderr).text(), | |
| 87 | run.exited, | |
| 88 | ]); | |
| 89 | expect(code).not.toBe(0); | |
| 90 | const { summary } = summariseCheckFailure(`${out}\n${err}`.split("\n"), "check exited with code 1"); | |
| 91 | expect(summary).toContain("#4429"); | |
| 92 | expect(summary).toContain("test/x.test.ts:4:"); | |
| 93 | }, 30_000); | |
| 94 | }); |
test/schema-migrations.test.ts +7 −4
| @@ -35,6 +35,7 @@ import { | ||
| 35 | 35 | } from "../src/kernel/migrate.ts"; |
| 36 | 36 | import { migrationFiles, migrations, NUMBERED } from "../src/core/schema.ts"; |
| 37 | 37 | import { failureReason } from "../src/apps/process.ts"; |
| 38 | import { breaches } from "./helpers/rule.ts"; | |
| 38 | 39 | import legacyPins from "../src/core/migration-pins.json" with { type: "json" }; |
| 39 | 40 | |
| 40 | 41 | const pins = readPins(join(import.meta.dir, "../src/core/migration-pins"), legacyPins); |
| @@ -427,10 +428,12 @@ describe("a migration on main is never edited (#783)", () => { | ||
| 427 | 428 | expect(pinBreaks(migrationFiles, pins)).toEqual([]); |
| 428 | 429 | }); |
| 429 | 430 | |
| 430 | test("every migration is pinned by its own pull: run bun scripts/pin-migrations.ts", () => { | |
| 431 | const missing = migrationFiles.filter((m) => !Object.hasOwn(pins, m.name)).map((m) => m.name); | |
| 432 | expect(missing).toEqual([]); | |
| 433 | }); | |
| 431 | breaches( | |
| 432 | "every migration is pinned by its own pull: run bun scripts/pin-migrations.ts", | |
| 433 | migrationFiles | |
| 434 | .filter((m) => !Object.hasOwn(pins, m.name)) | |
| 435 | .map((m) => `src/core/migrations/${m.name}.ts is not pinned: run bun scripts/pin-migrations.ts`), | |
| 436 | ); | |
| 434 | 437 | |
| 435 | 438 | test("pin files are read beside the frozen list, and a bad one is a defect", () => { |
| 436 | 439 | const dir = join(root, "pins"); |
new above
offline
Reflow this comment to the surrounding width. It runs long mid-sentence and reads as patched.