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

A test pins opening a pull that cannot be one: same branch, unknown head, or nothing ahead of base, and no pull is made #7142

Merged opened this

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

Merged

scout/forge:scout/7129-a-test-pins-opening-a-pull-that-cannot-b → main 1 commit fork 2 threads · 2 unresolved

  • Merged. scout/7129-a-test-pins-opening-a-pull-that-cannot-b was merged into main as 3c548c3.

Review

Sign in to review this change.
1 file changed +157 −0
test/pull-open-refusals.test.ts added +157
@@ -0,0 +1,157 @@
1 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
2 import { mkdtemp, rm } from "node:fs/promises";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { createApp, type App } from "../src/app.ts";
6 import { serve, type Servers } from "../src/http/server.ts";
7 import { gitClientEnv } from "./helpers/git-client.ts";
8 import { proveAll } from "./helpers/verified.ts";
9 import { promote } from "./helpers/members.ts";
10 import { tokenFor } from "./helpers/tokens.ts";
11
12 const gitEnv = gitClientEnv({
13 GIT_AUTHOR_NAME: "X",
14 GIT_AUTHOR_EMAIL: "[email protected]",
15 GIT_COMMITTER_NAME: "X",
16 GIT_COMMITTER_EMAIL: "[email protected]",
17 });
18
19 describe("opening a pull that cannot be one", () => {
20 let dataDir: string;
21 let work: string;
22 let app: App;
23 let servers: Servers;
24 let base: string;
25 let token = "";
26
27 async function gitOk(args: string[], cwd: string): Promise<void> {
28 const p = Bun.spawn(["git", ...args], {
29 cwd,
30 env: gitEnv,
31 stdout: "pipe",
32 stderr: "pipe",
33 });
34 const [err, code] = await Promise.all([
35 new Response(p.stderr).text(),
36 p.exited,
37 new Response(p.stdout).text(),
38 ]);
39 if (code !== 0) throw new Error(`git ${args.join(" ")}: ${err}`);
40 }
41
42 const call = (path: string, body: unknown): Promise<Response> =>
43 fetch(`${base}${path}`, {
44 method: "POST",
45 headers: {
46 authorization: `Bearer ${token}`,
47 "content-type": "application/json",
48 },
49 body: JSON.stringify(body),
50 });
51
52 const open = (body: Record<string, unknown>) =>
53 call("/api/v1/repos/alice/app/pulls", { title: "A change", ...body });
54
55 const pullRows = (): number =>
56 app.db
57 .query<{ n: number }, []>("SELECT count(*) AS n FROM pull")
58 .get()!.n;
59 const issueRows = (): number =>
60 app.db
61 .query<{ n: number }, []>("SELECT count(*) AS n FROM issue")
62 .get()!.n;
63
64 async function refused(body: Record<string, unknown>, code: string) {
65 const pulls = pullRows();
66 const issues = issueRows();
67 const res = await open(body);
68 expect(res.status).toBeGreaterThanOrEqual(400);
69 expect(res.status).toBeLessThan(500);
70 expect(JSON.stringify(await res.json())).toContain(code);
71 expect(pullRows()).toBe(pulls);
72 expect(issueRows()).toBe(issues);
73 }
74
75 beforeAll(async () => {
76 dataDir = await mkdtemp(join(tmpdir(), "forge-refuse-"));
77 work = await mkdtemp(join(tmpdir(), "forge-refuse-work-"));
78 app = createApp({ dataDir, port: 0 });
79 servers = await serve(app);
80 base = servers.url;
81 await fetch(`${base}/signup`, {
82 method: "POST",
83 body: new URLSearchParams({
84 username: "alice",
85 email: "[email protected]",
86 password: "refusal-pass-1",
87 }),
88 redirect: "manual",
89 });
90 token = tokenFor(app, "alice", "admin", "r");
91 proveAll(app);
92 promote(app.db);
93 await call("/api/v1/repos", { name: "app", visibility: "public" });
94 const remote = `${base.replace("http://", `http://alice:${token}@`)}/alice/app.git`;
95 await gitOk(["clone", remote, "app"], work);
96 const dir = join(work, "app");
97 await Bun.write(join(dir, "code.txt"), "one\n");
98 await gitOk(["add", "."], dir);
99 await gitOk(["commit", "-m", "seed"], dir);
100 await gitOk(["push", "origin", "main"], dir);
101 // A branch level with main: exists, but has nothing ahead of it.
102 await gitOk(["push", "origin", "main:level"], dir);
103 // A branch with a commit ahead of main.
104 await gitOk(["checkout", "-b", "ahead"], dir);
105 await Bun.write(join(dir, "code.txt"), "one\ntwo\n");
106 await gitOk(["commit", "-am", "two"], dir);
107 await gitOk(["push", "origin", "ahead"], dir);
108 });
109
110 afterAll(async () => {
111 servers.stop();
112 app.close();
113 await rm(dataDir, { recursive: true, force: true });
114 await rm(work, { recursive: true, force: true });
115 });
116
117 test("a head equal to base is same_branch", async () => {
118 await refused({ base: "main", head: "main" }, "same_branch");
119 });
120
121 test("a head branch that does not exist is unknown_head", async () => {
122 await refused({ base: "main", head: "ghost" }, "unknown_head");
123 });
124
125 test("a head with no commit ahead of base is nothing_to_merge", async () => {
126 await refused({ base: "main", head: "level" }, "nothing_to_merge");
127 });
128
129 test("a cross-repository head with base's branch name is allowed, and no number was spent by the refusals", async () => {
130 const forked = await call("/api/v1/repos/alice/app/forks", {
131 name: "app-fork",
132 });
133 expect(forked.status).toBeLessThan(300);
134 const forkOwnerRepo = ((await forked.json()) as { fullName?: string })
135 .fullName;

Fallback names hide a missing fullName; assert it is defined instead.

136 const dir = join(work, "app");
137 const remote = `${base.replace("http://", `http://alice:${token}@`)}/alice/${
138 forkOwnerRepo?.split("/")[1] ?? "app-fork"
139 }.git`;
140 await gitOk(["checkout", "-b", "main-fork-work", "main"], dir);
141 await Bun.write(join(dir, "fork.txt"), "fork\n");
142 await gitOk(["add", "."], dir);
143 await gitOk(["commit", "-m", "fork work"], dir);
144 await gitOk(["push", remote, "main-fork-work:main", "--force"], dir);
145
146 const res = await open({
147 base: "main",
148 head: "main",
149 headRepo: forkOwnerRepo ?? "alice/app-fork",
150 });
151 expect(res.status).toBe(201);
152 const pull = (await res.json()) as { number: number; crossRepo: boolean };
153 expect(pull.crossRepo).toBe(true);
154 // Three refusals came first; none of them used a number.
155 expect(pull.number).toBe(1);

number === 1 depends on test order; make the no-number-spent check its own test or snapshot the counter before the refusals.

156 });
157 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs