Public: anyone can read this; the crew can change it.
A full-suite report names a failing test under the file that ran it, never under the last file printed #9526
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9523-a-full-suite-report-names-a-failing-test → main 3 commits fork
6 files
changed +112
−5
scripts/shards.ts +2 −1
| @@ -7,7 +7,7 @@ | ||
| 7 | 7 | * is disjoint and covering, and that one red shard makes the run red. |
| 8 | 8 | */ |
| 9 | 9 | |
| 10 | import { nameUnnamed } from "../src/checks/bun-test.ts"; | |
| 10 | import { isSummaryLine, nameUnnamed } from "../src/checks/bun-test.ts"; | |
| 11 | 11 | |
| 12 | 12 | /** |
| 13 | 13 | * No more than this many `bun test` processes at once, whatever the host: |
| @@ -564,6 +564,7 @@ export function failingFiles(output: string): string[] { | ||
| 564 | 564 | for (const line of output.split("\n")) { |
| 565 | 565 | const header = HEADER.exec(line); |
| 566 | 566 | if (header) current = header[1]!; |
| 567 | else if (isSummaryLine(line)) current = undefined; | |
| 567 | 568 | else if (FAILED.test(line) && current !== undefined && !found.includes(current)) |
| 568 | 569 | found.push(current); |
| 569 | 570 | } |
src/checks/bun-test.ts +13 −2
| @@ -22,6 +22,19 @@ const PASS_COUNT = /^\s*(\d+) pass$/; | ||
| 22 | 22 | const FAIL_COUNT = /^\s*(\d+) fail$/; |
| 23 | 23 | |
| 24 | 24 | const HEADER = /^(\S+\.test\.[cm]?[jt]sx?):$/; |
| 25 | ||
| 26 | /** | |
| 27 | * Bun's closing summary (#9523): `N tests skipped:` and `N tests failed:` | |
| 28 | * each repeat their lines after the last file, then the counts and `Ran N | |
| 29 | * tests across M files.`. A reader that gives each `(fail)` line to the last | |
| 30 | * `file.test.ts:` header it saw stops at the first of these, or every | |
| 31 | * repeated failure lands on the last file printed; the next header (another | |
| 32 | * shard's report) starts a file again. | |
| 33 | */ | |
| 34 | const SUMMARY = /^(?:\d+ tests? (?:failed|skipped|todo):|\s*\d+ (?:pass|fail|skip|todo)|Ran \d+ tests? across .*)\s*$/; | |
| 35 | ||
| 36 | /** Whether a log line opens or belongs to bun's closing summary, outside any file's block. */ | |
| 37 | export const isSummaryLine = (line: string): boolean => SUMMARY.test(line); | |
| 25 | 38 | const UNNAMED_FAIL = /^\(fail\) ((?:.* > )?)\(unnamed\)/; |
| 26 | 39 | /** bun's own caret line under a hook that timed out, printed right after its `(fail)`. */ |
| 27 | 40 | const HOOK_TIMEOUT = /^\s*\^ a (\S+) hook timed out/; |
| @@ -29,8 +42,6 @@ const HOOK_TIMEOUT = /^\s*\^ a (\S+) hook timed out/; | ||
| 29 | 42 | const THROWN = /^(?:[A-Za-z]*Error|error): .+/; |
| 30 | 43 | const RESULT_LINE = /^\((?:fail|pass)\) /; |
| 31 | 44 | const MAX_THROWN = 120; |
| 32 | /** The tail that ends a file's report; bun's recap of `(fail)` lines can follow, headerless. */ | |
| 33 | const SUMMARY = /^\s*\d+ pass$|^\d+ tests? failed:$/; | |
| 34 | 45 | |
| 35 | 46 | /** |
| 36 | 47 | * The first error bun printed since the previous result line, printable only, |
src/checks/flake.ts +11 −1
| @@ -16,6 +16,7 @@ import type { EventBus } from "../core/events.ts"; | ||
| 16 | 16 | import { id } from "../kernel/id.ts"; |
| 17 | 17 | import type { CheckDetail } from "../core/runs.ts"; |
| 18 | 18 | import { importsOf } from "./tiers.ts"; |
| 19 | import { isSummaryLine } from "./bun-test.ts"; | |
| 19 | 20 | |
| 20 | 21 | /** The most failures that are re-run; more is a broken main, not a flake. */ |
| 21 | 22 | export const MAX_RERUN = 2; |
| @@ -282,7 +283,12 @@ export function fileIssueOnce( | ||
| 282 | 283 | return { number: made.value.number, opened: true }; |
| 283 | 284 | } |
| 284 | 285 | |
| 285 | /** Every failing test of a `bun test` log with the file whose report printed it, once each, in order. */ | |
| 286 | /** | |
| 287 | * Every failing test of a `bun test` log with the file whose report printed | |
| 288 | * it, once each, in order. Bun's closing summary repeats the failures after | |
| 289 | * the last file; from its first line until the next header a `(fail)` line | |
| 290 | * has no file, and is dropped rather than given to the last one printed. | |
| 291 | */ | |
| 286 | 292 | export function failedTests(lines: readonly string[]): Flaky[] { |
| 287 | 293 | const out: Flaky[] = []; |
| 288 | 294 | const seen = new Set<string>(); |
| @@ -293,6 +299,10 @@ export function failedTests(lines: readonly string[]): Flaky[] { | ||
| 293 | 299 | file = header[1]!.replace(/^\.\//, ""); |
| 294 | 300 | continue; |
| 295 | 301 | } |
| 302 | if (isSummaryLine(line)) { | |
| 303 | file = undefined; | |
| 304 | continue; | |
| 305 | } | |
| 296 | 306 | const name = FAIL.exec(line)?.[1]; |
| 297 | 307 | if (name === undefined || file === undefined || !PATH.test(file)) continue; |
| 298 | 308 | if (name.length > NAME_MAX || CONTROL.test(name) || seen.has(`${file}\0${name}`)) continue; |
src/checks/verify.ts +3
| @@ -15,6 +15,8 @@ | ||
| 15 | 15 | * next, which files a log says failed. The service runs them. |
| 16 | 16 | */ |
| 17 | 17 | |
| 18 | import { isSummaryLine } from "./bun-test.ts"; | |
| 19 | ||
| 18 | 20 | /** The environment a scheduled verification of the default branch runs in. */ |
| 19 | 21 | export const VERIFY_ENV = "verify"; |
| 20 | 22 | /** The environment a bisection step runs in. */ |
| @@ -114,6 +116,7 @@ export function failedFiles(lines: readonly string[]): string[] { | ||
| 114 | 116 | for (const line of lines) { |
| 115 | 117 | const header = HEADER.exec(line); |
| 116 | 118 | if (header) current = header[1]!.replace(/^\.\//, ""); |
| 119 | else if (isSummaryLine(line)) current = undefined; | |
| 117 | 120 | else if (FAILED.test(line) && current !== undefined && safeFile(current) && !flaky.has(current) && !found.includes(current)) { |
| 118 | 121 | found.push(current); |
| 119 | 122 | if (found.length >= MAX_FILES) break; |
test/full-suite-report.test.ts +68 −1
| @@ -14,13 +14,66 @@ import { flowLines } from "../src/tui/screens/flow.ts"; | ||
| 14 | 14 | import { MAX_FAILURES, redDetail, reportRed, reportTitle, REPORT_LABEL } from "../src/checks/report.ts"; |
| 15 | 15 | import { failedTests } from "../src/checks/flake.ts"; |
| 16 | 16 | import { heldByRed, redMain } from "../src/checks/red_main.ts"; |
| 17 | import { VERIFY_ENV } from "../src/checks/verify.ts"; | |
| 17 | import { failedFiles, VERIFY_ENV } from "../src/checks/verify.ts"; | |
| 18 | 18 | import { listIssues, listComments, labelsFor } from "../src/core/issues.ts"; |
| 19 | 19 | import type { Repo } from "../src/core/repos.ts"; |
| 20 | 20 | import { createRun, getRun, type Run } from "../src/core/runs.ts"; |
| 21 | 21 | import { createUser } from "../src/core/users.ts"; |
| 22 | 22 | |
| 23 | /** | |
| 24 | * What `bun test` 1.3 prints for three files, two with a failure, the green | |
| 25 | * one printed last, then its closing summary repeating every failure (#9523). | |
| 26 | */ | |
| 27 | const BUN_LOG = [ | |
| 28 | "bun test v1.3.14 (0d9b296a)", | |
| 29 | "", | |
| 30 | "test/a.test.ts:", | |
| 31 | "(pass) a > ok [0.20ms]", | |
| 32 | "1 | test(\"one\", () => expect(1).toBe(2));", | |
| 33 | "error: expect(received).toBe(expected)", | |
| 34 | "(fail) a > one [0.11ms]", | |
| 35 | "", | |
| 36 | "test/b.test.ts:", | |
| 37 | "(fail) b > two [0.06ms]", | |
| 38 | "(skip) b > later", | |
| 39 | "", | |
| 40 | "test/c.test.ts:", | |
| 41 | "(pass) c > fine [0.05ms]", | |
| 42 | "", | |
| 43 | "1 tests skipped:", | |
| 44 | "(skip) b > later", | |
| 45 | "", | |
| 46 | "", | |
| 47 | "2 tests failed:", | |
| 48 | "(fail) a > one [0.11ms]", | |
| 49 | "(fail) b > two [0.06ms]", | |
| 50 | "", | |
| 51 | " 2 pass", | |
| 52 | " 1 skip", | |
| 53 | " 2 fail", | |
| 54 | " 2 expect() calls", | |
| 55 | "Ran 5 tests across 3 files. [38.00ms]", | |
| 56 | ]; | |
| 57 | ||
| 23 | 58 | describe("failedTests", () => { |
| 59 | test("bun's closing summary gives no failure to the last file printed", () => { | |
| 60 | expect(failedTests(BUN_LOG)).toEqual([ | |
| 61 | { file: "test/a.test.ts", name: "a > one" }, | |
| 62 | { file: "test/b.test.ts", name: "b > two" }, | |
| 63 | ]); | |
| 64 | expect(failedFiles(BUN_LOG)).toEqual(["test/a.test.ts", "test/b.test.ts"]); | |
| 65 | }); | |
| 66 | ||
| 67 | test("a failure after the summary with no header of its own is dropped", () => { | |
| 68 | expect(failedTests(["test/a.test.ts:", "(pass) a", " 1 pass", "Ran 1 tests across 1 files.", "(fail) stray"])).toEqual([]); | |
| 69 | }); | |
| 70 | ||
| 71 | test("a second report in the same log starts its files again", () => { | |
| 72 | const shards = [...BUN_LOG, "test/d.test.ts:", "(fail) d > three", "1 tests failed:", "(fail) d > three"]; | |
| 73 | expect(failedTests(shards).map((t) => t.file)).toEqual(["test/a.test.ts", "test/b.test.ts", "test/d.test.ts"]); | |
| 74 | }); | |
| 75 | ||
| 76 | ||
| 24 | 77 | test("pairs each failure with the file that printed it, once", () => { |
| 25 | 78 | const lines = [ |
| 26 | 79 | "test/a.test.ts:", |
| @@ -141,6 +194,20 @@ describe("the report", () => { | ||
| 141 | 194 | expect(redDetail(undefined, [], lines)).toBeUndefined(); |
| 142 | 195 | }); |
| 143 | 196 | |
| 197 | test("a file whose own tests all passed is filed for nothing, though bun printed it last", () => { | |
| 198 | const run = createRun(app.db, { repoId: repo.id, kind: "check", env: VERIFY_ENV, commitOid: "d".repeat(40) }); | |
| 199 | const detail = redDetail(undefined, failedFiles(BUN_LOG), BUN_LOG)!; | |
| 200 | app.db | |
| 201 | .query("UPDATE run SET state = 'failed', check_detail = ?2 WHERE id = ?1") | |
| 202 | .run(run.id, JSON.stringify(detail)); | |
| 203 | reportRed(app, repo, getRun(app.db, run.id)!); | |
| 204 | const issues = reports(); | |
| 205 | expect(issues.map((i) => i.title).sort()).toEqual([reportTitle("test/a.test.ts"), reportTitle("test/b.test.ts")]); | |
| 206 | const b = issues.find((i) => i.title === reportTitle("test/b.test.ts"))!; | |
| 207 | expect(b.body).toContain("b > two"); | |
| 208 | expect(b.body).not.toContain("a > one"); | |
| 209 | }); | |
| 210 | ||
| 144 | 211 | test("the issue says only what the run recorded", () => { |
| 145 | 212 | reportRed(app, repo, red(VERIFY_ENV, "a".repeat(40))); |
| 146 | 213 | expect(reports()[0]!.body).not.toContain("run alone"); |
test/shards.test.ts +15
| @@ -708,6 +708,21 @@ describe("failingFiles", () => { | ||
| 708 | 708 | expect(failingFiles(report)).toEqual(["test/b.test.ts"]); |
| 709 | 709 | }); |
| 710 | 710 | |
| 711 | test("bun's closing summary names no file: the green file printed last is not run again", () => { | |
| 712 | const report = [ | |
| 713 | "test/b.test.ts:", | |
| 714 | "(fail) one [0.1ms]", | |
| 715 | "test/c.test.ts:", | |
| 716 | "(pass) fine", | |
| 717 | "1 tests failed:", | |
| 718 | "(fail) one [0.1ms]", | |
| 719 | " 1 pass", | |
| 720 | " 1 fail", | |
| 721 | "Ran 2 tests across 2 files. [9.00ms]", | |
| 722 | ].join("\n"); | |
| 723 | expect(failingFiles(report)).toEqual(["test/b.test.ts"]); | |
| 724 | }); | |
| 725 | ||
| 711 | 726 | test("a report that names no failing test names no file", () => { |
| 712 | 727 | expect(failingFiles("test/a.test.ts:\nerror: cannot load\n")).toEqual([]); |
| 713 | 728 | expect(failingFiles("(fail) before any header")).toEqual([]); |