[AIT-1008] Translate LiveObjects objects UTS test specs to Kotlin tests#1222
Open
sacOO7 wants to merge 10 commits into
Open
[AIT-1008] Translate LiveObjects objects UTS test specs to Kotlin tests#1222sacOO7 wants to merge 10 commits into
objects UTS test specs to Kotlin tests#1222sacOO7 wants to merge 10 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds LiveObjects Kotlin UTS unit and integration tests, REST provisioning helpers, deviation documentation updates, and Gradle exclusions for translate-only LiveObjects tests. ChangesLiveObjects UTS Kotlin test suite
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
sacOO7
changed the title
uts-to-kotlin skillobjects UTS unit specs to Kotlin (10/15 specs, 181 tests)
sacOO7
changed the title
objects UTS unit specs to Kotlin (10/15 specs, 181 tests)objects UTS unit specs to Kotlin tests
…s.md Record the five objects/unit UTS specs (live_counter, live_map, object_id, objects_pool, parent_references) that cannot be translated into the :uts module. They assert on the internal CRDT graph (ObjectsPool, live nodes, applyOperation, object-id generation, parent references), which is (a) not yet implemented in :liveobjects and (b) Kotlin-`internal`, so unreachable from the :uts test module at compile time. Covers: status of all 15 objects/unit specs (10 translated, 5 blocked), why these target internals, the two blockers, and the realistic visibility options (java-test-fixtures bridge, tests in :liveobjects/src/test, or reflection) with their trade-offs and a recommendation.
sacOO7
force-pushed
the
feature/uts-liveobjects-unit-tests
branch
from
18a9d0c to
43a3ab2
Compare
Add the integration-tier translation of standard_test_pool.md's REST fixture
provisioning (`provision_objects_via_rest`) under
uts/.../integration/standard/liveobjects/helpers.kt: `provisionObjectsViaRest`
plus op builders (mapSet/mapRemove/mapCreate/counterCreate/counterInc) and value
builders (string/number/boolean/bytes/objectId).
The helper follows the LiveObjects V2 objects REST API (per the OpenAPI), not the
legacy pseudocode: POST /channels/{channel}/object (singular), body is a single
operation or a bare array (no "messages" envelope), each op identified by its
payload key with an objectId/path target. Compiles against :java only
(AblyRest + HttpUtils); used by objects/integration/RTPO15.
Document it in the uts-to-kotlin skill's objects-mapping.md as a new section
("14. Integration-test helpers"), promoted out of the unit-only internal-graph
section, with the TOC renumbered accordingly.
…ation/proxy tests
sacOO7
changed the title
objects UTS unit specs to Kotlin testsobjects UTS test specs to Kotlin tests
Applied on top of the merge of feature/liveobjects-uts-tests: - Point the liveobjects integration/proxy tests (ObjectsFaultsTest, ObjectsLifecycleTest, ObjectsSyncTest) at SandboxApp.sandboxHost, matching the consolidated sandbox-host constant. - Update objects-mapping.md (§12 nested-cause, §14 sandbox host / V2 REST) and the unit liveobjects tests for the same refactor. - Rename integration liveobjects helpers.kt -> Helpers.kt (case + content). Compiles clean (:uts:compileTestKotlin). The liveobjects tests remain translate-only at runtime (pending LiveObjects plugin wiring in the test harness); the realtime tiers pass.
sacOO7
changed the title
objects UTS test specs to Kotlin testsobjects UTS test specs to Kotlin tests
Base automatically changed from
feature/liveobjects-uts-tests
to
feature/path-based-liveobjects-implementation
June 30, 2026 10:29
…ture/uts-liveobjects-unit-tests
Running the Step 7 faithfulness audit (audit_translation.py) across the objects module surfaced three undocumented intentional omissions in the unit tests. Each is benign but was not recorded as a deviation; add entries so the audit reconciles clean: - RTPO10/10d/11/11d — `keys()`/`values()` "IS Array" is a static-type tautology (typed Iterable return), so the type-check line is omitted; count/membership are asserted (PathObjectTest). - RTO23f — `get()` result "IS PathObject" is guaranteed by the LiveMapPathObject return type; the runtime type-check is omitted (RealtimeObjectTest). - RTLC12b/c/d — relocated to RTO26 with no executable spec content, so the marker entry is intentionally not translated (LiveCounterApiTest). Integration and proxy objects tests audited clean; no deviations needed there.
…nted The liveobjects SDK has no implementation yet, so the translate-only liveobjects tests fail at runtime with "LiveObjects plugin hasn't been installed". Exclude them from runUtsUnitTests and runUtsIntegrationTests (unit + integration standard and proxy) so the CI gates pass; they still compile via compileTestKotlin. Marked with a TODO to remove the exclusions once the liveobjects implementation is in place.
Contributor
There was a problem hiding this comment.
Pull request overview
Translates the LiveObjects objects UTS specs into Kotlin tests under the :uts module (unit + standard integration + proxy integration), including documentation of deviations/blocked specs and minimal Gradle wiring to support the new test styles.
Changes:
- Adds translated LiveObjects unit tests (
io.ably.lib.uts.unit.liveobjects.*) plus deviations documentation and a write-up for currently blocked internal-graph specs. - Adds translated LiveObjects integration tests for direct sandbox and proxy fault-injection tiers, including a REST provisioning helper for seeding objects via the V2 REST contract.
- Updates
:utsGradle configuration to support JUnit parameterized tests and to exclude translate-only LiveObjects tests from the tiered UTS tasks.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/ValueTypesTest.kt |
Unit tests for value-type construction and LiveMapValue union behaviour, with documented deviations where wire/evaluate internals aren’t public. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt |
Unit tests for RealtimeObject.get() preconditions, sync events, publish-and-apply observable effects, and GC behaviour (translate-only until engine lands). |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PublicObjectMessageTest.kt |
Unit tests for public ObjectMessage/ObjectOperation construction, using reflection to reach internal wire/public conversion. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectTest.kt |
Unit tests covering read/navigation APIs on PathObject and snapshot (compactJson) behaviour. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectSubscribeTest.kt |
Unit tests for path subscriptions and depth/prefix dispatch rules (translate-only until engine lands). |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectMutationsTest.kt |
Unit tests for map/counter mutation APIs via typed path views and expected error codes. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.kt |
Unit tests for Instance.subscribe(...) delivery semantics and tombstone/noop adaptations. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveMapApiTest.kt |
Unit tests for LiveMap public API behaviour, asserting observable effects where wire-shape assertions are internal. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveCounterApiTest.kt |
Unit tests for LiveCounter public API behaviour; includes reflection helpers to inspect outbound wire objects. |
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/InstanceTest.kt |
Unit tests for typed Instance API behaviour with deviations for cast-time failures vs spec error codes. |
uts/src/test/kotlin/io/ably/lib/uts/private_deviations.md |
Documents the 5 unit specs blocked on missing internal CRDT engine + cross-module internal visibility. |
uts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/ObjectsSyncTest.kt |
Standard integration tests against sandbox for attach→sync→get and multi-client sync behaviours. |
uts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/ObjectsLifecycleTest.kt |
Standard integration tests for end-to-end lifecycle (create/mutate/propagate), including REST-provisioned seed case. |
uts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/Helpers.kt |
Integration helper implementing REST provisioning against the V2 objects REST API contract. |
uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/liveobjects/ObjectsFaultsTest.kt |
Proxy integration tests injecting transport/channel faults during sync/mutation flows. |
uts/src/test/kotlin/io/ably/lib/uts/deviations.md |
Adds detailed deviation entries for LiveObjects UTS translations. |
uts/build.gradle.kts |
Adds JUnit params dependency and excludes translate-only LiveObjects tests from tiered UTS tasks. |
.claude/skills/uts-to-kotlin/references/objects-mapping.md |
Updates the mapping reference doc with integration REST provisioning helper guidance and error-cause notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
55
to
+59
| tasks.register<Test>("runUtsIntegrationTests") { | ||
| filter { | ||
| includeTestsMatching("io.ably.lib.uts.integration.*") | ||
| // liveobjects has no SDK implementation yet — exclude the translate-only liveobjects | ||
| // tests (standard + proxy) from the run; they still compile via compileTestKotlin. |
Comment on lines
+123
to
+153
| val rest = AblyRest( | ||
| ClientOptions().apply { | ||
| key = apiKey | ||
| // Target the same nonprod sandbox host that SandboxApp and the realtime clients use | ||
| // (sandbox.realtime.ably-nonprod.net) — NOT environment="sandbox", which resolves to the | ||
| // legacy prod-sandbox host sandbox-rest.ably.io (Hosts.java also forbids setting both | ||
| // environment and restHost). Matches standard_test_pool.md (ably/specification#497). | ||
| restHost = SandboxApp.sandboxHost | ||
| useBinaryProtocol = false | ||
| }, | ||
| ) | ||
|
|
||
| val path = "/channels/$channelName/object" // V2: singular `object` | ||
| val body: JsonElement = | ||
| if (operations.size == 1) operations[0] else JsonArray().apply { operations.forEach { add(it) } } | ||
|
|
||
| val response = rest.request( | ||
| "POST", | ||
| path, | ||
| null, | ||
| HttpUtils.requestBodyFromGson(body, rest.options.useBinaryProtocol), | ||
| null, | ||
| ) | ||
| check(response.success) { | ||
| "REST objects provisioning failed: HTTP ${response.statusCode} ${response.errorMessage}" | ||
| } | ||
|
|
||
| return response.items().flatMap { item -> | ||
| item.asJsonObject.get("objectIds")?.asJsonArray?.map { it.asString } ?: emptyList() | ||
| } | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PublicObjectMessageTest.kt (1)
225-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hard-coding internal wire action ordinals here.
0and3are protocol-internal encodings. If they drift,toObjectMessage(...)will deserialize the wrong variant and these tests stop exercising the derived-create path. Derive theactionvalue from the builder-generated direct-create JSON (or a shared helper) so this synthetic payload stays coupled to the same wire contract.Also applies to: 262-280
🤖 Prompt for 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. In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PublicObjectMessageTest.kt` around lines 225 - 246, The synthetic payload in the derived-create tests is hard-coding protocol wire ordinals for the operation action, which can desync from the real contract. Update the test setup around publicMessageWithDerivedCreate and the related cases in the same block to source the action value from builder-generated direct-create JSON or a shared helper instead of literals. Keep the derived-create payload coupled to the same JSON shape used by the production builder so toObjectMessage(...) continues to exercise the intended variant even if the wire encoding changes.
🤖 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
`@uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/liveobjects/ObjectsFaultsTest.kt`:
- Around line 141-147: The test currently publishes the mutation while clientB
is fully disconnected, so it does not exercise buffering during the re-sync
window. Update ObjectsFaultsTest to keep clientB in the SYNCING state after
reconnecting, then publish rootA.set(...) before the delayed OBJECT_SYNC
completes, and only assert on rootB after the sync finishes. Use the existing
clientB, rootA, rootB, awaitState, and objectChannel flow to make the timing
explicit.
- Around line 212-230: The current test only injects the channel error after
`channel.`object`.get().await()` has already finished syncing, so it never
exercises the `FAILED during SYNCING` case. Update `ObjectsFaultsTest` around
the `root = ...get().await()` / `session.triggerAction(...)` flow so the failure
is injected while the `object` sync is still in progress, and assert that the
`root.set(...)` or `publishAndApply` path is interrupted by the transition to
`ChannelState.failed` during sync rather than after sync has completed.
- Around line 271-275: The test in ObjectsFaultsTest should wait for client B to
actually enter the attach/sync window before calling rootA.set, because
publishing immediately after channelB.attach can race and hide the buffering
behavior. Add a synchronization barrier after channelB.attach in the test flow,
using the existing attach/sync path around objectChannel and channelB so the
mutation is sent only once B is confirmed to be syncing. Keep the assertion
logic unchanged, but ensure the mutation happens while B is genuinely mid-sync.
In
`@uts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/Helpers.kt`:
- Around line 52-58: The `operation()` helper currently permits both `objectId`
and `path` to be set, or neither, which can produce invalid fixtures. Update the
`operation(objectId, path, id, build)` builder to validate that exactly one REST
target is provided before constructing the `JsonObject`, and fail fast if the
inputs violate that rule. Use the existing `operation` helper as the enforcement
point so all callers inherit the V2 contract (`objectId` XOR `path`).
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/InstanceTest.kt`:
- Around line 333-347: The current test in InstanceTest is not proving
identity-based routing because the final id assertion can pass even if the
callback fired for the repoint operation instead of the original counter. Update
the assertions around the events list to validate the delivered event itself,
using the event payload from the listener (such as events[0].getObject() and/or
events[0].getMessage().operation.objectId), and make the test explicitly verify
that the first MAP_SET on root.score did not trigger a callback before the
counter increment does.
- Around line 309-320: The unsubscribe test in `RTINS16f - subscribe returns
Subscription for deregistration` is asserting `events.size == 0` immediately
after `mockWs.sendToClient(...)`, which can miss late asynchronous delivery.
Update the test to wait through the dispatch window in `runTest` before the
final assertion, using an observable processing milestone or a short wait, so it
verifies no event is delivered after `sub.unsubscribe()`. Keep the check tied to
`InstanceSubscriptionEvent`, `sub.unsubscribe()`, and the `events` list.
In
`@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.kt`:
- Around line 77-88: The negative subscription checks in LiveObjectSubscribeTest
are racing the dispatcher because they only wait for the first update before
asserting no more deliveries. Update the unsubscribe/noop/tombstone assertions
in the affected test cases to wait for the relevant callbacks to settle,
especially both listeners in the tombstone flow, using the existing helpers
around updates and pollUntil so the counts remain stable for a short window
before asserting no further messages were delivered.
In
`@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectSubscribeTest.kt`:
- Around line 126-130: The negative subscription checks in
PathObjectSubscribeTest are racy because the test asserts listener counts
immediately after async sends, so late callbacks can arrive after the assertion.
Update the affected scenarios to use the existing pollUntil(...) pattern or an
equivalent bounded quiet-period wait after mockWs.sendToClient and before
checking events.size, covering the depth-filtering, unsubscribe,
prefix-mismatch, and exactly-once assertions. Keep the assertions in the same
test methods but ensure the count stays unchanged for a short stable period
before validating it.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt`:
- Around line 941-944: The re-sync assertion in RealtimeObjectTest is using the
wrong serial/site, so it does not actually verify the apply-on-ACK clearing
behavior. Update the replay in the test to reuse the same ACK serial and site
identifier that were used earlier when the mutation was applied on ACK, and keep
the check in the same test flow around buildObjectMessage/buildCounterInc so it
fails if appliedOnAckSerials is not cleared during re-sync.
- Around line 429-438: The unsubscribe/resubscribe check in RealtimeObjectTest
is asserting too early, so a late callback could still arrive after the test
passes. Update the test around the mocked sync messages and the callCount
assertion to wait for async delivery to settle, using the existing
pollUntil(...) pattern already used elsewhere in this file, and keep the
assertion on callCount in place only after that drain.
- Around line 329-347: The test setup in RTO20e1 is sending a detached protocol
event even though the scenario is meant to cover a channel entering FAILED
during sync wait. Update the stimulus in `RealtimeObjectTest` to use the FAILED
state transition instead of `ProtocolMessage.Action.detached`, and keep the
existing assertions around `publishAndApply` so the test truly exercises
failed-state handling in
`setupSyncedChannel`/`root.get("score").asLiveCounter().increment(10)`.
- Around line 888-892: The siteTimeserials replay test is using a different
tuple than the helper’s ACK tuple, so it can miss a bug in apply-on-ACK updates.
In RealtimeObjectTest, update the inbound COUNTER_INC in the siteTimeserials
scenario to replay the actual ACK tuple used by the sibling ACK tests and the
helper’s ACK path, keeping the message aligned with the ACK that was already
seen. Use the existing test helpers and identifiers around buildObjectMessage,
buildCounterInc, and the siteTimeserials assertions to locate and adjust the
message payload.
- Around line 304-322: The test `RTO20e - publishAndApply waits for SYNCED
during SYNCING` only checks the final post-sync result, so it does not verify
that `increment(10)` was actually deferred while the channel was in `SYNCING`.
In `RealtimeObjectTest`, after calling
`root.get("score").asLiveCounter().increment(10)` and before sending the sync
message, add an assertion that the returned future is still not completed and/or
that the counter value is still the pre-sync value. Keep the existing
`incFuture` and `buildObjectSyncMessage` flow, but insert the pre-sync
pending-state check so the test proves `publishAndApply` waits for `SYNCED`.
- Around line 1029-1033: The “initial attach” scenario in RealtimeObjectTest is
starting from a pre-synced channel, so it never exercises a real first-time
attach path. Update the scenario setup in the relevant Scenario list (including
the one reused in the later block) so the “initial attach” case begins from an
unattached/unsynced channel state, then calls channel.attach() and validates the
expected SYNCING/SYNCED events from that true initial attach flow.
- Around line 191-199: The `RealtimeObjectTest` case is a documented deviation
that cannot be exercised through the public API, so it should not remain a
passing `@Test`. Update the `RTO15 - publish sends OBJECT ProtocolMessage`
method to be ignored/disabled, or remove the test annotation entirely and keep
the note only in the deviation documentation, so the test matrix does not report
a false green for an unexpressible scenario.
---
Nitpick comments:
In
`@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PublicObjectMessageTest.kt`:
- Around line 225-246: The synthetic payload in the derived-create tests is
hard-coding protocol wire ordinals for the operation action, which can desync
from the real contract. Update the test setup around
publicMessageWithDerivedCreate and the related cases in the same block to source
the action value from builder-generated direct-create JSON or a shared helper
instead of literals. Keep the derived-create payload coupled to the same JSON
shape used by the production builder so toObjectMessage(...) continues to
exercise the intended variant even if the wire encoding changes.
🪄 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
Run ID: 0e7a322c-f3d2-4391-bda5-27803981543b
📒 Files selected for processing (18)
.claude/skills/uts-to-kotlin/references/objects-mapping.mduts/build.gradle.ktsuts/src/test/kotlin/io/ably/lib/uts/deviations.mduts/src/test/kotlin/io/ably/lib/uts/integration/proxy/liveobjects/ObjectsFaultsTest.ktuts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/Helpers.ktuts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/ObjectsLifecycleTest.ktuts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/ObjectsSyncTest.ktuts/src/test/kotlin/io/ably/lib/uts/private_deviations.mduts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/InstanceTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveCounterApiTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveMapApiTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectMutationsTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectSubscribeTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PathObjectTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/PublicObjectMessageTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.ktuts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/ValueTypesTest.kt
Comment on lines
+141
to
+147
| // While B is disconnected, A publishes a mutation | ||
| rootA.set("key1", LiveMapValue.of("updated_during_disconnect")).await() | ||
|
|
||
| // Client B reconnects and re-syncs; the mutation should be visible | ||
| awaitState(clientB, ConnectionState.connected, 30.seconds) | ||
| rootB = withTimeout(15.seconds) { objectChannel(clientB, channelName).`object`.get().await() } | ||
| pollUntil(15.seconds) { rootB.get("key1").asString().value() == "updated_during_disconnect" } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
This never publishes while client B is re-syncing.
At Line 142 the mutation is sent while clientB is fully disconnected. By the time B reconnects, the new value can arrive entirely via the next OBJECT_SYNC snapshot, so this test can pass even if buffering during the re-sync window is broken. Hold B in SYNCING after reconnect, then publish from A before the delayed sync completes.
🤖 Prompt for 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.
In
`@uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/liveobjects/ObjectsFaultsTest.kt`
around lines 141 - 147, The test currently publishes the mutation while clientB
is fully disconnected, so it does not exercise buffering during the re-sync
window. Update ObjectsFaultsTest to keep clientB in the SYNCING state after
reconnecting, then publish rootA.set(...) before the delayed OBJECT_SYNC
completes, and only assert on rootB after the sync finishes. Use the existing
clientB, rootA, rootB, awaitState, and objectChannel flow to make the timing
explicit.
Comment on lines
+212
to
+230
| val channel = objectChannel(client, channelName) | ||
| val root = withTimeout(15.seconds) { channel.`object`.get().await() } | ||
|
|
||
| // Inject a channel ERROR (action 9) to transition the channel to FAILED. | ||
| session.triggerAction( | ||
| mapOf( | ||
| "type" to "inject_to_client", | ||
| "message" to mapOf( | ||
| "action" to 9, | ||
| "channel" to channelName, | ||
| "error" to mapOf("statusCode" to 400, "code" to 90000, "message" to "injected error"), | ||
| ), | ||
| ), | ||
| ) | ||
| awaitChannelState(channel, ChannelState.failed, 15.seconds) | ||
|
|
||
| // A mutation (publishAndApply internally) must fail since the channel is FAILED. | ||
| val error = assertFailsWith<AblyException> { | ||
| root.set("key", LiveMapValue.of("value")).await() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The FAILED during SYNCING path is not exercised here.
channel.object.get().await() at Line 213 only resolves after sync has completed, so the injected channel error at Lines 216-225 happens after the SYNCING phase. This still checks "mutations fail on a FAILED channel", but it does not cover the RTO20e timing where publishAndApply is interrupted by a transition to FAILED during sync.
🤖 Prompt for 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.
In
`@uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/liveobjects/ObjectsFaultsTest.kt`
around lines 212 - 230, The current test only injects the channel error after
`channel.`object`.get().await()` has already finished syncing, so it never
exercises the `FAILED during SYNCING` case. Update `ObjectsFaultsTest` around
the `root = ...get().await()` / `session.triggerAction(...)` flow so the failure
is injected while the `object` sync is still in progress, and assert that the
`root.set(...)` or `publishAndApply` path is interrupted by the transition to
`ChannelState.failed` during sync rather than after sync has completed.
Comment on lines
+271
to
+275
| val channelB = objectChannel(clientB, channelName) | ||
| channelB.attach() | ||
|
|
||
| // While B is syncing, A publishes a mutation. | ||
| rootA.set("existing", LiveMapValue.of("after")).await() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Wait until client B is actually in the attach/sync window before publishing.
The mutation at Line 275 is sent immediately after channelB.attach(). If that write wins the race before B's attach is established, the "after" value can be folded into the initial sync snapshot and this test still passes without proving the echo was buffered until sync completion. Add a barrier before publishing.
Proposed fix
clientB.connect()
awaitState(clientB, ConnectionState.connected, 15.seconds)
val channelB = objectChannel(clientB, channelName)
channelB.attach()
+ awaitChannelState(channelB, ChannelState.attached, 15.seconds)
// While B is syncing, A publishes a mutation.
rootA.set("existing", LiveMapValue.of("after")).await()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val channelB = objectChannel(clientB, channelName) | |
| channelB.attach() | |
| // While B is syncing, A publishes a mutation. | |
| rootA.set("existing", LiveMapValue.of("after")).await() | |
| val channelB = objectChannel(clientB, channelName) | |
| channelB.attach() | |
| awaitChannelState(channelB, ChannelState.attached, 15.seconds) | |
| // While B is syncing, A publishes a mutation. | |
| rootA.set("existing", LiveMapValue.of("after")).await() |
🤖 Prompt for 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.
In
`@uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/liveobjects/ObjectsFaultsTest.kt`
around lines 271 - 275, The test in ObjectsFaultsTest should wait for client B
to actually enter the attach/sync window before calling rootA.set, because
publishing immediately after channelB.attach can race and hide the buffering
behavior. Add a synchronization barrier after channelB.attach in the test flow,
using the existing attach/sync path around objectChannel and channelB so the
mutation is sent only once B is confirmed to be syncing. Keep the assertion
logic unchanged, but ensure the mutation happens while B is genuinely mid-sync.
Comment on lines
+52
to
+58
| private fun operation(objectId: String?, path: String?, id: String?, build: JsonObject.() -> Unit): JsonObject = | ||
| JsonObject().apply { | ||
| id?.let { addProperty("id", it) } // OperationBase.id — idempotency key | ||
| objectId?.let { addProperty("objectId", it) } | ||
| path?.let { addProperty("path", it) } | ||
| build() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Enforce exactly one REST target here.
The V2 contract in this file is objectId or path, but operation() currently allows both or neither. That makes every public builder capable of emitting malformed fixtures and pushes the failure into the REST call instead of failing fast at the helper boundary.
Proposed fix
-private fun operation(objectId: String?, path: String?, id: String?, build: JsonObject.() -> Unit): JsonObject =
- JsonObject().apply {
- id?.let { addProperty("id", it) } // OperationBase.id — idempotency key
- objectId?.let { addProperty("objectId", it) }
- path?.let { addProperty("path", it) }
- build()
- }
+private fun operation(objectId: String?, path: String?, id: String?, build: JsonObject.() -> Unit): JsonObject {
+ require((objectId == null) != (path == null)) {
+ "exactly one of objectId or path must be provided"
+ }
+ return JsonObject().apply {
+ id?.let { addProperty("id", it) } // OperationBase.id — idempotency key
+ objectId?.let { addProperty("objectId", it) }
+ path?.let { addProperty("path", it) }
+ build()
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private fun operation(objectId: String?, path: String?, id: String?, build: JsonObject.() -> Unit): JsonObject = | |
| JsonObject().apply { | |
| id?.let { addProperty("id", it) } // OperationBase.id — idempotency key | |
| objectId?.let { addProperty("objectId", it) } | |
| path?.let { addProperty("path", it) } | |
| build() | |
| } | |
| private fun operation(objectId: String?, path: String?, id: String?, build: JsonObject.() -> Unit): JsonObject { | |
| require((objectId == null) != (path == null)) { | |
| "exactly one of objectId or path must be provided" | |
| } | |
| return JsonObject().apply { | |
| id?.let { addProperty("id", it) } // OperationBase.id — idempotency key | |
| objectId?.let { addProperty("objectId", it) } | |
| path?.let { addProperty("path", it) } | |
| build() | |
| } | |
| } |
🤖 Prompt for 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.
In
`@uts/src/test/kotlin/io/ably/lib/uts/integration/standard/liveobjects/Helpers.kt`
around lines 52 - 58, The `operation()` helper currently permits both `objectId`
and `path` to be set, or neither, which can produce invalid fixtures. Update the
`operation(objectId, path, id, build)` builder to validate that exactly one REST
target is provided before constructing the `JsonObject`, and fail fast if the
inputs violate that rule. Use the existing `operation` helper as the enforcement
point so all callers inherit the V2 contract (`objectId` XOR `path`).
Comment on lines
+309
to
+320
| fun `RTINS16f - subscribe returns Subscription for deregistration`() = runTest { | ||
| val (_, _, root, mockWs) = setupSyncedChannel("test") | ||
| val counterInst = root.get("score").instance()!!.asLiveCounter() | ||
| val events = mutableListOf<InstanceSubscriptionEvent>() | ||
| val sub = counterInst.subscribe(InstanceListener { events.add(it) }) | ||
| sub.unsubscribe() | ||
|
|
||
| mockWs.sendToClient( | ||
| buildObjectMessage("test", listOf(buildCounterInc("counter:score@1000", 7, "99", "remote"))), | ||
| ) | ||
|
|
||
| assertEquals(0, events.size) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait through the dispatch window before concluding unsubscribe worked.
After sendToClient(...) this test asserts events.size == 0 immediately. If callback delivery is asynchronous, it can pass even when unsubscribe is broken and the event arrives a tick later. Gate the final assertion on some observable processing milestone, or hold the count steady across a short wait before asserting no delivery.
🤖 Prompt for 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.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/InstanceTest.kt` around
lines 309 - 320, The unsubscribe test in `RTINS16f - subscribe returns
Subscription for deregistration` is asserting `events.size == 0` immediately
after `mockWs.sendToClient(...)`, which can miss late asynchronous delivery.
Update the test to wait through the dispatch window in `runTest` before the
final assertion, using an observable processing milestone or a short wait, so it
verifies no event is delivered after `sub.unsubscribe()`. Keep the check tied to
`InstanceSubscriptionEvent`, `sub.unsubscribe()`, and the `events` list.
Comment on lines
+329
to
+347
| fun `RTO20e1 - publishAndApply fails when channel enters FAILED during sync wait`() = runTest { | ||
| val (_, _, root, mockWs) = setupSyncedChannel("test") | ||
|
|
||
| mockWs.sendToClient( | ||
| ProtocolMessage(ProtocolMessage.Action.attached).apply { | ||
| this.channel = "test" | ||
| channelSerial = "sync2:cursor" | ||
| setFlag(ProtocolMessage.Flag.has_objects) | ||
| }, | ||
| ) | ||
|
|
||
| val incFuture = root.get("score").asLiveCounter().increment(10) | ||
|
|
||
| mockWs.sendToClient( | ||
| ProtocolMessage(ProtocolMessage.Action.detached).apply { | ||
| this.channel = "test" | ||
| error = ErrorInfo("Channel detached", 400, 90000) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The stimulus here is DETACHED, not FAILED.
The test name/spec says “channel enters FAILED”, but Line 343 injects ProtocolMessage.Action.detached. That only exercises detach-while-syncing, so a regression in failed-state handling would be missed.
🤖 Prompt for 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.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt`
around lines 329 - 347, The test setup in RTO20e1 is sending a detached protocol
event even though the scenario is meant to cover a channel entering FAILED
during sync wait. Update the stimulus in `RealtimeObjectTest` to use the FAILED
state transition instead of `ProtocolMessage.Action.detached`, and keep the
existing assertions around `publishAndApply` so the test truly exercises
failed-state handling in
`setupSyncedChannel`/`root.get("score").asLiveCounter().increment(10)`.
Comment on lines
+429
to
+438
| mockWs.sendToClient( | ||
| ProtocolMessage(ProtocolMessage.Action.attached).apply { | ||
| this.channel = "test" | ||
| channelSerial = "sync2:cursor" | ||
| setFlag(ProtocolMessage.Flag.has_objects) | ||
| }, | ||
| ) | ||
| mockWs.sendToClient(buildObjectSyncMessage("test", "sync2:", STANDARD_POOL_OBJECTS)) | ||
|
|
||
| assertEquals(0, callCount) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Wait long enough to prove the listener stays unsubscribed.
This assertion runs immediately after queueing the re-sync messages. Elsewhere in this file you already use pollUntil(...) for async delivery; without a similar drain/wait here, a late buggy callback can fire after the test has already passed.
🤖 Prompt for 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.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt`
around lines 429 - 438, The unsubscribe/resubscribe check in RealtimeObjectTest
is asserting too early, so a late callback could still arrive after the test
passes. Update the test around the mocked sync messages and the callCount
assertion to wait for async delivery to settle, using the existing
pollUntil(...) pattern already used elsewhere in this file, and keep the
assertion on callCount in place only after that drain.
Comment on lines
+888
to
+892
| // Inbound COUNTER_INC from siteCode "test" with serial "t:1:0" (same as the ACK). If LOCAL had | ||
| // incorrectly written siteTimeserials, the newness check would reject this as stale. | ||
| mockWs.sendToClient( | ||
| buildObjectMessage("test", listOf(buildCounterInc("counter:score@1000", 10, "t:1:0", "test"))), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Replay the actual ACK tuple in the siteTimeserials test.
This comment says the inbound message uses “the same as the ACK”, but sibling ACK tests in this file use the helper’s ACK tuple (ack-0:0 / test-site), while this test injects t:1:0 / test. Because the tuple is different, the assertion still passes even if apply-on-ACK incorrectly updates siteTimeserials.
🤖 Prompt for 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.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt`
around lines 888 - 892, The siteTimeserials replay test is using a different
tuple than the helper’s ACK tuple, so it can miss a bug in apply-on-ACK updates.
In RealtimeObjectTest, update the inbound COUNTER_INC in the siteTimeserials
scenario to replay the actual ACK tuple used by the sibling ACK tests and the
helper’s ACK path, keeping the message aligned with the ACK that was already
seen. Use the existing test helpers and identifiers around buildObjectMessage,
buildCounterInc, and the siteTimeserials assertions to locate and adjust the
message payload.
Comment on lines
+941
to
+944
| // Replay the same serial used for apply-on-ACK. If cleared, this applies normally. | ||
| mockWs.sendToClient( | ||
| buildObjectMessage("test", listOf(buildCounterInc("counter:score@1000", 10, "t:1:0", "test"))), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This re-sync check is replaying the wrong serial.
The test claims to replay “the same serial used for apply-on-ACK”, but it injects t:1:0 / test instead of the ACK serial/site used earlier. If appliedOnAckSerials were never cleared on re-sync, this message would still apply, so the regression would go undetected.
🤖 Prompt for 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.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt`
around lines 941 - 944, The re-sync assertion in RealtimeObjectTest is using the
wrong serial/site, so it does not actually verify the apply-on-ACK clearing
behavior. Update the replay in the test to reuse the same ACK serial and site
identifier that were used earlier when the mutation was applied on ACK, and keep
the check in the same test flow around buildObjectMessage/buildCounterInc so it
fails if appliedOnAckSerials is not cleared during re-sync.
Comment on lines
+1029
to
+1033
| Scenario( | ||
| name = "initial attach", | ||
| trigger = { channel, _ -> channel.attach() }, | ||
| expectedEvents = listOf("SYNCING", "SYNCED"), | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The “initial attach” scenario starts from an already-synced channel.
Each iteration uses setupSyncedChannel("test"), so the "initial attach" case never exercises an actual initial attach. Calling channel.attach() from that state is either a no-op or a different path, which makes this scenario unable to validate the event sequence it names.
Also applies to: 1079-1088
🤖 Prompt for 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.
In `@uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/RealtimeObjectTest.kt`
around lines 1029 - 1033, The “initial attach” scenario in RealtimeObjectTest is
starting from a pre-synced channel, so it never exercises a real first-time
attach path. Update the scenario setup in the relevant Scenario list (including
the one reused in the later block) so the “initial attach” case begins from an
unattached/unsynced channel state, then calls channel.attach() and validates the
expected SYNCING/SYNCED events from that true initial attach flow.
Align the skill's Step 6 with the spec manual (ably/specification#498) so a diagnosed UTS spec error is handled spec-first instead of being quietly adapted to green: - Step 6 intro now names three acceptable end-states (spec-correct pass, SDK deviation stays green, UTS spec error fails fast) — removes the earlier "a red test is never acceptable" contradiction. - Decision tree NO-branch: fix the spec at source + record under deviations.md "UTS Spec Errors" + emit a fail-fast test. - Add the "spec-error fail-fast" Kotlin pattern (mirrors the manual's JS example) and retitle the section to "Test patterns for a diagnosed failure" since fail-fast is not a deviation. - Defer the deviations-file entry format to the manual's "Recording deviations" sections (removes the divergent inline field list).
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
Translates the
objects(LiveObjects) UTS unit specs into runnable Kotlin tests in theutsmodule, using the
uts-to-kotlinskill. Of the 15objects/unitspecs, 10 are translated(181 tests) and 5 are blocked by missing internal implementation (documented, not silently
skipped).
This is translate-only: every test compiles (
./gradlew :uts:compileTestKotlinis green). Mostrun only once the LiveObjects engine (
OBJECT_SYNCprocessing +RealtimeObject.get()) lands; themessage-layer and value-type construction tests run today.
What's included
10 translated spec files — 181
@Testmethodsobjects/unit/…)instance.mdInstanceTestInstance)path_object.mdPathObjectTestPathObject)path_object_mutations.mdPathObjectMutationsTestpath_object_subscribe.mdPathObjectSubscribeTestlive_counter_api.mdLiveCounterApiTestlive_map_api.mdLiveMapApiTestlive_object_subscribe.mdLiveObjectSubscribeTestpublic_object_message.mdPublicObjectMessageTestrealtime_object.mdRealtimeObjectTestget()+ sync events)value_types.mdValueTypesTestcreatesurface)Each test carries a verbatim
@UTS objects/unit/…tag tracing it to the source spec point. All filesfollow the
utsconventions (no star imports,runTest, sharedhelpers.kt).Deviations —
uts/src/test/kotlin/io/ably/lib/uts/deviations.mdPer-test divergences where ably-java (a typed SDK) differs from the spec's dynamic API, e.g.:
Instancecast throwsIllegalStateExceptionrather thanErrorInfo 92007;value()/size()partitioned off the wrong-typed view (not expressible);compact()unimplemented →compactJson()used;Blocked specs —
uts/src/test/kotlin/io/ably/lib/uts/private_deviations.mdThe 5 specs that could not be translated, with the reasons and a recommended path forward.
The 5 blocked specs
live_counter.md,live_map.md,object_id.md,objects_pool.md,parent_references.mdassert on theinternal CRDT graph (live nodes +
applyOperation,siteTimeserials, theObjectsPool, object-idgeneration, parent references). Two blockers:
:liveobjectscurrently ships only the public view layer; theCRDT engine these specs test does not exist (
ObjectsPool,generateObjectId,applyOperation, … =0 references). There is nothing to assert against.
internalvisibility (secondary). Once implemented, those symbols will be Kotlininternaland unreachable from the:utstest module at compile time.private_deviations.mdlays out the realistic options for when the engine lands — the recommended one(to keep tests under
uts/unit) is ajava-test-fixturesbridge in:liveobjectsexposing a smallpublic inspection surface; the lowest-ceremony alternative is authoring them in
:liveobjects/src/test.Notes / caveats
deviations.mdentries thatdocument SDK behaviour become live once the engine is implemented.
documented no-op test bodies, paired with a
// DEVIATIONnote +deviations.mdentry.LiveCounterApiTest/LiveMapApiTestcontain a few wire-shape assertions read reflectively /via observable round-trip; these are unverified until the SDK publish path lands (flagged in the code).
How it was produced
Driven by the
uts-to-kotlinskill (per-spec: read theobjectsmapping doc + infra, translate, compileonce for the whole module, review against the spec). The bulk batch was fanned out with a multi-agent
workflow (one agent per spec, parallel), then compiled and fixed centrally.
Testing
Update — integration & proxy tiers added
Extends this PR beyond the unit tier: the direct-sandbox integration and proxy integration
objectsspecs are now translated. Still translate-only (compiles; runs once the LiveObjects engine lands and against the sandbox). With these, all non-blockedobjectsspecs across all three tiers are translated — only the 5 internal-graph unit specs remain blocked (above).Integration tier — direct sandbox (
integration/standard/liveobjects/)Real-sandbox happy path: connect → sync → create/mutate via
PathObject→ verify propagation to a second client; no fault injection. Each case runs once per protocol variant (JSON + msgpack) via auseBinaryProtocol@ParameterizedTest.objects/integration/…)objects_lifecycle_test.mdObjectsLifecycleTestobjects_sync_test.mdObjectsSyncTestClients hit the nonprod sandbox (
sandbox.realtime.ably-nonprod.netviaProxyManager.sandboxRealtimeHost); a throwaway app is provisioned withSandboxAppin@BeforeAll/@AfterAll.Proxy tier — sandbox + fault injection (
integration/proxy/liveobjects/)Routes the SDK through the programmable
uts-proxyto inject transport faults (disconnect mid-OBJECT_SYNC, delayed sync, server-initiatedDETACHED, channelERROR→FAILED). JSON-only (the proxy inspects only text frames).objects/integration/proxy/…)objects_faults.mdObjectsFaultsTestOther changes
integration/standard/liveobjects/helpers.kt:provisionObjectsViaRest(...)plus op/value builders, the ably-java translation ofstandard_test_pool.md'sprovision_objects_via_rest. Built to the V2 objects REST contract (POST /channels/{ch}/object, payload-key ops, no{messages}envelope), targeting the nonprod sandbox host viarestHost. Used by RTPO15's rest-provisioned-data-sync case; compiles against:javaonly (publicAblyRest).junit-jupiter-paramsadded touts/build.gradle.kts— enables the protocol-variant@ParameterizedTestin the integration tier (version managed by the JUnit BOM already on the test classpath).objects-mapping.md§14 — documents the integration REST-helper mapping in theuts-to-kotlinskill reference.Related
The V2 + nonprod-host alignment of the UTS source spec is tracked upstream in ably/specification#497 (the
provision_objects_via_resthost/shape and the objects integration client endpoints).Testing
Summary by CodeRabbit
New Features
Bug Fixes
Documentation