Public: anyone can read this; the crew can change it.
test/apps-adversarial-teardown.test.ts runs under 10 s #3571
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3561-test-apps-adversarial-teardown-test-ts-r → main 2 commits fork
3 files
changed +70
−41
scripts/test-times.json +1 −1
| @@ -1,7 +1,7 @@ | ||
| 1 | 1 | { |
| 2 | 2 | "test/apps-adversarial-grants.test.ts": 28.3, |
| 3 | 3 | "test/apps-adversarial-rules.test.ts": 29.7, |
| 4 | "test/apps-adversarial-teardown.test.ts": 36.4, | |
| 4 | "test/apps-adversarial-teardown.test.ts": 3, | |
| 5 | 5 | "test/apps-adversarial-test-step.test.ts": 33.5, |
| 6 | 6 | "test/apps-adversarial.test.ts": 18, |
| 7 | 7 | "test/apps-busy.test.ts": 26.4, |
src/apps/runtime.ts +1 −1
| @@ -360,7 +360,7 @@ const TOUCH_REASONS: readonly string[] = [ | ||
| 360 | 360 | |
| 361 | 361 | const RESTART_WINDOW_MS = 10 * 60_000; |
| 362 | 362 | const RESTART_MAX = 3; |
| 363 | const RESTART_BACKOFF_MS = [2_000, 10_000, 30_000]; | |
| 363 | export const RESTART_BACKOFF_MS = [2_000, 10_000, 30_000]; | |
| 364 | 364 | |
| 365 | 365 | const runtimes = new WeakMap<App, AppsRuntime>(); |
| 366 | 366 |
test/apps-adversarial-teardown.test.ts +68 −39
| @@ -5,13 +5,13 @@ | ||
| 5 | 5 | * (#2038); each file of the split boots its own forge and runtime. |
| 6 | 6 | */ |
| 7 | 7 | |
| 8 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 8 | import { afterAll, beforeAll, describe, expect, spyOn, test } from "bun:test"; | |
| 9 | 9 | import { existsSync } from "node:fs"; |
| 10 | 10 | import { mkdtemp, rm } from "node:fs/promises"; |
| 11 | 11 | import { tmpdir } from "node:os"; |
| 12 | 12 | import { join } from "node:path"; |
| 13 | 13 | import { createApp, type App } from "../src/app.ts"; |
| 14 | import { AppsRuntime } from "../src/apps/runtime.ts"; | |
| 14 | import { AppsRuntime, RESTART_BACKOFF_MS } from "../src/apps/runtime.ts"; | |
| 15 | 15 | import { envDir } from "../src/apps/data.ts"; |
| 16 | 16 | import { newApp } from "../src/actions/apps.ts"; |
| 17 | 17 | import { mergePull, openPull } from "../src/actions/pulls.ts"; |
| @@ -55,11 +55,32 @@ describe("apps adversarial: teardown races", () => { | ||
| 55 | 55 | run.state === "cancelled") |
| 56 | 56 | ) |
| 57 | 57 | return run; |
| 58 | await Bun.sleep(50); | |
| 58 | await Bun.sleep(10); | |
| 59 | 59 | } |
| 60 | 60 | throw new Error("run did not settle in time"); |
| 61 | 61 | } |
| 62 | 62 | |
| 63 | /** Every deploy run of one environment exists and is in a final state. */ | |
| 64 | async function quiet(repoId: string, env: string, timeoutMs = 60_000) { | |
| 65 | const deadline = Date.now() + timeoutMs; | |
| 66 | while (Date.now() < deadline) { | |
| 67 | const runs = listRuns(app.db, repoId, { env, kind: "deploy", limit: 50 }); | |
| 68 | if ( | |
| 69 | runs.length > 0 && | |
| 70 | runs.every((r) => ["ok", "failed", "cancelled"].includes(r.state)) | |
| 71 | ) | |
| 72 | return; | |
| 73 | await Bun.sleep(10); | |
| 74 | } | |
| 75 | throw new Error(`deploys of ${env} did not settle in time`); | |
| 76 | } | |
| 77 | ||
| 78 | async function gone(path: string, timeoutMs = 10_000): Promise<void> { | |
| 79 | const deadline = Date.now() + timeoutMs; | |
| 80 | while (existsSync(path) && Date.now() < deadline) await Bun.sleep(10); | |
| 81 | expect(existsSync(path)).toBe(false); | |
| 82 | } | |
| 83 | ||
| 63 | 84 | beforeAll(async () => { |
| 64 | 85 | dataDir = await mkdtemp(join(tmpdir(), "forge-adv-")); |
| 65 | 86 | work = await mkdtemp(join(tmpdir(), "forge-adv-work-")); |
| @@ -120,35 +141,48 @@ describe("apps adversarial: teardown races", () => { | ||
| 120 | 141 | |
| 121 | 142 | // Wait for the preview to actually come up (live, not just queued). |
| 122 | 143 | await settled(repo.id, (r) => r.env === `pull-${pull.number}`); |
| 123 | expect((await viaHost(`resurrect--${pull.number}.${DOMAIN}`)).status).toBe( | |
| 124 | 200, | |
| 125 | ); | |
| 144 | // Arm a watch on the first restart backoff (RESTART_BACKOFF_MS[0], the | |
| 145 | // delay of a fresh environment's first crash): a timer of that length set | |
| 146 | // after the close is a restart being scheduled. | |
| 147 | const armed: unknown[] = []; | |
| 148 | const realSetTimeout = globalThis.setTimeout; | |
| 149 | const spy = spyOn(globalThis, "setTimeout").mockImplementation((( | |
| 150 | fn: () => void, | |
| 151 | ms?: number, | |
| 152 | ...rest: unknown[] | |
| 153 | ) => { | |
| 154 | if (ms === RESTART_BACKOFF_MS[0]) armed.push(fn); | |
| 155 | return (realSetTimeout as (...a: unknown[]) => unknown)(fn, ms, ...rest); | |
| 156 | }) as unknown as typeof setTimeout); | |
| 157 | try { | |
| 158 | expect( | |
| 159 | (await viaHost(`resurrect--${pull.number}.${DOMAIN}`)).status, | |
| 160 | ).toBe(200); | |
| 126 | 161 | |
| 127 | const closed = await mergePull( | |
| 128 | app, | |
| 129 | alice, | |
| 130 | repo, | |
| 131 | getPull(app.db, repo.id, pull.number)!, | |
| 132 | "merge", | |
| 133 | ); | |
| 134 | expect(closed.ok).toBe(true); | |
| 162 | const closed = await mergePull( | |
| 163 | app, | |
| 164 | alice, | |
| 165 | repo, | |
| 166 | getPull(app.db, repo.id, pull.number)!, | |
| 167 | "merge", | |
| 168 | ); | |
| 169 | expect(closed.ok).toBe(true); | |
| 135 | 170 | |
| 136 | // The teardown itself must remove the directory promptly. | |
| 137 | const goneDeadline = Date.now() + 10_000; | |
| 138 | while ( | |
| 139 | existsSync(envDir(dataDir, record.id, `pull-${pull.number}`)) && | |
| 140 | Date.now() < goneDeadline | |
| 141 | ) | |
| 142 | await Bun.sleep(50); | |
| 143 | expect(existsSync(envDir(dataDir, record.id, `pull-${pull.number}`))).toBe( | |
| 144 | false, | |
| 145 | ); | |
| 171 | // The teardown removes the directory last, after the process has been | |
| 172 | // stopped, so the directory going is the stop having settled. | |
| 173 | await gone(envDir(dataDir, record.id, `pull-${pull.number}`)); | |
| 146 | 174 | |
| 147 | // The bug: `watch()`'s exit handler mistook the deliberate stop for a | |
| 148 | // crash and scheduled a restart ~2s later (RESTART_BACKOFF_MS[0]). Wait | |
| 149 | // well past that window and confirm nothing came back — no directory, | |
| 150 | // no live process answering the preview's host, no new "restart" run. | |
| 151 | await Bun.sleep(3_500); | |
| 175 | // The bug: `watch()`'s exit handler mistook the deliberate stop for a | |
| 176 | // crash and scheduled a restart ~2s later. The handler runs as a | |
| 177 | // continuation of the exit, so one turn of the loop lets it run; the | |
| 178 | // restart timer is then observed being armed (or not), no clock waited. | |
| 179 | await new Promise((r) => setImmediate(r)); | |
| 180 | expect(armed).toEqual([]); | |
| 181 | } finally { | |
| 182 | spy.mockRestore(); | |
| 183 | } | |
| 184 | // And nothing came back — no directory, no live process answering the | |
| 185 | // preview's host, no new "restart" run. | |
| 152 | 186 | expect(existsSync(envDir(dataDir, record.id, `pull-${pull.number}`))).toBe( |
| 153 | 187 | false, |
| 154 | 188 | ); |
| @@ -199,16 +233,11 @@ describe("apps adversarial: teardown races", () => { | ||
| 199 | 233 | ); |
| 200 | 234 | expect(closedQuickly.ok).toBe(true); |
| 201 | 235 | |
| 202 | const deadline = Date.now() + 15_000; | |
| 203 | while ( | |
| 204 | existsSync(envDir(dataDir, record.id, `pull-${pull.number}`)) && | |
| 205 | Date.now() < deadline | |
| 206 | ) | |
| 207 | await Bun.sleep(50); | |
| 208 | expect(existsSync(envDir(dataDir, record.id, `pull-${pull.number}`))).toBe( | |
| 209 | false, | |
| 210 | ); | |
| 211 | await Bun.sleep(3_500); | |
| 236 | await gone(envDir(dataDir, record.id, `pull-${pull.number}`)); | |
| 237 | // The in-flight deploy must not put anything back as it winds down: wait | |
| 238 | // for every deploy run of this pull to reach a final state (and for one | |
| 239 | // to exist), rather than for the clock. | |
| 240 | await quiet(repo.id, `pull-${pull.number}`); | |
| 212 | 241 | expect(existsSync(envDir(dataDir, record.id, `pull-${pull.number}`))).toBe( |
| 213 | 242 | false, |
| 214 | 243 | ); |
new above
offline