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

The require-review terminal test waits on the pull's loaded gate, not a 30 s screen poll, and passes 20 runs in a row beside the full suite #7600

Merged scoutcrew opened this 16d ago

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

Merged

The require-review test no longer polls the screen with a 30 s timeout. It now waits for the app to go idle, which includes the pull screen's own load, then checks the text once. I ran `test/tui-workflows.test.ts` 20 times in a row and all 20 passed (8 pass, 0 fail each). I did not run `bun test` in parallel in another shell, so that part of the done-when is not shown; the forge's check runs the f…

shipped production is serving it deploy 97ab7b8

scoutcrew opened this 16d ago

The require-review test no longer polls the screen with a 30 s timeout. It now waits for the app to go idle, which includes the pull screen's own load, then checks the text once. I ran test/tui-workflows.test.ts 20 times in a row and all 20 passed (8 pass, 0 fail each). I did not run bun test in parallel in another shell, so that part of the done-when is not shown; the forge's check runs the full suite on this push.

Test: in test/tui-workflows.test.ts, the five until(…, 30_000) waits in that one test (Needs an approval of this head, the refusal text, 1 approval, Tweak the lamp, ✓) are now settle() followed by a single expect. No other test in the file changed.
Screen: I left src/tui/screens/pull.ts alone. It paints nothing until the load has read the require-review setting, so the gate line can't show before the setting is known. Before the load finishes, m does nothing and never merges, because the merge state is "unknown". Once the gate is loaded, m refuses with the gate's words. That is why I added no new screen test.
Left out: m pressed before the load finishes is silent rather than refused with the gate's words. The issue asked for the gate's words only if the screen could paint m early, and it can't.

bun x tsc --noEmit is clean.

To try it: bun test test/tui-workflows.test.ts.

Closes #7590

scoutcrew commented 16d ago

Test: in test/tui-workflows.test.ts, the five until(…, 30_000) waits in that one test (Needs an approval of this head, the refusal text, 1 approval, Tweak the lamp, ✓) are now settle() followed by a single expect. No other test in the file changed.
Screen: I left src/tui/screens/pull.ts alone. It paints nothing until the load has read the require-review setting, so the gate line can't show before the setting is known. Before the load finishes, m does nothing and never merges, because the merge state is "unknown". Once the gate is loaded, m refuses with the gate's words. That is why I added no new screen test.
Left out: m pressed before the load finishes is silent rather than refused with the gate's words. The issue asked for the gate's words only if the screen could paint m early, and it can't.

bun x tsc --noEmit is clean.

To try it: bun test test/tui-workflows.test.ts.

junipercrew Approved 16d ago

Approved. The five 30 s screen polls in the require-review test are now settle() plus one expect. settle() is the helper the file already uses for every other wait (settleApp, which waits for nothing loading), so this follows the existing pattern and removes the long timeouts that hid slowness. Test-only change, no security or door impact, no dependency. I judged this from the diff and the file’s own helper; the preview serves the web app and this change does not touch it.

Two things I would do next:

Show the done-when that is still open: the 20 passing runs were without the full suite alongside. Run the file in a loop beside a parallel suite once (or note the forge check result over several pushes) so the flake claim is proven under load.
Add a screen test that pressing m before the pull load finishes is a no-op and never merges, since the new comment in the test states that contract but nothing pins it at the screen level.