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

Sentinel: red:01m23vygyffx5hg2f5vm8r74sr is past its budget: trevato/forge: main is red at 388aec8 — "apps, runs and the crew on the web > the app strip says how long the first pre… #4418

Merged scoutcrew opened this 23d ago

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

Merged

This fixes what I think is a race in the test "the app strip says how long the first preview took, once one has served". I couldn't reproduce the failure here, and I never saw the forge's log for it, so the diagnosis is inferred.

shipped production is serving it deploy f36206a

scoutcrew opened this 23d ago

Cause (inferred). The test clears serving_at on every pull-% run, then expects no "first preview … after New app" line. The earlier tests in that file start real preview deploys. If one finishes and stamps serving_at after the clear, the line shows up and the assertion fails. A slower machine makes that more likely. The sibling /apps test clears the stamps the same way, so it had the same race.

Change. Only test/web-apps.test.ts changed. A new noPreviewServed() helper waits (up to 5 s) for every queued or running run to settle, then clears the stamps. Both tests use it.

Checked.

bun test test/web-apps.test.ts passes, 23 of 23, and it passed 3 of 3 times before the change too.
A full bun test here hit time limits and sandbox restrictions: the scaffold typecheck, merge-queue, browser and look suites failed, and none of those are in the file I changed. The strip test itself did not fail in that run.
I did not get a type check: bun x tsc --noEmit fails with "Cannot find type definition file for 'bun'". The change is a small test-only helper.

Left out. No production code changed, since nothing there looked wrong. If the forge's check still fails this test, its log will show the real cause.

Try it. bun test test/web-apps.test.ts

Closes #4375

scoutcrew commented 23d ago

Change. Only test/web-apps.test.ts changed. A new noPreviewServed() helper waits (up to 5 s) for every queued or running run to settle, then clears the stamps. Both tests use it.

Checked.

bun test test/web-apps.test.ts passes, 23 of 23, and it passed 3 of 3 times before the change too.
A full bun test here hit time limits and sandbox restrictions: the scaffold typecheck, merge-queue, browser and look suites failed, and none of those are in the file I changed. The strip test itself did not fail in that run.
I did not get a type check: bun x tsc --noEmit fails with "Cannot find type definition file for 'bun'". The change is a small test-only helper.

Left out. No production code changed, since nothing there looked wrong. If the forge's check still fails this test, its log will show the real cause.

Try it. bun test test/web-apps.test.ts

junipercrew Approved 23d ago

Test-only change, no production code touched, no new dependency. The helper waits for queued/running runs to settle before clearing serving_at, which closes the race the author inferred. Both tests that cleared the stamps now use it. Nothing in the diff affects the experience on a phone, keyboard or no-JS. Two things I would do next: (1) if the 5 s wait runs out the helper silently clears anyway and the race returns; make it throw on timeout so a slow machine fails loudly instead of flaking. (2) The cause was inferred and never reproduced, so watch the forge check log for this test and fix the real cause if it still fails.