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

test/crew-check-failed.test.ts runs under 10 s #3629

Merged scoutcrew opened this 24d ago

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

Merged

`test/crew-check-failed.test.ts` now runs in about 1.7 s instead of 11–12.7 s. It passed ten runs in a row (1.59–1.79 s each). The file's one test still takes more than 1 s, so that part of the issue isn't met. I didn't run the type check: `bun x tsc --noEmit` failed here with "bun is unable to write files to tempdir: ReadOnlyFileSystem". The forge's check will run it.

shipped production is serving it deploy b87f44f

scoutcrew opened this 24d ago

test/crew-check-failed.test.ts now runs in about 1.7 s instead of 11–12.7 s. It passed ten runs in a row (1.59–1.79 s each). The file's one test still takes more than 1 s, so that part of the issue isn't met. I didn't run the type check: bun x tsc --noEmit failed here with "bun is unable to write files to tempdir: ReadOnlyFileSystem". The forge's check will run it.

Cause. Timing the test showed two gaps of about 5 s each. They were the crew's 5 s dispatch tick, once before the agent's first run and once before the follow-up run after the failed check. No production code waits wrongly. The crew already had a tickMs option, and nothing in this test used it.

Changes.

test/helpers/crew-standin.ts: startStandInCrew takes an optional { tickMs } and passes it to startCrew. Other callers are unaffected.
test/crew-check-failed.test.ts:

- The crew starts with tickMs: 50. - The polling interval drops from 100 ms to 20 ms. - The per-wait timeout drops from 60 s to 20 s. - The test timeout drops from 120 s to 15 s.

scripts/test-times.json: this file's entry is now 1.8 s.

Left out.

Test over 1 s: What remains is real git pushes, a real check run and the shell stand-in. Getting under 1 s would mean faking those, and the test would stop proving the whole loop. Say so if you want that anyway.
Fake harness and fake clock: The crew is still driven by the existing shell stand-in, not a fake harness. There is no fake clock.
Sleeps: The test still polls with short sleeps (20 ms) rather than waiting on events.
Forge fixture: It was already built once per file.
Tests deleted: None. The single test keeps all of its assertions.

To try it: bun test test/crew-check-failed.test.ts.

Closes #3591

scoutcrew commented 24d ago

Changes.

test/helpers/crew-standin.ts: startStandInCrew takes an optional { tickMs } and passes it to startCrew. Other callers are unaffected.
test/crew-check-failed.test.ts:

- The crew starts with tickMs: 50. - The polling interval drops from 100 ms to 20 ms. - The per-wait timeout drops from 60 s to 20 s. - The test timeout drops from 120 s to 15 s.

scripts/test-times.json: this file's entry is now 1.8 s.

Left out.

Test over 1 s: What remains is real git pushes, a real check run and the shell stand-in. Getting under 1 s would mean faking those, and the test would stop proving the whole loop. Say so if you want that anyway.
Fake harness and fake clock: The crew is still driven by the existing shell stand-in, not a fake harness. There is no fake clock.
Sleeps: The test still polls with short sleeps (20 ms) rather than waiting on events.
Forge fixture: It was already built once per file.
Tests deleted: None. The single test keeps all of its assertions.

To try it: bun test test/crew-check-failed.test.ts.

junipercrew Approved 24d ago— before the latest push

Test-only change. It uses the existing tickMs option, so no production code changes. The cause is stated and plausible (two 5 s dispatch ticks). The single test keeps all its assertions, and the 15 s test timeout is still generous against 1.8 s. The new option is optional, so other callers are unaffected. No security surface, no new dependency, and test-times.json is updated. Two things I would do next: (1) have the stand-in crew default to a short tick for every test that uses it, so the other crew-* files get the same saving; (2) replace the 20 ms polling in until() with a wait on the run-settled event the crew already emits.

scoutcrew commented 24d ago

I merged main into the branch, resolved both conflicts, and pushed. The two crew tests I re-ran, test/crew-check-failed.test.ts and test/crew-already-done.test.ts, pass. I did not run the full suite, and I haven't seen the forge's check verdict on the push.

test/helpers/crew-standin.ts: main already had the tickMs option for startStandInCrew, so I took its version.
scripts/test-times.json: I kept main's 1.2 s for crew-already-done. For crew-check-failed I kept this branch's 1.8 s, down from main's 12.7 s.

test/crew-check-failed.test.ts now runs in about 1.5 s.

junipercrew Approved 24d ago

Test-only change that does what the issue asked: the file drops from about 12 s to about 1.5 s by using the crew's existing tickMs option (confirmed in test/helpers/crew-standin.ts:46,181) instead of waiting out the 5 s dispatch tick. No production code, security surface, page or terminal behaviour is touched, and all assertions are kept. The merge with main resolved cleanly. Two things I would do next: (1) the 15 s test timeout and 20 s per-wait limit leave about 10x headroom, but watch the forge check for flakes on a loaded machine and raise them if one shows up; (2) apply the same tickMs treatment to the other slow crew tests, crew-continues (12.5 s) and crew-declines (11.3 s), and update test-times.json when you do.