trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

test/app-domains.test.ts: "an app gets its own domain" is green on a loaded runner and rejoins the gate #1167

Merged masoncrew opened this

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

ConversationFiles
Merged

The root cause was already fixed on main by 7fa3b90, and `test/app-domains.test.ts` is already in `GATE`. This pull changes only the test file: when the deploy fails, the error now says why. I couldn't run the file here (see "How to try it"), so the forge check on this push is the first run of the change.

shipped production is serving it deploy da65c36

masoncrew opened this

The root cause was already fixed on main by 7fa3b90, and test/app-domains.test.ts is already in GATE. This pull changes only the test file: when the deploy fails, the error now says why. I couldn't run the file here (see "How to try it"), so the forge check on this push is the first run of the change.

Cause. Before 7fa3b90, the checks failed in beforeAll at test/app-domains.test.ts:349. The deploy ended as failed after about 32 s:

348 |     const run = await settled((r) => r.env === "production");
349 |     expect(run.state).toBe("ok");
Expected: "ok"
Received: "failed"
(fail) an app gets its own domain > (unnamed) [32127.99ms]

That is from check runs 01m4q4zmjm3s7besjftah59gde and 01m4r8savpzr4441zwne1d76fb (31843.98 ms). The wait was fine: it ends when the deploy run itself is ok or failed. The deploy failed because a forge started inside a check's cell wrapped each app in another cell nested inside it, and those apps never answered. 7fa3b90 sets FORGE_CELLS=off for the check's child processes. In every full check since then (11 runs, from 01m4rg0ayhb3dvxv6d36jrhstk to 01m4s01jpmepm958xn045cnp03), all 9 "an app gets its own domain" cases pass, including runs that went red on other files.

What changed in test/app-domains.test.ts.

  • Failed deploy: the setup step now throws the run's id, state, summary and last 20 log lines, instead of only "failed". The original expect(run.state).toBe("ok") is still there.
  • Deploy that never finishes: until and settled are now one settled. If time runs out, it names where the latest production deploy stood (or says none was queued), instead of "did not happen in time". The 60 s limit and the 120 s hook timeout are unchanged.
  • Cleanup after a broken setup: afterAll now stops only what setup actually started. Before, a broken setup also threw runtime.stop on an undefined value, so one failure showed up as two.

No assertion is weakened, no timeout is raised, and scripts/gate.ts is untouched.

How to try it. bun test test/app-domains.test.ts. I couldn't run it three times as the issue asks: this sandbox refuses to let the forge open its internal unix socket (EPERM: operation not permitted, listen '…/forge-….sock'), so setup can't run here at all. The forge's check on this push is the real run. For the parts of the file that do run here:

  • bun x tsc --noEmit exits 0.
  • bun test test/check-list.test.ts gives 8 pass, 0 fail.
  • bun test test/app-domains.test.ts passes the 4 checkDomain cases. The 9 cases that need the forge running can't start here, and the setup failure is now reported once (EPERM) instead of twice.

🤖 Generated with Claude Code

Closes #1045

junipercrew Approved

Test-only change; no page, terminal or security surface touched, so nothing to see in the preview. A failed deploy now reports its state, summary and log tail instead of "did not happen in time", and afterAll no longer throws a second error when setup failed part-way. Two things I would do next: (1) the issue is about passing on a loaded runner, and this pull makes the failure legible rather than less likely, so if it still flakes, raise the 60 s settle window or wait on the run event instead of polling; (2) tidy the nits in the comments.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs