Skip to content

test: replace the remaining wall-clock bounds with time limits and ordering - #24

Closed
MarcoDotIO wants to merge 5 commits into
mainfrom
test/no-wall-clock-bounds
Closed

MarcoDotIO wants to merge 5 commits into
mainfrom
test/no-wall-clock-bounds

Conversation

@MarcoDotIO

Copy link
Copy Markdown
Owner

Follow-up to #22. With ~3,150 Swift Testing tests running in parallel on the macOS runner, the cooperative pool saturates and runs stall for 5–8 s (CI run 36644330107). Upper bounds on elapsed time fail spuriously under that load. This PR removes the bounds that #22 left and adds one-minute time limits. Each test still fails, instead of hanging, when the code it guards regresses.

Stacked on #22. This branch starts from test/event-driven-test-waits, so until #22 merges the diff also shows its two commits. The three commits below are this PR's.

Changes

Test Old bound Now
GatewayDeviceAuthOffActorTests.issuedTokenPersistenceNeverBlocksTheChannelActor < 3 s Ordering. The lock is released only after the actor answers, so the token can reach disk only if the actor answered while the write was pending. A write on the actor fails with SQLITE_BUSY after 30 s and the token never lands. A time limit alone would miss this, because 30 s < 1 min. A new DEBUG hook, _test_setDeviceTokenPersistenceStartedHandler, replaces the 200 ms sleep that waited for hello-ok to reach the write.
FileTransferNodeCommandsTests.file fetch refuses a FIFO promptly < 2 s Time limit. A blocking open(2) is a syscall, so the cancellation handler opens the FIFO's write end once to release it.
AsyncTimeoutRaceTests.operationThatIgnoresCancellationStillTimesOut < 5 s Time limit. The operation parks on a gate that the test opens on cancellation, instead of a never-resumed continuation that a loser-joining race would wait on forever.
ChatLinkPreviewTests.total deadline can fire before the session starts < 1 s Time limit. URLSession's own timeouts move to 3600 s, so only the fetcher's 0 s deadline can end the fetch. The fetch is awaited through AsyncTimeout.withTimeout(seconds: 0) because a deadline lost before start leaves it unresumed even on cancellation.
gatewayCoreWaitUntil (56 call sites, 8 suites) 10 s / 15 s Polls until the condition holds or the test is cancelled. Every calling suite gets .timeLimit(.minutes(1)). On cancellation it records GatewayCoreWaitTimeout as an issue before throwing. Swift Testing drops errors thrown after a time-limit cancellation, and the issue says which wait hung.
ChatViewModelSessionActionTests wait helpers 15 s Same approach. waitForForkStart awaits the gate stream directly, and the suite gets a time limit.

Verification

  • swift test on macOS: 3,150 tests in 341 suites passed. Scripts/lint-swift.sh: 0 violations.
  • Pool starvation (scratch copies that spawn activeProcessorCount * 2 blockers of Task.detached(priority: Task.currentPriority) { usleep(6_000_000) } at the critical point): all four new tests pass after about 12 s. The old AsyncTimeout and link-preview versions fail their bounds.
  • Regressions (scratch source edits, reverted). Each new test fails on the time limit and the run ends at about 60 s without hanging:
    • device-auth write moved back onto the actor. Instrumented, the actor answered after 30.35 s and the token was never stored.
    • O_NONBLOCK removed from the file-fetch open.
    • AsyncTimeout rewritten as a task group that joins the loser.
    • link preview: the deadline never aborts; separately, an abort that lands before start is lost.
    • gatewayCoreWaitUntil { false } and eventually { false } end on the time limit. The gateway wait reports Timeout waiting for: <label>.

Remaining

waitUntil in TestAsyncHelpers.swift (15 s default, about 760 call sites) still uses a wall-clock deadline. It was out of scope here.

🤖 Generated with Claude Code

MarcoDotIO and others added 5 commits September 30, 2026 10:14
anotherAgentsRunBeforeTheAckNeverHijacksTheIntent and
preAckEventsOfTheAckedRunAreReplayed failed on main (CI run 36644330107,
xcode-27 runner) with "Caught error: CancellationError()" after ~8 s; the
same tree passed on PR #20. waitForSend polled the requester every 5 ms and
threw once a 5 s ContinuousClock deadline passed. With ~3,150 Swift Testing
tests saturating the cooperative pool, the host.send task (and the poller
itself) did not run for more than 5 s, so the deadline expired although
nothing in GatewayOpenClawIntentHost.send is slow. Blocking every cooperative
thread for 6 s reproduces the failure locally.

