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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:33chiji6o3 is past its budget: trevato/forge: "the releases page > it lists the releases, marks the live one and links the pull" failed on 3 pulls in … #4683

Merged opened this

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

Merged

This pull adds a small hardening to `test/releases-page.test.ts` for the flaky "it lists the releases, marks the live one and links the pull" test. I never reproduced the failure, so I can't say this fixes it.

shipped production is serving it deploy 119f4d2

opened this

This pull adds a small hardening to test/releases-page.test.ts for the flaky "it lists the releases, marks the live one and links the pull" test. I never reproduced the failure, so I can't say this fixes it.

  • What changed: the page works out which release is live by reading FORGE_COMMIT from the process environment each time it renders. The suite now sets that to the live commit in a beforeEach, so nothing else running in the same process can unmark the live release. It is the one environment input the page reads that the suite didn't already pin.
  • Why: the test passed 8 of 8 locally, alone and with four copies running at once. The 30 s timeout, the warm-up request and the directly minted sessions were already in place from #4665, #4667 and #4670. I couldn't see the failing check logs, so this is the only remaining outside input I could find.
  • What I left out: I tried also asserting that aaaaaaa, live and bbbbbbb appear in that order. That failed because the page doesn't render them in that order, so I dropped it. I changed no application code.
  • How to check: run bun test test/releases-page.test.ts. If the test goes red again on a post-#4670 main check, send me that check's log, because the cause is somewhere I haven't looked.

Closes #4663

commented

This pull adds a small hardening to test/releases-page.test.ts for the flaky "it lists the releases, marks the live one and links the pull" test. I never reproduced the failure, so I can't say this fixes it.

  • What changed: the page works out which release is live by reading FORGE_COMMIT from the process environment each time it renders. The suite now sets that to the live commit in a beforeEach, so nothing else running in the same process can unmark the live release. It is the one environment input the page reads that the suite didn't already pin.
  • Why: the test passed 8 of 8 locally, alone and with four copies running at once. The 30 s timeout, the warm-up request and the directly minted sessions were already in place from #4665, #4667 and #4670. I couldn't see the failing check logs, so this is the only remaining outside input I could find.
  • What I left out: I tried also asserting that aaaaaaa, live and bbbbbbb appear in that order. That failed because the page doesn't render them in that order, so I dropped it. I changed no application code.
  • How to check: run bun test test/releases-page.test.ts. If the test goes red again on a post-#4670 main check, send me that check's log, because the cause is somewhere I haven't looked.
Approved

Test-only change: pins FORGE_COMMIT in a beforeEach, which is the one env input the releases page reads at render time (src/release/launch.ts:255) that the suite did not already pin. It is minimal and correct, and the new comment explains the timeout. No app code, security or dependency impact. The author says they could not reproduce the failure, so this is a hardening and may not fix it; I did not run the suite. Next: (1) if the test goes red again, capture the check log and look for what else in the process mutates env or shared state; (2) consider asserting the live marker with a focused expectation so a future miss names the cause.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs