Public: anyone can read this; the crew can change it.
Flaky: test/reviews.test.ts #2132
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2106-flaky-test-reviews-test-ts → main 2 commits fork 2 threads · 2 unresolved
2 files
changed +35
−6
scripts/gate.ts −1
| @@ -63,7 +63,6 @@ export const EXCLUDED: readonly Exclusion[] = [ | ||
| 63 | 63 | reason: "not yet admitted", |
| 64 | 64 | issue: 261, |
| 65 | 65 | }, |
| 66 | { file: "test/reviews.test.ts", reason: "not yet admitted", issue: 261 }, | |
| 67 | 66 | { file: "test/runs-lease.test.ts", reason: "not yet admitted", issue: 261 }, |
| 68 | 67 | { file: "test/scaffold.test.ts", reason: "not yet admitted", issue: 261 }, |
| 69 | 68 | { file: "test/search.test.ts", reason: "not yet admitted", issue: 261 }, |
test/reviews.test.ts +35 −5
| @@ -49,6 +49,23 @@ describe("review comments on diff lines", () => { | ||
| 49 | 49 | redirect: "manual", |
| 50 | 50 | }); |
| 51 | 51 | |
| 52 | // Each test that reads or rewrites a branch opens its own pull on its own | |
| 53 | // branch, so no test depends on what another left behind. | |
| 54 | async function openPr(branch: string): Promise<string> { | |
| 55 | const dir = join(work, "app"); | |
| 56 | await gitOk(["checkout", "-B", branch, "main"], dir); | |
| 57 | await Bun.write(join(dir, "code.txt"), "alpha\nBETA\ngamma\ndelta\n"); | |
| 58 | await gitOk(["commit", "-am", "change"], dir); | |
| 59 | await gitOk(["push", "origin", branch], dir); | |
| 60 | const res = await post("/rev/app/pulls/new", { | |
| 61 | base: "main", | |
| 62 | head: branch, | |
| 63 | title: "PR", | |
| 64 | body: "", | |
| 65 | }); | |
| 66 | return res.headers.get("location")!.replace(/\/files$/, ""); | |
| 67 | } | |
| 68 | ||
| 52 | 69 | beforeAll(async () => { |
| 53 | 70 | dataDir = await mkdtemp(join(tmpdir(), "forge-rev-")); |
| 54 | 71 | work = await mkdtemp(join(tmpdir(), "forge-rev-work-")); |
| @@ -139,8 +156,9 @@ describe("review comments on diff lines", () => { | ||
| 139 | 156 | }); |
| 140 | 157 | |
| 141 | 158 | test("a posted review comment appears inline, anchored to its line", async () => { |
| 159 | const pull = await openPr("topic-a"); | |
| 142 | 160 | // "BETA" is the new-side content of line 2; anchor to it. |
| 143 | const res = await post("/rev/app/pulls/1/review", { | |
| 161 | const res = await post(`${pull}/review`, { | |
| 144 | 162 | path: "code.txt", |
| 145 | 163 | side: "new", |
| 146 | 164 | line: "2", |
| @@ -149,29 +167,41 @@ describe("review comments on diff lines", () => { | ||
| 149 | 167 | }); |
| 150 | 168 | expect(res.status).toBe(303); |
| 151 | 169 | const pr = await ( |
| 152 | await fetch(`${base}/rev/app/pulls/1/files`, { headers: { cookie } }) | |
| 170 | await fetch(`${base}${pull}/files`, { headers: { cookie } }) | |
| 153 | 171 | ).text(); |
| 154 | 172 | expect(pr).toContain("why so loud?"); |
| 155 | 173 | expect(pr).not.toContain("outdated"); // hash matches the current line |
| 156 | 174 | }); |
| 157 | 175 | |
| 158 | 176 | test("a comment whose line content changed is marked outdated, not misplaced", async () => { |
| 177 | const pull = await openPr("topic-b"); | |
| 159 | 178 | // Anchor a comment to line 3 ("gamma"), then rewrite that line on the |
| 160 | 179 | // branch. The server hashed the content it saw, so the comment must now |
| 161 | 180 | // read as outdated — never silently re-land on the new text. |
| 162 | const posted = await post("/rev/app/pulls/1/review", { | |
| 181 | const kept = await post(`${pull}/review`, { | |
| 182 | path: "code.txt", | |
| 183 | side: "new", | |
| 184 | line: "2", | |
| 185 | body: "why so loud?", | |
| 186 | }); | |
| 187 | expect(kept.status).toBe(303); | |
| 188 | const posted = await post(`${pull}/review`, { | |
| 163 | 189 | path: "code.txt", |
| 164 | 190 | side: "new", |
| 165 | 191 | line: "3", |
| 166 | 192 | body: "stale anchor", |
| 167 | 193 | }); |
| 168 | 194 | expect(posted.status).toBe(303); |
| 195 | const before = await ( | |
| 196 | await fetch(`${base}${pull}/files`, { headers: { cookie } }) | |
| 197 | ).text(); | |
| 198 | expect(before).not.toContain("outdated"); | |
outdated
Pushes |
||
| 169 | 199 | const dir = join(work, "app"); |
| 170 | 200 | await Bun.write(join(dir, "code.txt"), "alpha\nBETA\nGAMMA\ndelta\n"); |
| 171 | 201 | await gitOk(["commit", "-am", "rewrite line 3"], dir); |
| 172 | await gitOk(["push", "origin", "feature"], dir); | |
| 202 | await gitOk(["push", "origin", "topic-b"], dir); | |
| 173 | 203 | const pr = await ( |
| 174 | await fetch(`${base}/rev/app/pulls/1/files`, { headers: { cookie } }) | |
| 204 | await fetch(`${base}${pull}/files`, { headers: { cookie } }) | |
| 175 | 205 | ).text(); |
| 176 | 206 | expect(pr).toContain("stale anchor"); |
| 177 | 207 | expect(pr).toContain("outdated"); // content hash no longer matches |
new above
offline
The title is the branch name, so
outdatedis always in the page HTML. That makestoContain("outdated")pass vacuously. Use a neutral title.