Public: anyone can read this; the crew can change it.
Waves lint: read the forge records offline instead of git history plus a hand list #11398
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11388-waves-lint-read-the-forge-records-offlin → main 3 commits fork 3 threads · 3 unresolved
5 files
changed +108
−20
scripts/export-records.ts added +28
| @@ -0,0 +1,28 @@ | ||
| 1 | /** | |
| 2 | * Writes test/fixtures/forge-records.txt: every issue and pull number on the | |
| 3 | * forge, as ranges ("1-40 42 45-50"), so tests can check a `#N` offline. | |
| 4 | * Run: FORGE_URL=… FORGE_TOKEN=… bun scripts/export-records.ts | |
| 5 | */ | |
| 6 | import { join } from "node:path"; | |
| 7 | import { compactRecords } from "../src/story/records.ts"; | |
| 8 | ||
| 9 | const { FORGE_URL, FORGE_TOKEN } = process.env; | |
| 10 | if (!FORGE_URL || !FORGE_TOKEN) { | |
| 11 | console.error("export-records: set FORGE_URL and FORGE_TOKEN"); | |
| 12 | process.exit(1); | |
| 13 | } | |
| 14 | const base = `${FORGE_URL}/api/v1/repos/trevato/forge`; | |
| 15 | const nums = new Set<number>(); | |
| 16 | for (const kind of ["issues", "pulls"]) { | |
| 17 | let cursor: number | null = null; | |
| 18 | do { | |
| 19 | const q = `state=all&limit=200${cursor ? `&cursor=${cursor}` : ""}`; | |
| 20 | const res = await fetch(`${base}/${kind}?${q}`, { headers: { authorization: `Bearer ${FORGE_TOKEN}` } }); | |
| 21 | if (!res.ok) throw new Error(`${kind}: ${res.status}`); | |
| 22 | const body = (await res.json()) as { items: { number: number }[]; next_cursor?: number | null }; | |
outdated
Move this compaction into a pure, exported function and test it. Nothing covers it now. |
||
| 23 | for (const i of body.items) nums.add(i.number); | |
| 24 | cursor = body.next_cursor ?? null; | |
| 25 | } while (cursor); | |
| 26 | } | |
| 27 | await Bun.write(join(import.meta.dir, "../test/fixtures/forge-records.txt"), compactRecords(nums) + "\n"); | |
| 28 | console.log(`${nums.size} numbers`); |
src/story/records.ts added +37
| @@ -0,0 +1,37 @@ | ||
| 1 | /** | |
| 2 | * The forge's issue and pull numbers as a compact text: ranges and singletons | |
| 3 | * ("1-40 42 45-50"). One definition of the format, for the export script and | |
| 4 | * the tests that read it. Plain data in, plain data out. | |
| 5 | */ | |
| 6 | ||
| 7 | /** Sorted, de-duplicated numbers as ranges; singletons stay bare. */ | |
| 8 | export function compactRecords(numbers: Iterable<number>): string { | |
| 9 | const sorted = [...new Set(numbers)].sort((a, b) => a - b); | |
| 10 | const parts: string[] = []; | |
| 11 | for (let i = 0; i < sorted.length; ) { | |
| 12 | let j = i; | |
| 13 | while (j + 1 < sorted.length && sorted[j + 1]! === sorted[j]! + 1) j++; | |
| 14 | parts.push(j > i ? `${sorted[i]}-${sorted[j]}` : `${sorted[i]}`); | |
| 15 | i = j + 1; | |
| 16 | } | |
| 17 | return parts.join(" "); | |
| 18 | } | |
| 19 | ||
| 20 | /** The numbers a compact text names. Throws on anything that is not a range or number. */ | |
| 21 | export function parseRecords(text: string): Set<number> { | |
| 22 | const out = new Set<number>(); | |
| 23 | for (const part of text.split(/\s+/).filter(Boolean)) { | |
| 24 | const m = /^(\d+)(?:-(\d+))?$/.exec(part); | |
| 25 | if (!m) throw new Error(`forge records: bad range "${part}"`); | |
| 26 | const lo = Number(m[1]); | |
| 27 | const hi = Number(m[2] ?? m[1]); | |
| 28 | if (hi < lo) throw new Error(`forge records: backwards range "${part}"`); | |
| 29 | for (let n = lo; n <= hi; n++) out.add(n); | |
| 30 | } | |
| 31 | return out; | |
| 32 | } | |
| 33 | ||
| 34 | /** Whether `n` is known: listed, or above the export's highest, which offline cannot be disproved. */ | |
| 35 | export function isKnownRecord(known: ReadonlySet<number>, max: number, n: number): boolean { | |
| 36 | return n > max || known.has(n); | |
| 37 | } |
test/fixtures/forge-records.txt added +1
| @@ -0,0 +1,1 @@ | ||
| 1 | 1-11397 |
test/story-records.test.ts added +23
| @@ -0,0 +1,23 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { compactRecords, isKnownRecord, parseRecords } from "../src/story/records.ts"; | |
| 3 | ||
| 4 | test("compactRecords joins runs, keeps singletons, and ignores order and repeats", () => { | |
| 5 | expect(compactRecords([5, 1, 2, 3, 3, 7, 8])).toBe("1-3 5 7-8"); | |
| 6 | expect(compactRecords([4])).toBe("4"); | |
| 7 | expect(compactRecords([])).toBe(""); | |
| 8 | }); | |
| 9 | ||
| 10 | test("parseRecords round-trips compactRecords and refuses bad text", () => { | |
| 11 | const nums = [1, 2, 3, 9, 20, 21]; | |
| 12 | expect([...parseRecords(compactRecords(nums))]).toEqual(nums); | |
| 13 | expect(parseRecords("").size).toBe(0); | |
| 14 | expect(() => parseRecords("1-x")).toThrow("bad range"); | |
| 15 | expect(() => parseRecords("5-2")).toThrow("backwards"); | |
| 16 | }); | |
| 17 | ||
| 18 | test("isKnownRecord passes numbers above the maximum only", () => { | |
| 19 | const k = new Set([1, 3]); | |
| 20 | expect(isKnownRecord(k, 3, 2)).toBe(false); | |
| 21 | expect(isKnownRecord(k, 3, 3)).toBe(true); | |
| 22 | expect(isKnownRecord(k, 3, 4)).toBe(true); | |
| 23 | }); |
test/waves-lint.test.ts +19 −20
| @@ -1,32 +1,23 @@ | ||
| 1 | 1 | import { expect, test } from "bun:test"; |
| 2 | 2 | import { readdirSync, readFileSync } from "node:fs"; |
| 3 | 3 | import { join } from "node:path"; |
| 4 | import { isKnownRecord, parseRecords } from "../src/story/records.ts"; | |
| 4 | 5 | import { parseWave } from "../src/story/wave.ts"; |
| 5 | 6 | |
| 6 | 7 | const root = join(import.meta.dir, ".."); |
| 7 | 8 | const dir = join(root, "waves"); |
| 8 | 9 | |
| 9 | 10 | /** |
| 10 | * The records a test can open offline: every `#N` in the git history's | |
| 11 | * subjects and bodies (pulls land as "(#N)" or "Merge pull request #N"). Issues | |
| 12 | * and pulls that never left a commit are not there, so these two real pulls, | |
| 13 | * found on the forge, are named by hand until the forge's records are readable | |
| 14 | * offline (see the follow-up issue). | |
| 11 | * The records a test can open offline: every issue and pull number on the | |
| 12 | * forge, exported to test/fixtures/forge-records.txt (`bun scripts/export-records.ts` | |
| 13 | * refreshes it). A number above the export's highest cannot be disproved | |
| 14 | * offline, so it passes; only gaps below the highest are checkable. | |
| 15 | 15 | */ |
outdated
Keep parseRecords in a small module shared with the script, so the format has one definition and a round-trip test. |
||
| 16 | const NOT_IN_HISTORY = [6232, 6964]; | |
| 17 | function recordedNumbers(): Set<number> | null { | |
| 18 | const git = (args: string[]) => Bun.spawnSync(["git", ...args], { cwd: root, stdout: "pipe", stderr: "ignore" }); | |
| 19 | if (git(["rev-parse", "--is-shallow-repository"]).stdout.toString().trim() !== "false") return null; | |
| 20 | const log = git(["log", "--format=%s%n%b"]); | |
| 21 | if (log.exitCode !== 0) return null; | |
| 22 | const out = new Set<number>(NOT_IN_HISTORY); | |
| 23 | for (const m of log.stdout.toString().matchAll(/#(\d+)/g)) out.add(Number(m[1])); | |
| 24 | return out; | |
| 25 | } | |
| 26 | const records = recordedNumbers(); | |
| 16 | const records = parseRecords(readFileSync(join(root, "test/fixtures/forge-records.txt"), "utf8")); | |
| 17 | const recordsMax = Math.max(...records); | |
| 27 | 18 | |
| 28 | 19 | /** Every problem with one wave's text, each naming `file`. Pure. */ |
| 29 | function lintWave(file: string, text: string, known: ReadonlySet<number>): string[] { | |
| 20 | function lintWave(file: string, text: string, known: ReadonlySet<number>, max = Math.max(...known)): string[] { | |
| 30 | 21 | const parsed = parseWave(file, text); |
| 31 | 22 | if (!parsed.ok) return [parsed.error.message]; |
| 32 | 23 | const w = parsed.value; |
| @@ -43,7 +34,7 @@ function lintWave(file: string, text: string, known: ReadonlySet<number>): strin | ||
| 43 | 34 | else if (title !== url) bad(`link text #${title ?? "?"} does not match ${p.url}`); |
| 44 | 35 | else { |
| 45 | 36 | listed.add(Number(url)); |
| 46 | if (!known.has(Number(url))) bad(`#${url} is in no record of this repository`); | |
| 37 | if (!isKnownRecord(known, max, Number(url))) bad(`#${url} is in no record of this repository`); | |
| 47 | 38 | } |
| 48 | 39 | } |
| 49 | 40 | for (const h of w.highlights) |
| @@ -52,10 +43,10 @@ function lintWave(file: string, text: string, known: ReadonlySet<number>): strin | ||
| 52 | 43 | return out; |
| 53 | 44 | } |
| 54 | 45 | |
| 55 | test.skipIf(records === null)("every wave in waves/ is a title, a milestone, a summary and links that resolve", () => { | |
| 46 | test("every wave in waves/ is a title, a milestone, a summary and links that resolve", () => { | |
| 56 | 47 | const files = readdirSync(dir).filter((f) => f.endsWith(".md")); |
| 57 | 48 | expect(files.length).toBeGreaterThan(0); |
| 58 | const problems = files.flatMap((f) => lintWave(f, readFileSync(join(dir, f), "utf8"), records!)); | |
| 49 | const problems = files.flatMap((f) => lintWave(f, readFileSync(join(dir, f), "utf8"), records)); | |
| 59 | 50 | expect(problems).toEqual([]); |
| 60 | 51 | }); |
| 61 | 52 | |
| @@ -87,3 +78,11 @@ test("lintWave names the file for each fault", () => { | ||
| 87 | 78 | expect(lintWave("1.0.0.md", good.replace("/pulls/1", "/pulls/2"), K)[0]).toContain("does not match"); |
| 88 | 79 | expect(lintWave("1.0.0.md", good.replace("/pulls/1", "/x/1"), K)[0]).toContain("not an issue or pull"); |
| 89 | 80 | }); |
| 81 | ||
| 82 | test("a number above the export passes, a gap below it does not", () => { | |
| 83 | const withGap = new Set([1, 3]); | |
| 84 | expect(lintWave("1.0.0.md", good.replaceAll("#1", "#2").replace("/pulls/1", "/pulls/2"), withGap)[0]).toContain("in no record"); | |
| 85 | expect(lintWave("1.0.0.md", good.replaceAll("#1", "#9").replace("/pulls/1", "/pulls/9"), withGap)).toEqual([]); | |
| 86 | expect(records.has(6232) && records.has(6964)).toBe(true); | |
| 87 | expect(recordsMax).toBeGreaterThan(11000); | |
| 88 | }); |
new above
offline
Check FORGE_URL and FORGE_TOKEN here and exit with a message. Unset, this builds "undefined/api/v1/...".