[AIT-1245] objects/uts: RTO27 lifecycle tests, RTO17/18 sync-event fix, and path_object quiescence barriers - #2278
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe changes replace fixed async flushing with polling barriers, update sync-state expectations for ChangesLiveObjects test and lifecycle updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
temporarily deployed
to
staging/pull/2278/bundle-report
July 30, 2026 09:39
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/uts/objects/unit/realtime_object.test.ts`:
- Around line 1607-1617: Extend the lifecycle assertions in the affected tests
to independently inspect the pooled maps map:profile@1000 and map:prefs@1000
after both detached/failed and suspended transitions. Verify their contents are
cleared or retained according to the expected lifecycle behavior, alongside the
existing root and counter assertions, so nested-map regressions cannot be hidden
by the root clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 14927b9b-a7e4-4d02-bc6d-e37232ce39d8
📒 Files selected for processing (2)
test/uts/objects/unit/path_object.test.tstest/uts/objects/unit/realtime_object.test.ts
There was a problem hiding this comment.
Pull request overview
Updates the UTS-derived objects/unit test suite to match recent spec changes (ably/specification#512), improving alignment around channel-state lifecycle behavior, sync event sequencing, and quiescence barriers for async inbound object updates.
Changes:
- Add RTO27 lifecycle tests asserting pooled object data clearing on
DETACHED/FAILED(no update events) and retention onSUSPENDED. - Correct RTO17/RTO18 test wording/expectations to require
SYNCING → SYNCEDeven whenHAS_OBJECTSis absent. - Replace single-yield quiescence (
flushAsync) with a spec-alignedpollUntilbarrier inpath_objectcycle tests to avoid races on async inboundMAP_SET.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/uts/objects/unit/realtime_object.test.ts | Adds RTO27 lifecycle coverage and updates sync-event expectations for ATTACHED without HAS_OBJECTS. |
| test/uts/objects/unit/path_object.test.ts | Uses pollUntil quiescence barrier to reliably observe async inbound MAP_SET effects before asserting compact/compactJson cycle behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
temporarily deployed
to
staging/pull/2278/bundle-report
July 30, 2026 10:26
Inactive
sacOO7
changed the title
github-actions
Bot
temporarily deployed
to
staging/pull/2278/bundle-report
July 31, 2026 08:59
Inactive
…x, path_object barriers Derived-test updates matching the objects/unit UTS spec changes in ably/specification#512. Test specs only — no SDK changes. - RTO27 (realtime_object): new tests for the objects-data lifecycle on channel state transitions — DETACHED/FAILED clear every pooled object's data to its zero value without emitting update events (objects remain in the pool); SUSPENDED retains the data. Driven via the internal actOnChannelState handler and asserted against _objectsPool (not black-box observable). - RTO17/RTO18 (realtime_object): rename "ATTACHED without HAS_OBJECTS emits SYNCED only" to "... emits SYNCING then SYNCED", matching RTO4c (sync state always transitions SYNCING -> SYNCED, even without the HAS_OBJECTS flag). - RTPO13c5 / RTPO14 (path_object): replace the flushAsync() barrier with the spec's poll_until(condition) — poll until the inbound MAP_SET ("back_ref") has applied before the positive read. Verified: realtime_object 41 passing, path_object 30 passing.
… with @SPEC RTO27 - realtime_object.test.ts (RTO27): assert the nested pooled map map:profile@1000 independently — cleared on DETACHED/FAILED, retained on SUSPENDED — via the pool (size 3 -> 0 / stays 3), so a nested-object regression can't be hidden by the root clear. Addresses the CodeRabbit review; mirrors the spec change in ably/specification#512 and the ably-java derived test. - realtimeobject.ts: annotate actOnChannelState with @SPEC RTO27 (and inline RTO27a/a1/a2/b) — comment-only; the behaviour was previously unannotated.
The dispatcher routes the ATTACHED transition to onAttached (RTO4) and all other
states per RTO27, so tag both spec points instead of RTO27 alone, and mark the
`attached` case inline as RTO4 (outside RTO27's scope). Broaden the RTO27b comment
to match the clarified spec clause: every non-{DETACHED,FAILED} state (SUSPENDED,
INITIALIZED, ATTACHING, DETACHING) retains the objects data unchanged. Comment/doc
only; no behaviour change.
sacOO7
force-pushed
the
objects-uts-align-spec-512
branch
from
July 31, 2026 09:04
87c5e1f to
cf38187
Compare
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.
Summary
Derived-test updates under
test/uts/objects/unit/that bring ably-js in line with the objectsobjects/unitUTS spec changes in ably/specification#512. Test specs only — no production/SDK changes.Changes
RTO27 — objects-data lifecycle on channel state transitions (
realtime_object.test.ts)New tests for the newly-specified RTO27:
0) without emitting update events; the objects themselves remain in the pool.White-box: driven through the internal
actOnChannelStatehandler and asserted against_objectsPool, because the behaviour isn't black-box observable (access onDETACHEDthrows, andSUSPENDEDis a connection-level state).RTO17/RTO18 — sync-event name correction (
realtime_object.test.ts)Renamed "ATTACHED without HAS_OBJECTS emits SYNCED only" → "… emits SYNCING then SYNCED", matching RTO4c: the sync state always transitions
SYNCING → SYNCED, even when theHAS_OBJECTSflag is absent.path_object quiescence barriers (
path_object.test.ts)RTPO13c5/RTPO14read local state immediately after an inboundMAP_SET(send_to_client), which applies asynchronously. Replaced theflushAsync()single-yield with the spec'spoll_until(condition)barrier — poll untilback_refhas landed inprofile.prefsbefore the positive read. This matches the spec's barrier form and the equivalent ably-java derived test.Testing
realtime_object.test.ts— 41 passingpath_object.test.ts— 30 passingRelated
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
SYNCINGis followed bySYNCEDwhen object presence is not signaled.Tests