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

test/crew-remotes.test.ts runs in under 10 s, every test under 1 s, proving what it proves today #10234

Merged opened this

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

Merged

`test/crew-remotes.test.ts` now runs in about 2.4 s, down from 11.75 s. Each test is under 1 s, and it checks the same things it checked before.

shipped production is serving it deploy f21b126

opened this

test/crew-remotes.test.ts now runs in about 2.4 s, down from 11.75 s. Each test is under 1 s, and it checks the same things it checked before.

Before and after (bun test test/crew-remotes.test.ts in the crew sandbox; times per test are from --reporter=junit):

filetests
before (main 829eb7f6c)11.75 s1 test, 11.5 s of it (1.27 s once the clock fix below is applied)
after, 3 runs2.45 / 2.42 / 2.43 scarry the forge's public URL… 0.50–0.56 s; are set right again… 0.50–0.59 s

Why it was slow: the crew starts queued runs on a 5 s clock by default, and the test waited on that clock twice: once for the first run, once for the run after the comment. startCrew already takes tickMs, so the test now passes tickMs: 20. Its poll sleeps 20 ms instead of 100, and its deadline is 4 s, under the 5 s test timeout, so a run that never finishes reports "agent run did not settle" instead of a timeout.

The split: the one test did two agent runs and checked two separate things, so it is now two tests. The second reuses the issue and the clone the first one leaves; Bun runs tests in a file in order. Each test starts with a one-line statement of what it proves:

  • A fresh clone's fetch and push remotes are the forge's public URL, and a plain git push over them opens the pull.
  • A kept clone's stale remotes are reset to the forge's public URL on every visit, and the next push updates the pull.

Nothing was deleted or skipped. The file has 2 tests instead of 1 and 14 assertions instead of 13 (the second test first checks that the first run succeeded).

Each test fails without the code it guards (src/crew/runner.ts, changed by hand and then restored):

  • Removing the push-remote line for a fresh clone (lines 1917–1920) fails the first test: Expected: ".../bot/garden.git", Received: ".../alice/garden.git".
  • Setting the remotes only when the clone is first made (dropping the reset at 1897–1901 and making 1916–1920 run only on a new clone) fails the second test: Expected: ".../alice/garden.git", Received: "http://localhost:1/stale.git".

Something to look at: removing only the reset on every visit (1897–1901) fails neither the old test nor the new one. The second visit doesn't keep the clone as it was left, so it falls through to the set at 1916, which sets the remotes again anyway. Only a resumed run, whose clone is kept and is on its branch, depends on 1897–1901, and this file doesn't reach that case. I left it alone, since this issue covers the test file only.

bun scripts/gate.ts, bun x tsc --noEmit and the 13 files scripts/fast.ts names (170 pass, 0 fail) all pass. This sandbox has no keep_plan tool, so there is no plan on the issue.

🤖 Generated with Claude Code

Closes #10227

commented

test/crew-remotes.test.ts now runs in about 2.4 s, down from 11.75 s. Each test is under 1 s, and it checks the same things it checked before.

Before and after (bun test test/crew-remotes.test.ts in the crew sandbox; times per test are from --reporter=junit):

filetests
before (main 829eb7f6c)11.75 s1 test, 11.5 s of it (1.27 s once the clock fix below is applied)
after, 3 runs2.45 / 2.42 / 2.43 scarry the forge's public URL… 0.50–0.56 s; are set right again… 0.50–0.59 s

Why it was slow: the crew starts queued runs on a 5 s clock by default, and the test waited on that clock twice: once for the first run, once for the run after the comment. startCrew already takes tickMs, so the test now passes tickMs: 20. Its poll sleeps 20 ms instead of 100, and its deadline is 4 s, under the 5 s test timeout, so a run that never finishes reports "agent run did not settle" instead of a timeout.

The split: the one test did two agent runs and checked two separate things, so it is now two tests. The second reuses the issue and the clone the first one leaves; Bun runs tests in a file in order. Each test starts with a one-line statement of what it proves:

  • A fresh clone's fetch and push remotes are the forge's public URL, and a plain git push over them opens the pull.
  • A kept clone's stale remotes are reset to the forge's public URL on every visit, and the next push updates the pull.

Nothing was deleted or skipped. The file has 2 tests instead of 1 and 14 assertions instead of 13 (the second test first checks that the first run succeeded).

Each test fails without the code it guards (src/crew/runner.ts, changed by hand and then restored):

  • Removing the push-remote line for a fresh clone (lines 1917–1920) fails the first test: Expected: ".../bot/garden.git", Received: ".../alice/garden.git".
  • Setting the remotes only when the clone is first made (dropping the reset at 1897–1901 and making 1916–1920 run only on a new clone) fails the second test: Expected: ".../alice/garden.git", Received: "http://localhost:1/stale.git".

Something to look at: removing only the reset on every visit (1897–1901) fails neither the old test nor the new one. The second visit doesn't keep the clone as it was left, so it falls through to the set at 1916, which sets the remotes again anyway. Only a resumed run, whose clone is kept and is on its branch, depends on 1897–1901, and this file doesn't reach that case. I left it alone, since this issue covers the test file only.

bun scripts/gate.ts, bun x tsc --noEmit and the 13 files scripts/fast.ts names (170 pass, 0 fail) all pass. This sandbox has no keep_plan tool, so there is no plan on the issue.

🤖 Generated with Claude Code

Approved

Test-only change; no user-facing surface, so the preview has nothing to exercise. The cause is right: the crew pump ticked on a 5 s clock and the test waited on it twice. tickMs: 20 fixes that, and the poll sleep drops to 20 ms. Each of the two tests opens with a one-line invariant. 2.4 s against 11.75 s is quoted with before and after.

Two things I would do next:

  1. The second test depends on state the first leaves (issue, firstRun, the clone), so it breaks under a randomized order or a --only run. Either fold the setup into a beforeAll or accept the order and say so in a comment.
  2. The 4 s settle deadline sits under the default 5 s test timeout. On a loaded gate a slow first run reports "agent run did not settle" rather than passing. Give each test an explicit timeout of about 8 s, so the limit stays well above the 0.5 s measured.

Mason also found that nothing in this file covers the reset on every visit (runner.ts 1897-1901). That is worth its own issue.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs