Conversation
…y; pin fresh-channel pipeline readiness Three derived-test fixes surfaced by real CI failures in ably-java, all of the same class: pseudocode steps whose faithful rendering is racy on an asynchronous SDK. - objects_faults.md: both scenarios awaited DISCONNECTED with AWAIT_STATE *after* the disconnect stimulus. DISCONNECTED after a drop from CONNECTED is transient at microsecond scale (RTN15a queues the reconnect before the DISCONNECTED emit), and AWAIT_STATE is level-triggered — the transition can fire and be superseded before the waiter registers, which the corpus itself already forbids (writing-test-specs.md, "Verifying Transient States"). Replace both with the record-and-verify pattern: recording registered before the stimulus, a poll_until gate on the recorded list where the wait is load-bearing, a sticky AWAIT_STATE CONNECTED for the final wait, and a CONTAINS_IN_ORDER assert for the observation-only scenario. Comments state the register-before-stimulus ordering as load-bearing so a translator who reorders can see the breakage. - realtime_object.md (RTO17-RTO18, "initial attach"): the fresh-channel scenario had no process_pending_events() between channels.get() and attach(), so an SDK whose objects message pipeline initializes asynchronously can drop the OBJECT_SYNC delivered by a synchronous mock. Add the step, matching the existing per-site convention. - writing-derived-tests.md: two mapping-table additions — a transient-state caveat on the AWAIT_STATE row (never await post-stimulus; use record-and-verify) and a recording-lists row (appended from SDK callback threads, read by poll_until from the test thread: multithreaded SDKs must render them as thread-safe lists).
There was a problem hiding this comment.
Pull request overview
Updates UTS guidance and scenarios to avoid asynchronous state and pipeline-readiness races.
Changes:
- Records transient connection states before disconnect stimuli.
- Flushes pending pipeline initialization before initial attach.
- Documents transient-state and thread-safe recording-list mappings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
uts/objects/integration/proxy/objects_faults.md |
Adopts record-and-verify disconnect handling. |
uts/objects/unit/realtime_object.md |
Adds a pipeline-readiness yield. |
uts/docs/writing-derived-tests.md |
Adds translation guidance for transient states and recording lists. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+225
to
+226
| // A publishes while B is down. Best-effort: RTN15a may reconnect/re-sync B before this | ||
| // round-trips (then it tests plain delivery, not RTO7/RTO8); the final poll tolerates both. |
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
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.
Problem
Three CI failures in ably-java's UTS-derived tests traced back to the same class of defect in the spec pseudocode: steps whose faithful rendering is racy on an asynchronous SDK.
uts/objects/integration/proxy/objects_faults.mddidAWAIT_STATE client.connection.state == DISCONNECTEDafter the disconnect stimulus.DISCONNECTEDafter a drop fromCONNECTEDis transient at microsecond scale — per RTN15a the client library queues the reconnect before the DISCONNECTED emit — andAWAIT_STATEis level-triggered from its call point, so the transition can fire and be superseded before any waiter registers. One missed window is unrecoverable (the state never recurs in these scenarios). The corpus already forbids exactly this shape (writing-test-specs.md, Verifying Transient States (Record-and-Verify Pattern), incl. the "May miss it!" example and common mistake MakeCryptoa static class consistently across all the SDKs #15) — these two sites predate-checked as non-conforming since creation. All three SDK renderings (java, js, cocoa) inherited the ordering literally; ably-java hit it in CI.RTO17-RTO18"initial attach" scenario inuts/objects/unit/realtime_object.mdhad noprocess_pending_events()betweenchannels.get()andattach(). An SDK whose objects message pipeline initializes asynchronously (ably-java: the incoming-message collector subscribes on a scheduled coroutine) can lose the OBJECT_SYNC delivered by a synchronous mock to a not-yet-subscribed pipeline — reproduced deterministically in ably-java under a 2-core constraint.CI evidence (ably-java): the proxy-tier failure in run 33166346294, the same transient-DISCONNECTED shape in run 33174366038 and run 34841083986, and the RTO17/RTO18 pipeline race in run 34874555278.
Changes
uts/objects/integration/proxy/objects_faults.mdBoth scenarios move to the corpus's record-and-verify pattern, with translator-proof comments (the register-before-stimulus ordering is stated as load-bearing — "reordering breaks the test"):
state_changesrecording registered before the stimulus; apoll_until(state_changes CONTAINS DISCONNECTED)gate afterattach()(the drop lands only after the sync frame round-trips, so an immediate sticky wait would no-op and race); the final wait targets the stickyCONNECTED; the observation is asserted withCONTAINS_IN_ORDER [DISCONNECTED, CONNECTING, CONNECTED](annotated as a subsequence match, since the pre-connect recording captures leading initial-connect states).trigger_action; the mid-test gate polls the recorded list (not live state) before client A publishes; the "while B is disconnected" best-effort caveat is stated honestly so translators don't add a brittle "assert still disconnected" step.uts/objects/unit/realtime_object.mdThe
RTO17-RTO18"initial attach" scenario gainsprocess_pending_events()afterchannels.get()(with the standard per-site pointer comment), so the objects message pipeline is subscribed beforeattach()— the same per-site convention the corpus already uses for dispatch-ordering yields.uts/docs/writing-derived-tests.mdTwo mapping-table additions so translators hit the rules at derivation time:
AWAIT_STATErow now carries the transient-state caveat: never await DISCONNECTED/SUSPENDED post-stimulus — use record-and-verify.state_changes = []/events = []are appended from SDK callback threads whilepoll_until/final asserts read them from the test thread — multithreaded SDKs must render them as thread-safe lists.Verification
The corrected shapes are rendered 1:1 in ably-java by ably-pubsub-java PR #1244: the previously flaking tests pass 10–25× consecutive stress runs (the RTO17/RTO18 scenario 2500 iterations under a CI-like 2-core constraint, where the old shape failed at iteration 1), and the full PR CI is green including the proxy tier against the real sandbox.
Related and follow-up
AWAIT_STATE ... DISCONNECTEDshape (channel_faults.md,connection_resume.md×4,heartbeat.md,presence_reentry.md);connection_open_failures.mdis the one safe site (drop from CONNECTING under a persistent suppress rule). Those have shipped renderings in multiple SDKs, so their sweep is left to a separate PR.