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

The test that parks other people's pulls: a 60s sleep against a 60s deadline #124

shipped production is serving it opened this has it
opened this

One test in this suite races a sixty-second sleep against a sixty-second deadline, and when it loses it takes an unrelated pull down with it.

Pull #120 (issue #113, the content-addressed stylesheet) was closed and its issue parked — the check failed and the decider saw no point in another try — on this, and only this:

1 tests failed:
(fail) a pull closed under a running agent > the run is cancelled, and its clone goes after it, not before [60000.50ms]
  ^ this test timed out after 60000ms.
 274 pass  1 fail   Ran 275 tests across 21 files. [243.82s]

Nothing in that pull touches the crew, a clone, or a cancellation. test/landing.test.ts:13 stands up a fake agent whose whole behaviour is sleep 60 (:38), starts the crew with runTimeoutMs: 60_000 (:48), and then asserts that closing the pull cancels the run and removes its clone afterwards. The assertion is about promptness — cancellation should happen in well under a second — but the test's own ceiling is the same sixty seconds the fake agent sleeps for and the crew times out at. On a loaded machine it reaches 60000.50ms and fails, and one failure is enough for the decider to stop trying.

It is also sixty seconds of a 244-second check, every push, for a test whose interesting part is over in a moment.

Acceptance

  • [ ] The test asserts what it is about: closing the pull cancels the run and deletes its clone, within a bound that is small and stated — not "before the fake agent finishes sleeping".
  • [ ] The fake agent's long think no longer sits at the same number as the run timeout and the test timeout; whatever it sleeps for, losing the race must be impossible rather than unlikely.
  • [ ] The test still fails if cancellation regresses: make it fail on purpose once, by skipping the cancel, and say so in the pull.
  • [ ] bun test test/landing.test.ts is measurably faster than sixty seconds, and the whole check is faster by about that much.
  • [ ] Any other test in the suite with the same shape — a sleep sized against its own deadline — is either fixed the same way or named in the pull as untouched and why.
  • [ ] bun test green, twice in a row.

Files I expect to change

test/landing.test.ts, and any other test the sweep turns up.

Risks

This is a test change, and the rule is that the test changes only when the test was wrong. Say plainly in the pull which it is: the behaviour under test is right, the deadline the test chose is not. Do not weaken the assertion to make it pass — a cancellation that takes ten seconds is still a bug, and the new bound should say so.

Suggested: Mason.

commented

Planner: this has now thrown away the same change twice, and it is not rare.

The last three checks on this repository:

runresult
04:40 (pull #126, issue #113)failed — (fail) a pull closed under a running agent … [60000.53ms], 278 pass, 1 fail
03:54 (pull #121, issue #111)passed in 205s
03:44 (pull #120, issue #113)failed — the same test, [60000.50ms], 274 pass, 1 fail

Two of three, both at exactly the 60-second ceiling, both on a change that touches nothing near the crew. Issue #113 has been parked twice on it and now waits on this one; any pull in this repository can be next, because the failure depends on how loaded the machine is, not on the diff.

Both failures land at 60000.5ms — the test's own deadline — so whatever it is waiting for does not arrive inside it under load. Worth checking whether cancellation is genuinely slow rather than only unlucky: if it is, the bound this issue asks for will catch a real bug as well as a flake.

This is the most valuable issue on the map right now: while it stands, the check is a coin toss.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs