Public: anyone can read this; the crew can change it.
A running check is named where a person looks: the pull's checks line and the run's title say what it is, how long it has run, and link to it #2297
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1123-a-running-check-is-named-where-a-person → main 4 commits fork 2 threads · 2 unresolved
10 files
changed +96
−23
src/checks/summary.ts +2 −1
| @@ -9,6 +9,7 @@ | ||
| 9 | 9 | |
| 10 | 10 | import type { CheckDetail, Run } from "../core/runs.ts"; |
| 11 | 11 | import { parseBunTestFailure } from "./bun-test.ts"; |
| 12 | import { oneLine } from "./words.ts"; | |
| 12 | 13 | |
| 13 | 14 | /** |
| 14 | 15 | * The first line of the block a timed full gate ends its log with |
| @@ -104,7 +105,7 @@ export function checkWords( | ||
| 104 | 105 | : `Check passed in ${spanWords(Date.parse(run.finishedAt) - began)}`; |
| 105 | 106 | if (run.state === "failed") { |
| 106 | 107 | const first = run.checkDetail?.names[0] ?? run.summary; |
| 107 | return first === "" ? "Check failed" : `Check failed: ${first}`; | |
| 108 | return first === "" ? "Check failed" : `Check failed: ${oneLine(first)}`; | |
| 108 | 109 | } |
| 109 | 110 | return undefined; |
| 110 | 111 | } |
src/checks/words.ts added +8
| @@ -0,0 +1,8 @@ | ||
| 1 | /** One line of a failure's summary on a narrow screen; the rest is in the run. */ | |
| 2 | const LINE_MAX = 80; | |
| 3 | ||
| 4 | /** `text` cut to one line of at most `LINE_MAX` characters, with an ellipsis. */ | |
| 5 | export function oneLine(text: string): string { | |
| 6 | const flat = text.replace(/\s+/g, " ").trim(); | |
| 7 | return flat.length > LINE_MAX ? `${flat.slice(0, LINE_MAX - 1)}…` : flat; | |
| 8 | } | |
src/tui/screens/pull.ts +21 −12
| @@ -51,6 +51,8 @@ import { getIssue, type Issue } from "../../core/issues.ts"; | ||
| 51 | 51 | import { isProposed, pullMayRun } from "../../core/pull_trust.ts"; |
| 52 | 52 | import { askCheck } from "../../checks/again.ts"; |
| 53 | 53 | import { checkStale, checksHold } from "../../checks/stale.ts"; |
| 54 | import { checkWords } from "../../checks/summary.ts"; | |
| 55 | import { oneLine } from "../../checks/words.ts"; | |
| 54 | 56 | import { resolveOid } from "../../git/merge.ts"; |
| 55 | 57 | import { getPull, pullDir, variantSiblings, type Pull } from "../../core/pulls.ts"; |
| 56 | 58 | import { voteVariant } from "../../actions/variants.ts"; |
| @@ -1242,28 +1244,35 @@ export function checksText( | ||
| 1242 | 1244 | ? "checks: no deploy of this head yet" |
| 1243 | 1245 | : ""; |
| 1244 | 1246 | const run = check.run; |
| 1245 | const summary = `${ | |
| 1246 | run === null || run.summary === "" ? "" : ` · ${run.summary}` | |
| 1247 | }${ | |
| 1247 | const against = | |
| 1248 | 1248 | run !== null && run.baseOid !== "" && baseBranch !== "" |
| 1249 | 1249 | ? ` · checked against ${baseBranch} at ${run.baseOid.slice(0, 7)}` |
| 1250 | : "" | |
| 1251 | }`; | |
| 1250 | : ""; | |
| 1251 | const summary = `${ | |
| 1252 | run === null || run.summary === "" ? "" : ` · ${run.summary}` | |
| 1253 | }${against}`; | |
| 1254 | const isCheck = run !== null && run.kind === "check"; | |
| 1255 | const words = run === null ? undefined : checkWords({ ...run, state: check.state }); | |
| 1256 | const ofRun = `${run === null ? "" : ` · run ${run.id}`}${against}`; | |
| 1252 | 1257 | switch (check.state) { |
| 1253 | 1258 | case "ok": |
| 1254 | 1259 | return stale |
| 1255 | 1260 | ? `checks: passed against an older ${baseBranch || "base"}${summary}` |
| 1256 | : `checks: passed${summary}`; | |
| 1261 | : words !== undefined | |
| 1262 | ? `${words}${ofRun}` | |
| 1263 | : `checks: passed${summary}`; | |
| 1257 | 1264 | case "failed": |
| 1258 | return `checks: failed${summary}`; | |
| 1265 | return words !== undefined | |
| 1266 | ? `${words}${ofRun}` | |
| 1267 | : `checks: failed${summary}`; | |
| 1259 | 1268 | case "running": |
| 1260 | return "checks: deploying this head"; | |
| 1269 | return words !== undefined ? `${words}${ofRun}` : "checks: deploying this head"; | |
| 1261 | 1270 | case "queued": |
| 1262 | return `checks: a deploy of this head is queued${ | |
| 1271 | return `checks: a ${isCheck ? "check" : "deploy"} of this head is queued${ | |
| 1263 | 1272 | run === null ? "" : ` (queued ${relTime(run.createdAt)})` |
| 1264 | 1273 | }`; |
| 1265 | 1274 | case "cancelled": |
| 1266 | return `checks: the deploy was cancelled${summary}`; | |
| 1275 | return `checks: the ${isCheck ? "check" : "deploy"} was cancelled${summary}`; | |
| 1267 | 1276 | } |
| 1268 | 1277 | } |
| 1269 | 1278 | |
| @@ -1273,10 +1282,10 @@ export function checksRefusal(check: CheckSummary, old = false): string { | ||
| 1273 | 1282 | return "checked against an older base; press C to check again"; |
| 1274 | 1283 | switch (check.state) { |
| 1275 | 1284 | case "failed": |
| 1276 | return `the deploy of this head failed${check.run?.summary ? `: ${check.run.summary}` : ""}`; | |
| 1285 | return `the ${check.run?.kind === "check" ? "check" : "deploy"} of this head failed${check.run?.summary ? `: ${oneLine(check.run.summary)}` : ""}`; | |
| 1277 | 1286 | case "running": |
| 1278 | 1287 | case "queued": |
| 1279 | return "wait for the deploy of this head to pass"; | |
| 1288 | return `wait for the ${check.run?.kind === "check" ? "check" : "deploy"} of this head to pass`; | |
| 1280 | 1289 | default: |
| 1281 | 1290 | return errorText("checks_required"); |
| 1282 | 1291 | } |
src/tui/screens/runlist.ts +4
| @@ -58,6 +58,10 @@ export function runTitle( | ||
| 58 | 58 | names: ReadonlyMap<string, { username: string }>, |
| 59 | 59 | set: IconSet = "plain", |
| 60 | 60 | ): string { |
| 61 | if (run.kind === "check") { | |
| 62 | const pull = run.env.startsWith("pull-") ? Number(run.env.slice(5)) : undefined; | |
Number(run.env.slice(5)) can be NaN, which prints "for #NaN". Guard with Number.isInteger and omit the suffix otherwise; add a test. |
||
| 63 | return `Check of ${run.commitOid.slice(0, 7)}${pull === undefined ? "" : ` for #${pull}`}`; | |
| 64 | } | |
| 61 | 65 | if (run.kind === "deploy") return `deploy ${envLabel(run.env)}`; |
| 62 | 66 | const who = |
| 63 | 67 | run.actorId === null |
src/web/pages/pull.ts +5 −5
| @@ -249,19 +249,19 @@ export function checksLine( | ||
| 249 | 249 | >${icon("clock")}Checked against an older ${baseBranch || "base"}</span |
| 250 | 250 | >${link}` |
| 251 | 251 | : check.state === "ok" |
| 252 | ? html`<span class="ok">${icon("check")}${words ?? "Checks passed"}</span>${link}` | |
| 252 | ? html`<span class="ok">${icon("check")}${words ?? "Check passed"}</span>${link}` | |
| 253 | 253 | : check.state === "failed" |
| 254 | ? html`<span class="bad">${icon("x")}${words ?? "Checks failed"}</span>${link}` | |
| 254 | ? html`<span class="bad">${icon("x")}${words ?? "Check failed"}</span>${link}` | |
| 255 | 255 | : check.state === "queued" |
| 256 | 256 | ? html`<span class="now" |
| 257 | >${icon("loader")}Checks | |
| 257 | >${icon("loader")}Check | |
| 258 | 258 | ${run ? queuedPhrase(run.createdAt) : "queued"}</span |
| 259 | 259 | >${link}` |
| 260 | 260 | : check.state === "running" |
| 261 | ? html`<span class="now">${icon("loader")}${words ?? "Checks running"}</span | |
| 261 | ? html`<span class="now">${icon("loader")}${words ?? "Check running"}</span | |
| 262 | 262 | >${link}` |
| 263 | 263 | : check.state === "cancelled" |
| 264 | ? html`<span>${icon("ban")}Checks cancelled</span>${link}` | |
| 264 | ? html`<span>${icon("ban")}Check cancelled</span>${link}` | |
| 265 | 265 | : waiting |
| 266 | 266 | ? html`<span>${icon("lock")}${WAITING_WORDS}</span>` |
| 267 | 267 | : html`<span>${icon("clock")}No checks on this head yet</span>`; |
test/check-running-line.test.ts added +51
| @@ -0,0 +1,51 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import type { Run } from "../src/core/runs.ts"; | |
| 3 | import { checksLine } from "../src/web/pages/pull.ts"; | |
| 4 | import { checksRefusal, checksText } from "../src/tui/screens/pull.ts"; | |
| 5 | import { runTitle } from "../src/tui/screens/runlist.ts"; | |
| 6 | ||
| 7 | const at = (msAgo: number) => new Date(Date.now() - msAgo).toISOString(); | |
| 8 | const run = (over: Partial<Run>): Run => | |
| 9 | ({ | |
| 10 | id: "r1", | |
| 11 | kind: "check", | |
| 12 | state: "running", | |
| 13 | commitOid: "abcdef0123456789", | |
| 14 | env: "pull-7", | |
| 15 | summary: "", | |
| 16 | baseOid: "", | |
| 17 | createdAt: at(13 * 60_000), | |
| 18 | startedAt: at(12 * 60_000), | |
| 19 | finishedAt: null, | |
| 20 | checkDetail: null, | |
| 21 | ...over, | |
| 22 | }) as Run; | |
| 23 | const line = (r: Run) => | |
| 24 | checksLine("/a/b", { state: r.state, run: r }, false, true).text; | |
| 25 | const term = (r: Run) => checksText({ state: r.state, run: r }, false, true); | |
| 26 | ||
| 27 | test("a long failure is cut to one line on the page and in the terminal", () => { | |
| 28 | const bad = run({ state: "failed", summary: "x".repeat(300) }); | |
| 29 | expect(line(bad)).toContain(`${"x".repeat(79)}…`); | |
| 30 | expect(line(bad)).not.toContain("x".repeat(80)); | |
| 31 | expect(term(bad)).toContain(`${"x".repeat(79)}…`); | |
| 32 | expect(checksRefusal({ state: "failed", run: bad })).not.toContain("x".repeat(80)); | |
| 33 | }); | |
| 34 | ||
| 35 | test("a queued check is singular like the other states", () => { | |
| 36 | expect(line(run({ state: "queued" }))).toMatch(/Check\s+queued/); | |
| 37 | }); | |
| 38 | ||
| 39 | test("the terminal says the same words as the page and names the run", () => { | |
| 40 | expect(term(run({}))).toBe("Check running · 12 min · run r1"); | |
| 41 | const done = run({ state: "ok", finishedAt: at(0), startedAt: at(41 * 60_000) }); | |
| 42 | expect(term(done)).toBe("Check passed in 41 min · run r1"); | |
| 43 | const bad = run({ state: "failed", summary: "boom" }); | |
| 44 | expect(term(bad)).toBe("Check failed: boom · run r1"); | |
| 45 | expect(checksRefusal({ state: "failed", run: bad })).toBe("the check of this head failed: boom"); | |
| 46 | expect(term(run({ state: "queued" }))).toContain("a check of this head is queued"); | |
| 47 | }); | |
| 48 | ||
| 49 | test("the terminal's run list titles a check run", () => { | |
| 50 | expect(runTitle(run({}), new Map())).toBe("Check of abcdef0 for #7"); | |
| 51 | }); |
test/check-stale-wiring.test.ts +1 −1
| @@ -83,7 +83,7 @@ describe("a green from an older main says so where people read it", () => { | ||
| 83 | 83 | const res = await fetch(`${servers.url}/alice/lib/pulls/${pull.number}/files`); |
| 84 | 84 | const page = await res.text(); |
| 85 | 85 | expect(page).toContain("Checked against an older main"); |
| 86 | expect(page).not.toContain("Checks passed"); | |
| 86 | expect(page).not.toContain("Check passed"); | |
| 87 | 87 | }); |
| 88 | 88 | |
| 89 | 89 | test("the terminal's pull screen says passed against an older main", async () => { |
test/check-stale.test.ts +1 −1
| @@ -21,7 +21,7 @@ test("the pull page says so in place of green", () => { | ||
| 21 | 21 | const stale = checksLine("/a/b", ok, false, true, false, "", "", 0, "main", true).text; |
| 22 | 22 | expect(stale).toContain("Checked against an older main"); |
| 23 | 23 | expect(stale).not.toContain('class="ok"'); |
| 24 | expect(checksLine("/a/b", ok, false, true, false, "", "", 0, "main", false).text).toContain("Checks passed"); | |
| 24 | expect(checksLine("/a/b", ok, false, true, false, "", "", 0, "main", false).text).toContain("Check passed"); | |
| 25 | 25 | }); |
| 26 | 26 | |
| 27 | 27 | test("the terminal says the green is against an older main", async () => { |
test/checks.test.ts +1 −1
| @@ -480,7 +480,7 @@ describe("checks", () => { | ||
| 480 | 480 | expect(checksText(green, false, true, "main", true)).toContain( |
| 481 | 481 | "checks: passed against an older main", |
| 482 | 482 | ); |
| 483 | expect(checksText(green, false, true, "main", false)).toContain("checks: passed"); | |
| 483 | expect(checksText(green, false, true, "main", false)).toContain("Check passed"); | |
| 484 | 484 | expect(checksText(green, false, true, "main", false)).not.toContain("older"); |
| 485 | 485 | expect( |
| 486 | 486 | checksForHead(app.db, pull.id, branched.value.commitOid)[0]?.id, |
test/pull-checks-line.test.ts +2 −2
| @@ -7,7 +7,7 @@ test("a conflicted pull with a passing check has no green mark and says so", () | ||
| 7 | 7 | const line = checksLine("/a/b", ok, false, true, false, "", "", 2).text; |
| 8 | 8 | expect(line).not.toContain('class="ok"'); |
| 9 | 9 | expect(line).toContain("Conflicts in 2 files"); |
| 10 | expect(line).not.toContain("Checks passed"); | |
| 10 | expect(line).not.toContain("Check passed"); | |
| 11 | 11 | expect(checksLine("/a/b", ok, false, true, false, "", "", 1).text).toContain( |
| 12 | 12 | "Conflicts in 1 file</span", |
| 13 | 13 | ); |
| @@ -16,6 +16,6 @@ test("a conflicted pull with a passing check has no green mark and says so", () | ||
| 16 | 16 | test("resolving the conflict brings the green back", () => { |
| 17 | 17 | const line = checksLine("/a/b", ok, false, true, false, "", "", 0).text; |
| 18 | 18 | expect(line).toContain('class="ok"'); |
| 19 | expect(line).toContain("Checks passed"); | |
| 19 | expect(line).toContain("Check passed"); | |
| 20 | 20 | expect(line).not.toContain("Conflicts in"); |
| 21 | 21 | }); |
new above
offline
The ellipsis is a single character, so the line is at most 80 characters, which is right. Consider a test for text of exactly 80 characters, which should not be cut.