Conversation
) `drainPendingTasks()` was 20 `Task.yield()` calls plus a 5ms sleep. `Task.yield()` is a scheduling hint, not a wait, so the ~60 sites that used it to "let the client catch up" raced two ways in a simulator: a simulated event (`simulateAck`, ...) could arrive before the `AppductClient` actor had reached `transport.connect`, in which case it is dropped as stray, and an assertion could read the synchronous `state`/`sessionId` snapshot before the actor had applied the event. TestSupport now offers `waitUntil(_:timeout:)`, which polls an observable condition (sync or async closure) with a short sleep and fails the test with a readable message -- never hangs -- on timeout, plus `allowQueuedWorkToRun()`, a deliberate bounded pause used only where a test asserts that *nothing* happens. Every drain site is replaced by a wait on the condition the test actually depends on: `transport.connectCallCount` reaching a value (a handshake is in flight, so a simulated ack will be picked up), a frame having reached `sentMessages`, the actor's `state`/`sessionId` reaching the expected value, or a collected listener event having arrived. `drainPendingTasks` is gone. Two negative assertions (the ignored late tool result, and a re-delivered deep link for the session already held) keep a bounded wait, now explicit and commented. No production code changed. Running the suite in a simulator in CI (and a test spec in AppductCore.podspec) remains a follow-up. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxtF2u7HBiZLmduthmmfvn
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #61.
Why
drainPendingTasks()inTestSupport.swiftwas 20Task.yield()calls plus a 5 ms sleep, used at 60 sites to "let the client catch up" before asserting.Task.yield()is a scheduling hint, not a wait, so in an iOS simulator a simulated event could arrive before theAppductClientactor had reachedtransport.connect(and be dropped as stray), or an assertion could read thestate/sessionIdsnapshot before the actor applied the event. That is whyAppductCore.podspeccarries no test spec.What changed
Tests only.
Sources/is untouched.TestSupport.swift:drainPendingTasksis removed.waitUntil(_:timeout:)polls an observable condition (sync or async closure) every 2 ms and, after 5 s, records anXCTFailwith the condition's description and throws, so a broken expectation fails readably instead of hanging.allowQueuedWorkToRun()is a deliberate bounded pause for negative assertions only.AppductAPITests.swift(14 sites) andAppductClientTests.swift(46 sites): every drain is replaced by a wait on the condition the test actually depends on:simulateAck:transport.connectCallCount >= n.performHandshakearms its pending attempt and then callstransport.connect, so a bumped counter proves the ack will be picked up.await client.state == …,await client.sessionId == …, orfacade.state == ….transport.sentMessages.contains { … }for the expected frame.EventCollectorholding the expected event.await facade.client.errorListeners.count >= 1(internal actor state via@testable import).Appduct.addListenerregisters the error channel last, so that implies all three channels are in place. A public registration handle would be the alternative if touching internals is unwelcome.testHandleUrlEmitsBootstrapErrorForMalformedPayloadandtestRestoreSessionStartsResumeFromAValidLeasegainedthrows; no other signatures changed.Verification
No Swift toolchain is available in the environment this was written in, so
swift testhas not been run on this branch. The change was reviewed line by line: every test usingtry await waitUntilis declaredthrows, brace balance matches the previous versions, and compound conditions use separateawaits rather thanawaitinside an&&autoclosure. The macOSswift testjob intest.yamlis the gate; please treat a red run there as this PR's to fix.Follow-up (not in this PR)
Running the suite in an iOS simulator in CI, ideally with
-test-iterations, and restoring a test spec inAppductCore.podspeconce it is reliable there.🤖 Generated with Claude Code
https://claude.ai/code/session_01NxtF2u7HBiZLmduthmmfvn
Generated by Claude Code