- OpenClawAppIntentsRunMatchingTests: HeldChatSendRequester yields each
  chat.send's params to an AsyncStream the moment the request arrives, and
  waitForSend awaits that stream. Assertions are unchanged.
- GatewayNetworkConnectionTransportTests: the loopback NWListener start waits
  on stateUpdateHandler (ready/failed/cancelled) instead of polling
  listener.state against the same 5 s deadline, which also had no final
  re-check after a late wake-up.
- WatchNodeClientTests: eventually() drops its 5 s deadline. Its conditions
  read production client state that has no change hook, so it still polls,
  but slowness no longer fails it.

Each suite gains .timeLimit(.minutes(1)) for hang protection. Every wait
ends on cancellation, so a time-limit overrun reports "Time limit was
exceeded" instead of hanging.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
keepalivePingIsBoundedWhenNoPongArrives and
handshakeTimeoutOptionBoundsTheWholeHandshake failed on PR #22's macOS job
(Xcode 27) with `ContinuousClock.now - start < .seconds(5)` at ~5.7 s. A
trivial test in the same window took 5.3 s, so the runner stalled; the
timeouts under test were 50 ms and 100 ms.

- keepalive ping: the socket never pongs, so the thrown URLError already
  proves the ping deadline ended the wait; a time limit catches a hang.
- handshake option: the fallback budget is raised to an hour through
  _test_setConnectTimeoutSeconds, so a channel that ignored the 100 ms option
  would trip the time limit instead of finishing at the 30 s default.
  Checked with a scratch test that drops the option: it fails with "Time
  limit was exceeded" and does not hang.

Both tests take .timeLimit(.minutes(1)) and pass while every cooperative
thread is blocked for several seconds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ot elapsed time

issuedTokenPersistenceNeverBlocksTheChannelActor asserted the actor answered
within 3 s while a token write waited on another connection's SQLite lock.
A saturated test pool can stall the run for longer than that, and a time
limit alone would miss the regression it guards (a write on the actor holds
it for SQLite's 30 s busy timeout, less than the one-minute limit).

The test now relies on ordering. It releases the lock only after the actor
answers, so the token can reach disk only if the actor answered while the
write was pending. A write on the actor fails with SQLITE_BUSY first, the
token never lands, and the final wait trips the new suite time limit.

The 200 ms sleep that let hello-ok reach the write is replaced by a DEBUG
hook, _test_setDeviceTokenPersistenceStartedHandler, that fires on the actor
just before the persistence hop. Once the hop starts, shutdown cannot stop it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… limit

The three tests asserted wall-clock bounds (2 s, 5 s, 1 s) that a saturated
test pool can overrun. Each now has a one-minute time limit instead, and
each makes sure a regression ends on the limit's cancellation instead of
hanging the run:

- file fetch refuses a FIFO: a blocking open(2) never returns. The
  cancellation handler opens the FIFO's write end once to release it.
- operationThatIgnoresCancellationStillTimesOut: the parked operation is a
  gate the test opens on cancellation (and afterwards), not a
  never-resumed continuation that a loser-joining race would wait on
  forever.
- total deadline can fire before the session starts: URLSession's own
  timeouts move past the limit, so only the fetcher's zero-second
  deadline can end the fetch. The fetch is awaited through a no-deadline
  AsyncTimeout race, because a deadline lost before start would leave it
  unresumed even on cancellation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aits

gatewayCoreWaitUntil gave up after 10 s (15 s at two call sites) and the
ChatViewModelSessionActionTests helpers after 15 s. A pool stall adds to
those waits, so they could time out even though the condition was about to
hold.

Both now wait until the condition holds or the test is cancelled. Every
suite that uses them has a one-minute time limit. gatewayCoreWaitUntil
records its GatewayCoreWaitTimeout as an issue before throwing, because
Swift Testing drops errors thrown after a time-limit cancellation and the
label says which wait hung. waitForForkStart awaits the gate's stream
directly instead of racing it against a sleep.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant