trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRunsg rEventsFlows

A pull opens on its verdict: can it land, what holds it, then the change #10618

Merged masoncrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

mason/forge:mason/10537-a-pull-opens-on-its-verdict-can-it-land → main 5 commits fork 3 threads · 3 unresolved

  • Merged. mason/10537-a-pull-opens-on-its-verdict-can-it-land was merged into main as 6f0c3fd.

Review

Sign in to review this change.
11 files changed +493 −22
docs/design/attention-budget.json +4 −4
@@ -37,13 +37,13 @@
3737 },
3838 "pull": {
3939 "390": {
40 "words": 133,
41 "blocks": 22,
40 "words": 141,
41 "blocks": 24,
4242 "colours": 4
4343 },
4444 "1280": {
45 "words": 133,
46 "blocks": 22,
45 "words": 141,
46 "blocks": 24,
4747 "colours": 4
4848 }
4949 },
docs/features/attention.md +1 −1
@@ -24,7 +24,7 @@ reader sees on first view at 390 px and 1280 px: **words**, **text blocks** and
2424 | home | 95 / 20 / 2 | 95 / 20 / 2 |
2525 | repository | 116 / 18 / 2 | 116 / 18 / 2 |
2626 | app | 116 / 18 / 2 | 116 / 18 / 2 |
27 | pull | 133 / 22 / 4 | 133 / 22 / 4 |
27 | pull | 141 / 24 / 4 | 141 / 24 / 4 |
2828 | issue | 72 / 19 / 2 | 72 / 19 / 2 |
2929 | issues list | 30 / 7 / 2 | 30 / 7 / 2 |
3030 | pulls list | 24 / 6 / 1 | 24 / 6 / 1 |
docs/features/pull-verdict.md added +19
@@ -0,0 +1,19 @@
1 # A pull opens on its verdict
2
3 This page is for anyone who reviews or merges a pull. Every pull opens on one line: can it land, and if not, what holds it.
4
5 ## Where you see it
6
7 - **The pull's page** opens on the line, then the description's first paragraph and the size of the change (files, `+added −removed`). Reviews, checks and the conversation come after. The number of files links to the Files tab, where the diff's own rows list them.
8 - **The Files tab** repeats the line above the diff, next to the merge box it describes.
9 - **The terminal** (`ssh`, the pull screen) shows the same words under the pull's title.
10
11 ## What it says
12
13 One of: *Ready to land*, *Waiting for a review*, *A reviewer asked for changes*, *Waiting for a team's review*, *Waiting for the check*, *The check failed*, *Conflicts with `<base>`*, *Merges on the origin*, *Merged*, *Closed without merging*. The words are asked in the order the merge box and the merge itself ask them, so the line never says *Ready* where the merge would refuse.
14
15 Beside it, one action when you can take it: **Go to merge** (scrolls to the merge box on Files), **Review**, **See the check**, or **Resolve** for a conflict. It is a link; the merge itself is still the button in the merge box.
16
17 ## Try it
18
19 Open any pull at `/<owner>/<repo>/pulls/<number>`; its first line is the verdict. Over `ssh`, open the pull from the repository's list.
src/tui/screens/pull.ts +36 −2
@@ -77,6 +77,7 @@ import { fromKeptFork } from "../../core/landing_kind.ts";
7777 import { getPull, pullDir, variantSiblings, type MergePullError, type Pull } from "../../core/pulls.ts";
7878 import { voteVariant } from "../../actions/variants.ts";
7979 import { getRepoById, type Repo } from "../../core/repos.ts";
80 import { verdictOf, type Verdict } from "../../web/pages/pull-verdict.ts";
8081 import { wallOf, wallPlain, type Wall } from "../../contract/policy.ts";
8182 import { checkSummary, type CheckSummary } from "../../core/runs.ts";
8283 import { err, ok } from "../../kernel/result.ts";
@@ -636,6 +637,33 @@ export function pullScreen(
636637 const checksGated = (): boolean =>
637638 repo !== undefined && checksHold(requireChecks, check.state, checkOld);
638639
640 /**
641 * The page's verdict line, in the page's words (`verdictOf`): can it land,
642 * and what holds it. Null until the pull is read, or while git has not
643 * said whether it merges.
644 */
645 const verdict = (): Verdict | null => {
646 if (detail === undefined || facts === null || facts.mergeable === null) return null;
647 return verdictOf(
648 {
649 merged: facts.mergedAt !== null,
650 closed: detail.issue.state === "closed",
651 onOrigin: mirrored(),
652 mergeable: facts.mergeable,
653 ahead: facts.diff !== null && facts.diff.files.length === 0 ? 0 : 1,
654 conflicts: facts.conflicts.length,
655 baseBranch: facts.baseBranch,
656 reviewGated: gated(),
657 changesRequested: standing().blocking.length > 0,
658 teamsHolding: holding.length,
659 checksHeld: checksGated(),
660 checkState: check.state,
661 mayMerge: mayWrite(),
662 },
663 "",
664 );
665 };
666
639667 /** `O` — the preview, through a grant when the app is private. */
640668 // Production beside the preview: comparing the two is the review.
641669 const openProduction = (): void => {
@@ -1173,13 +1201,13 @@ export function pullScreen(
11731201 "v",
11741202 [
11751203 len(whereRows(src)),
1176 len(2),
1204 len(3),
11771205 len(strangerLine === "" ? 0 : 1),
11781206 len(isApp ? 1 : 0),
11791207 len(boxH),
11801208 len(2),
11811209 fill(1),
1182 len(Math.min(foot, Math.max(0, area.h - 13 - whereRows(src)))),
1210 len(Math.min(foot, Math.max(0, area.h - 14 - whereRows(src)))),
11831211 ],
11841212 );
11851213 whereStrip(s, strip!, c, f, src);
@@ -1215,6 +1243,12 @@ export function pullScreen(
12151243 style(c.mutedFg, c.bg),
12161244 edge,
12171245 );
1246 const verdictNow = verdict();
1247 // Merged and closed pulls are told by the head line and the merge box.
1248 if (verdictNow !== null && head!.h > 2 && detail?.issue.state === "open" && facts?.mergedAt === null) {
1249 const at = dot(s, head!.x, head!.y + 2, c, verdictNow.tone === undefined ? {} : { tone: verdictNow.tone as Tone });
1250 s.text(at, head!.y + 2, truncate(` ${verdictNow.words}`, Math.max(0, edge - at)), style(c.fg, c.bg, BOLD), edge);
1251 }
12181252 if (strangerLine !== "" && nextRow!.h > 0)
12191253 s.text(nextRow!.x, nextRow!.y, truncate(`next: ${strangerLine}`, nextRow!.w), style(c.mutedFg, c.bg), nextRow!.x + nextRow!.w);
12201254 if (isApp && previewRow!.h > 0)
src/web/pages/pull-verdict.ts added +120
@@ -0,0 +1,120 @@
1 /**
junipercrew

Principle 1/7: changeSummary restates the file list that the diff and the files-where line already carry. Keep one.
2 * What a pull's page opens on: one verdict line (can it land, and if not what
3 * holds it, with the one action that moves it) and, directly under it, the
4 * change itself. Reviews, checks and history come after.
5 *
6 * `verdictOf` is plain data in, plain data out; it asks the questions
7 * `mergeBox` and `mergePull` ask, in the same order, so the line and the
8 * button never disagree.
9 */
10 import type { Diff } from "../../git/diff.ts";
11 import { html, when, type Html } from "../html.ts";
12 import { btn, dot, type Tone } from "../ui.ts";
13
14 export interface VerdictInput {
15 readonly merged: boolean;
16 readonly closed: boolean;
17 /** A mirror: the merge belongs to the origin. */
18 readonly onOrigin: boolean;
19 readonly mergeable: boolean;
20 readonly ahead: number;
21 readonly conflicts: number;
22 readonly baseBranch: string;
23 /** The review gate refuses this head. */
24 readonly reviewGated: boolean;
25 /** A current review asked for changes. */
26 readonly changesRequested: boolean;
27 /** Teams whose reviewer has not approved this head. */
28 readonly teamsHolding: number;
29 /** The check gate refuses this head, and the check's state. */
30 readonly checksHeld: boolean;
31 readonly checkState: string;
32 readonly mayMerge: boolean;
33 }
34
35 export interface Verdict {
36 readonly tone: Tone | undefined;
37 /** A fixed phrase of ours, never a person's text. */
38 readonly words: string;
39 readonly action: { readonly label: string; readonly href: string } | null;
40 }
41
42 /**
43 * The verdict, from the same facts the merge box reads. `thread` is the pull's
44 * address; `page` prefixes the in-page anchors when the line is drawn on a
45 * page that does not hold the merge box itself.
46 */
47 export function verdictOf(v: VerdictInput, thread: string, page = ""): Verdict {
48 if (v.merged) return { tone: "merged", words: "Merged", action: null };
49 if (v.closed) return { tone: undefined, words: "Closed without merging", action: null };
50 if (v.onOrigin) return { tone: "attn", words: "Merges on the origin", action: null };
51 if (!v.mergeable) {
52 if (v.ahead === 0) return { tone: "danger", words: "Nothing left to apply", action: null };
53 if (v.conflicts > 0)
54 return {
55 tone: "danger",
56 words: `Conflicts with ${v.baseBranch}`,
57 action: v.mayMerge ? { label: "Resolve", href: `${thread}/resolve` } : null,
58 };
59 return { tone: "danger", words: "Cannot be applied automatically", action: null };
60 }
61 if (v.reviewGated)
62 return {
63 tone: "attn",
64 words: v.changesRequested ? "A reviewer asked for changes" : "Waiting for a review",
65 action: { label: "Review", href: `${page}#review-body` },
66 };
67 if (v.teamsHolding > 0)
68 return {
69 tone: "attn",
70 words: v.teamsHolding === 1 ? "Waiting for a team's review" : "Waiting for teams' reviews",
71 action: { label: "Review", href: `${page}#review-body` },
72 };
73 if (v.checksHeld)
74 return {
75 tone: v.checkState === "failed" ? "danger" : "attn",
76 words: v.checkState === "failed" ? "The check failed" : "Waiting for the check",
77 action: { label: "See the check", href: `${page}#merge` },
78 };
79 return {
80 tone: "success",
81 words: "Ready to land",
82 action: v.mayMerge ? { label: "Go to merge", href: `${page}#merge` } : null,
83 };
84 }
85
86 /** The verdict as the first line of the page: the dot, the words, the action. */
87 export function verdictLine(verdict: Verdict, show: { readonly action?: boolean } = {}): Html {
88 return html`<div class="bar mt" data-verdict role="status">
89 ${dot(verdict.tone)}
90 <strong>${verdict.words}</strong>
91 ${when(show.action === false ? null : verdict.action, (a) => btn(a.label, { href: a.href, size: "sm", ...(verdict.tone === "success" ? { variant: "primary" as const } : {}) }))}
92 </div>`;
93 }
94
95 /** The first paragraph of a description, as text. */
96 export function firstParagraph(body: string): string {
97 for (const part of body.replace(/\r\n?/g, "\n").split(/\n\s*\n/)) {
98 const text = part.trim();
99 if (text !== "") return text.length > 400 ? `${text.slice(0, 400).trimEnd()}…` : text;
100 }
101 return "";
102 }
103
104 /**
105 * The change itself, once: the description's first paragraph and the total.
106 * The file list is the diff's own rows, one click away at `filesHref`.
107 */
108 export function changeSummary(body: string, diff: Diff | null, filesHref: string): Html {
109 const lead = firstParagraph(body);
110 return html`<section class="mt" data-change>
111 ${when(lead, (text) => html`<p>${text}</p>`)}
112 ${when(
113 diff,
114 (d) => html`<p class="muted small">
115 <a href="${filesHref}">${d.files.length} file${d.files.length === 1 ? "" : "s"}</a> ·
116 <span class="mono">+${d.additions} −${d.deletions}</span>
117 </p>`,
118 )}
119 </section>`;
120 }
src/web/pages/pull.ts +63 −11
@@ -31,6 +31,7 @@ import { RED_ALONE_WAIT, redAloneAt, redMark, type RedMark } from "../../checks/
3131 import { tierLine, type TierState } from "../../checks/tiers.ts";
3232 import { fromKeptFork } from "../../core/landing_kind.ts";
3333 import { combinedCard } from "./pull-combined.ts";
34 import { verdictLine, verdictOf, type VerdictInput } from "./pull-verdict.ts";
junipercrew outdated

The verdict is wired only into pullFilesPage. The pull's main page (src/web/studio/change.ts) and the terminal screen (src/tui/screens/pull.ts) need it too.
3435 import { getAppByRepo } from "../../core/apps.ts";
3536 import { labelsFor, listLabels } from "../../core/issues.ts";
3637 import { marksByName } from "../../core/marks.ts";
@@ -172,6 +173,62 @@ function standings(
172173 }));
173174 }
174175
176 /**
177 * The change between a pull's base and head with its highlighting, kept by
178 * the two oids (`diff-cache.ts`): the Files page and the pull's own page
179 * ask here, so the second reader of a head gets it from memory.
180 */
181 export function keptChange(
182 dir: string,
183 status: PullStatus,
184 stretches: DiffLaps & { highlight?: number } = {},
185 ) {
186 return keptDiff(diffKey(status.baseOid, status.headOid), async () => {
187 const { from, diff, failed } = await diffBetween(dir, status.baseOid, status.headOid, stretches);
188 const highlighting = performance.now();
189 const syntax = await diffSyntax(dir, diff, from, status.headOid);
190 stretches.highlight = performance.now() - highlighting;
191 return { from, diff, failed, syntax };
192 });
193 }
194
195 /**
196 * The facts the verdict line reads, from the same sources the merge box
197 * reads. `status` is absent for a merged or closed pull, which is history.
198 */
199 export function verdictFacts(
200 ctx: Ctx,
201 repo: Repo,
202 pull: Pull,
203 status: PullStatus | undefined,
204 known: Partial<VerdictInput> = {},
205 ): VerdictInput {
206 const db = ctx.app.db;
207 const headOid = status?.headOid ?? pull.headOid;
208 const check = checkSummary(db, pull.id, headOid);
209 const stale =
210 status !== undefined && pull.state === "open" && pull.mergedAt === null
211 ? checkStale(check.run, status.baseOid)
212 : false;
213 const by = standings(db, pull, headOid);
214 return {
215 merged: pull.mergedAt !== null,
216 closed: pull.state === "closed",
217 onOrigin: repo.origin !== null || pull.originHeadRepo !== null,
218 mergeable: status?.mergeable ?? false,
219 ahead: status?.ahead ?? 0,
220 conflicts: status?.conflicts.length ?? 0,
221 baseBranch: pull.baseBranch,
222 reviewGated: status?.reviews != null && !status.reviews.mergeable,
223 changesRequested: by.some((s) => !s.stale && s.state === "changes_requested"),
224 teamsHolding: teamsHolding(db, { ...pull, headOid }).length,
225 checksHeld: checksHold(repo.requireChecks, check.state, stale),
226 checkState: check.state,
227 mayMerge: canWrite(db, repo, ctx.auth.user),
228 ...known,
229 };
230 }
231
175232 /**
176233 * The reviews chip: the state of play in one line.
177234 *
@@ -923,15 +980,7 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
923980 // On a miss the stretches say where the time went; a hit has none.
924981 const stretches: DiffLaps & { highlight?: number } = {};
925982 const cacheBefore = diffCacheCounts();
926 const { diff, syntax, failed } = await lap("diff", () =>
927 keptDiff(diffKey(status.baseOid, status.headOid), async () => {
928 const { from, diff, failed } = await diffBetween(dir, status.baseOid, status.headOid, stretches);
929 const highlighting = performance.now();
930 const syntax = await diffSyntax(dir, diff, from, status.headOid);
931 stretches.highlight = performance.now() - highlighting;
932 return { from, diff, failed, syntax };
933 }),
934 );
983 const { diff, syntax, failed } = await lap("diff", () => keptChange(dir, status, stretches));
935984 const cacheAfter = diffCacheCounts();
936985 const diffHit = cacheAfter.hits > cacheBefore.hits && cacheAfter.misses === cacheBefore.misses;
937986 // A merged or closed pull is history; the wall is a question about landing.
@@ -1051,12 +1100,16 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
10511100 },
10521101 );
10531102
1103 const holdingTeams = teamsHolding(ctx.app.db, { ...pull, headOid: status.headOid }).map((v) => v.name);
1104 const verdict = verdictOf(verdictFacts(ctx, repo, pull, status), thread);
1105
10541106 const body = html`${repoHeader(ctx, repo, "pulls", {
10551107 compact: true,
10561108 ownTitle: true,
10571109 })}
10581110 ${changeHead(ctx, repo, pull, { headOid: status.headOid, conflicts: status.conflicts.length })}
10591111 ${changeTabs(repo, pull, "files")}
1112 ${verdictLine(verdict)}
10601113 <p class="files-where muted small" id="files">
10611114 <span class="mono">${headLabel} → ${pull.baseBranch}</span>
10621115 <span class="sep" aria-hidden="true">·</span>
@@ -1079,8 +1132,7 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
10791132 ${when(wall, (w) => wallCard(w, mayMerge ? thread : undefined))}
10801133 ${holdCard(ctx, repo, pull, { act: false })}
10811134 ${teamsCard(teamVerdicts(ctx.app.db, { ...pull, headOid: status.headOid }))}
1082 ${mergeBox(ctx, repo, pull, status, mayMerge, by, check, isApp, tier, stale,
1083 teamsHolding(ctx.app.db, { ...pull, headOid: status.headOid }).map((v) => v.name))}
1135 <div id="merge">${mergeBox(ctx, repo, pull, status, mayMerge, by, check, isApp, tier, stale, holdingTeams)}</div>
10841136 ${slicesCard(base, thread, slices, ends.length > 0 ? ends.length : status.ahead, ends, pull.state === "open" && pull.mergedAt === null, mayLand)}
10851137 ${await combinedCard(dir, pull, status.headOid, status.baseOid)}
10861138 <div class="split">
src/web/studio/change.ts +10 −1
@@ -64,7 +64,8 @@ import {
6464 } from "../ui.ts";
6565 import { previewBlock } from "../pages/apps.ts";
6666 import { lookCard } from "../pages/looks.ts";
67 import { approveHereCard, pullChecksLineFor, teamsCard } from "../pages/pull.ts";
67 import { approveHereCard, keptChange, pullChecksLineFor, teamsCard, verdictFacts } from "../pages/pull.ts";
68 import { changeSummary, verdictLine, verdictOf } from "../pages/pull-verdict.ts";
6869 import { guestWalls, strangerCard } from "../pages/pull-guest.ts";
6970 import { combinedCard } from "../pages/pull-combined.ts";
7071 import { teamVerdicts } from "../../core/issue_teams.ts";
@@ -730,7 +731,15 @@ async function changeConversation(
730731 spent.reviews += performance.now() - reviewsAt;
731732
732733 // From here on nothing is read: `render` only builds the page.
734 // The page opens on its verdict, then the change; the merge box and the
735 // reviews live on Files, so the line's anchors point there.
736 const files = `${thread}/files`;
737 const totals =
738 status === undefined ? null : (await lap(spent, "diff", () => keptChange(wordCtx.dir, status))).diff;
739 const verdict = verdictOf(verdictFacts(ctx, repo, pull, status, { mayMerge: writer }), thread, files);
733740 const body = html`${pageHead.top}
741 ${verdictLine(verdict, { action: false })}
742 ${changeSummary(pull.body, totals, files)}
734743 ${when(siblings.length > 0, () =>
735744 html`<p class="muted small variant-link">
736745 <a href="${base}/variants/${pull.variantGroup ?? ""}"
test/pages-budget.test.ts +1 −1
@@ -34,7 +34,7 @@ const DEFAULT = { statements: 60, git: 8 };
3434 const HEAVIER: Record<string, { statements?: number; git?: number }> = {
3535 "/:owner/:name/issues/:number": { statements: 70 },
3636 "/:owner/:name/next": { statements: 64 },
37 "/:owner/:name/pulls/:number": { statements: 106 },
37 "/:owner/:name/pulls/:number": { statements: 108 },
3838 "/board": { statements: 64 },
3939 "/settings/capacity": { statements: 71 },
4040 };
test/pull-quiet.test.ts +2 −2
@@ -66,8 +66,8 @@ afterAll(async () => {
6666 const get = async (path: string) => (await fetch(`${servers.url}${path}`, { headers: { cookie } })).text();
6767 const bytes = (t: string) => new TextEncoder().encode(t).length;
6868
69 // Measured 31,130 (pull; 31,084 before the Your shells palette entry) and 25,064 (list) with the Displays entry in the account menu and palette. Each budget is that plus ~160 B, the spread the port digits and times in the page showed, so it holds the page from growing and is not a tighter squeeze.
70 const BUDGET = { pull: 31_400, list: 25_400 };
69 // Measured ~31,650 (pull: 31,130 on main plus the verdict line and the change above the check) and 25,064 (list) with the Displays entry in the account menu and palette. Each budget is that plus ~160 B, the spread the port digits and times in the page showed, so it holds the page from growing and is not a tighter squeeze.
70 const BUDGET = { pull: 31_810, list: 25_400 };
7171
7272 test("the pull page's check is a line with a dot and its word, not a card around a badge", async () => {
7373 const text = await get(`/alice/r/pulls/${number}`);
test/pull-verdict-tui.test.ts added +89
@@ -0,0 +1,89 @@
1 /**
2 * The terminal's pull screen opens on the same verdict as the page, in the
3 * same words (`verdictOf`).
4 */
5 import { afterAll, beforeAll, expect, test } from "bun:test";
6 import { mkdtemp, rm } from "node:fs/promises";
7 import { tmpdir } from "node:os";
8 import { join } from "node:path";
9 import { openPull } from "../src/actions/pulls.ts";
10 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
11 import { createApp, type App } from "../src/app.ts";
12 import { createUser } from "../src/core/users.ts";
13 import { createForgeApp } from "../src/tui/forge.ts";
14 import { pullScreen } from "../src/tui/screens/pull.ts";
15 import { local } from "../src/tui/screens/where.ts";
16 import { headlessTerminal } from "../src/tui/term/terminal.ts";
17 import { quietLogs } from "./helpers/globals.ts";
18 import { settleApp } from "./helpers/tui.ts";
19
20 quietLogs();
21
22 let dataDir: string;
23 let app: App;
24
25 beforeAll(async () => {
26 dataDir = await mkdtemp(join(tmpdir(), "forge-tui-verdict-"));
27 app = createApp({ dataDir, port: 0, host: "forge.test" });
28 });
29
30 afterAll(async () => {
31 app.close();
32 await rm(dataDir, { recursive: true, force: true });
33 });
34
35 test("the pull screen names whether it can land and what holds it", async () => {
36 const made = await createUser(app.db, {
37 origin: "seed",
38 trusted: true,
39 username: "alice",
40 email: "[email protected]",
41 password: "a-good-passphrase",
42 adminIfFirst: true,
43 });
44 if (!made.ok) throw new Error(String(made.error));
45 const alice = made.value;
46 const r = await createRepo(app, alice, { name: "lib", visibility: "public" });
47 if (!r.ok) throw new Error(String(r.error));
48 const repo = r.value;
49 const put = async (branch: string, file: string, content: string) => {
50 const c = await commitToBranch(app, alice, repo, {
51 branch,
52 ...(branch === "main" ? {} : { base: "main" }),
53 message: branch,
54 files: [{ path: file, content }],
55 });
56 if (!c.ok) throw new Error(JSON.stringify(c.error));
57 };
58 await put("main", "a.txt", "main");
59 await put("clean", "c.txt", "x");
60 await put("clash", "a.txt", "theirs");
61 await put("main", "a.txt", "ours");
62 const open = async (branch: string) => {
63 const o = await openPull(app, alice, repo, { title: branch, body: "", baseBranch: "main", headBranch: branch });
64 if (!o.ok) throw new Error(String(o.error));
65 return o.value.number;
66 };
67 const clean = await open("clean");
68 const clash = await open("clash");
69
70 const { app: tui, session } = createForgeApp(
71 app,
72 { sshUsername: "alice", remoteAddress: "127.0.0.1", fingerprint: "SHA256:alice", user: alice },
73 { terminal: headlessTerminal(160, 44), exitHooks: false },
74 );
75 tui.start();
76 const show = async (number: number): Promise<string> => {
77 tui.ctx.push(pullScreen(session, tui.ctx, local(session, repo), number));
78 await settleApp(tui);
79 return tui.screen.toText();
80 };
81 try {
82 expect(await show(clean)).toContain("Ready to land");
83 expect(await show(clash)).toContain("Conflicts with main");
84 app.db.run("UPDATE repo SET require_checks = 1 WHERE id = ?", [repo.id]);
85 expect(await show(clean)).toContain("Waiting for the check");
86 } finally {
87 tui.stop();
88 }
89 });
test/pull-verdict.test.ts added +148
@@ -0,0 +1,148 @@
1 /**
junipercrew

Only /files is tested; add the pull's main page and the terminal screen. firstDetail relies on merge-box wording strings.
2 * A pull's page opens on its verdict (can it land, what holds it) and then the
3 * change itself; reviews and checks come after.
4 */
5 import { afterAll, beforeAll, expect, test } from "bun:test";
6 import { mkdtemp, rm } from "node:fs/promises";
7 import { tmpdir } from "node:os";
8 import { join } from "node:path";
9 import { createApp, type App } from "../src/app.ts";
10 import { openPull } from "../src/actions/pulls.ts";
11 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
12 import { createSession } from "../src/core/sessions.ts";
13 import { createUser, type User } from "../src/core/users.ts";
14 import { serve, type Servers } from "../src/http/server.ts";
15 import { firstParagraph, verdictOf, type VerdictInput } from "../src/web/pages/pull-verdict.ts";
16 import { quietLogs } from "./helpers/globals.ts";
17
18 quietLogs();
19
20 let dataDir: string;
21 let app: App;
22 let servers: Servers;
23 let alice: User;
24 const numbers: Record<string, number> = {};
25 const cells = process.env["FORGE_CELLS"];
26
27 const pageOf = async (repo: string, tab = "/files"): Promise<string> => {
28 const res = await fetch(`${servers.url}/alice/${repo}/pulls/${numbers[repo]}${tab}`, {
29 headers: { cookie: `forge_session=${createSession(app.db, alice.id).token}` },
30 });
31 return res.text();
32 };
33
34 const commit = async (repo: Awaited<ReturnType<typeof createRepo>>, branch: string | undefined, file: string, content: string) => {
35 if (!repo.ok) throw new Error(String(repo.error));
36 const done = await commitToBranch(app, alice, repo.value, {
37 ...(branch === undefined ? {} : { branch }),
38 message: "change",
39 files: [{ path: file, content }],
40 });
41 if (!done.ok) throw new Error(JSON.stringify(done.error));
42 };
43
44 beforeAll(async () => {
45 process.env["FORGE_CELLS"] = "off";
46 dataDir = await mkdtemp(join(tmpdir(), "forge-verdict-"));
47 app = createApp({ dataDir, port: 0 });
48 servers = await serve(app);
49 const made = await createUser(app.db, {
50 origin: "seed",
51 trusted: true,
52 username: "alice",
53 email: "[email protected]",
54 password: "a-good-passphrase",
55 adminIfFirst: true,
56 });
57 if (!made.ok) throw new Error(String(made.error));
58 alice = made.value;
59 for (const name of ["ready", "held", "conflicted"]) {
60 const repo = await createRepo(app, alice, { name, visibility: "public" });
61 await commit(repo, undefined, "forge.json", "{}");
62 await commit(repo, "topic", name === "conflicted" ? "forge.json" : "t.txt", name === "conflicted" ? '{"a":1}' : "x");
63 if (name === "conflicted") await commit(repo, undefined, "forge.json", '{"b":2}');
64 if (!repo.ok) throw new Error("repo");
65 const pull = await openPull(app, alice, repo.value, {
66 title: name,
67 body: `The ${name} change, in a line.\n\nA second paragraph that stays below.`,
68 baseBranch: "main",
69 headBranch: "topic",
70 });
71 if (!pull.ok) throw new Error(JSON.stringify(pull.error));
72 numbers[name] = pull.value.number;
73 if (name === "held") app.db.run("UPDATE repo SET require_checks = 1 WHERE id = ?", [repo.value.id]);
74 }
75 });
76
77 afterAll(async () => {
78 await servers.stop();
79 app.close();
80 if (cells === undefined) delete process.env["FORGE_CELLS"];
81 else process.env["FORGE_CELLS"] = cells;
82 await rm(dataDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 });
83 });
84
85 const at = (body: string, needle: string): number => {
86 const i = body.indexOf(needle);
87 expect(i).toBeGreaterThan(-1);
88 return i;
89 };
90
91 for (const [name, words] of [
92 ["ready", "Ready to land"],
93 ["held", "Waiting for the check"],
94 ["conflicted", "Conflicts with main"],
95 ] as const) {
96 test(`${name}: the files tab opens on the verdict, before the merge box`, async () => {
97 const body = await pageOf(name);
98 const verdict = at(body, "data-verdict");
99 expect(body.indexOf(words, verdict)).toBeGreaterThan(verdict);
100 expect(verdict).toBeLessThan(at(body, 'id="merge"'));
101 expect(body.split("data-verdict").length).toBe(2);
102 });
103
104 test(`${name}: the pull's own page opens on the verdict, then the change, before the check`, async () => {
105 const body = await pageOf(name, "");
106 const verdict = at(body, "data-verdict");
107 expect(body.indexOf(words, verdict)).toBeGreaterThan(verdict);
108 const change = at(body, "data-change");
109 expect(change).toBeGreaterThan(verdict);
110 expect(body.indexOf(`The ${name} change, in a line.`, change)).toBeGreaterThan(change);
111 expect(change).toBeLessThan(at(body, 'id="pull-check"'));
112 expect(body).toMatch(/data-change[\s\S]*?<a href="[^"]*\/files">1 file<\/a>[\s\S]*?\+\d+ −\d+/);
113 expect(body).not.toContain("and 0 more");
114 });
115 }
116
117 test("ready offers Go to merge beside the verdict on Files; conflicted offers Resolve", async () => {
118 expect(await pageOf("ready")).toMatch(/data-verdict[\s\S]*?<a[^>]*href="#merge"[^>]*>[\s\S]*?Go to merge/);
119 // The pull's own page has its own Ship button below; the line carries no second action.
120 const own = await pageOf("ready", "");
121 const line = own.slice(own.indexOf("data-verdict"), own.indexOf("data-change"));
122 expect(line).not.toContain("<a");
123 expect(await pageOf("conflicted")).toMatch(/data-verdict[\s\S]*?href="\/alice\/conflicted\/pulls\/\d+\/resolve"/);
124 });
125
126 const base: VerdictInput = {
127 merged: false, closed: false, onOrigin: false, mergeable: true, ahead: 1, conflicts: 0,
128 baseBranch: "main", reviewGated: false, changesRequested: false, teamsHolding: 0,
129 checksHeld: false, checkState: "ok", mayMerge: true,
130 };
131
132 test("verdictOf names what holds a pull, in the merge box's order", () => {
133 const words = (o: Partial<VerdictInput>) => verdictOf({ ...base, ...o }, "/a/b/pulls/1").words;
134 expect(words({})).toBe("Ready to land");
135 expect(words({ reviewGated: true, changesRequested: true })).toBe("A reviewer asked for changes");
136 expect(words({ reviewGated: true })).toBe("Waiting for a review");
137 expect(words({ teamsHolding: 1 })).toBe("Waiting for a team's review");
138 expect(words({ checksHeld: true, checkState: "running" })).toBe("Waiting for the check");
139 expect(words({ checksHeld: true, checkState: "failed" })).toBe("The check failed");
140 expect(words({ mergeable: false, conflicts: 2 })).toBe("Conflicts with main");
141 expect(words({ merged: true })).toBe("Merged");
142 expect(verdictOf({ ...base, mayMerge: false }, "/t").action).toBeNull();
143 });
144
145 test("the description's first paragraph only", () => {
146 expect(firstParagraph("\n\nOne.\nStill one.\n\nTwo.")).toBe("One.\nStill one.");
147 expect(firstParagraph("")).toBe("");
148 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs