From e6f757c12d121664aab4090c4193cd544326dffb Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 29 Jul 2026 15:02:12 +0530 Subject: [PATCH 1/6] objects: UTS test-spec corrections from cross-SDK audit Corrections and clarifications to the objects UTS unit test specs, surfaced while translating them to ably-java and cross-checked against ably-js and ably-cocoa source. - RTO17-RTO18 (realtime_object.md): remove the "re-attach after detach" sync-event scenario. It is redundant with "re-sync on new ATTACHED" (identical onAttached -> new-sync path) and not portably expressible - an unsolicited server DETACHED transitions the channel to SUSPENDED (not DETACHED) in some SDKs. ably-js already folds it into "re-sync on new ATTACHED". - RTO18d (realtime_object.md, objects-features.md): remove the duplicate-listener UTS test and add an editor's note. Whether re-registering the same listener fires once or twice is a platform-idiomatic design choice (verified in source: ably-js and ably-cocoa append -> twice; ably-java de-duplicates by listener instance -> once), rooted in the general EventEmitter contract (RTE4). Not universally testable; each SDK pins its own. - Op-path LiveObjectUpdate portability (internal_live_counter.md, internal_live_map.md): note that the update returned by applyOperation may be observed via the return value OR the emitted update event / resulting state, so event-based / typed SDKs can satisfy the same requirement. - Read-after-send quiescence (path_object.md): add a poll_until barrier before the positive reads in RTPO13c5 / RTPO14 (inbound ops apply asynchronously). - objects_pool.md: add a NOTE documenting the SDK-internal "DETACHED/FAILED clears objects data; SUSPENDED retains" behaviour and why it is deliberately not a UTS requirement (no normative point; unobservable via the public API), guarded by SDK-local tests. - internal_live_counter_api.md: assert the null-means-omitted increment contract directly where null is not distinguishable from an omitted argument. --- specifications/objects-features.md | 25 +++++++ uts/objects/unit/internal_live_counter.md | 9 +++ uts/objects/unit/internal_live_counter_api.md | 3 + uts/objects/unit/internal_live_map.md | 10 +++ uts/objects/unit/objects_pool.md | 18 +++++ uts/objects/unit/path_object.md | 10 +++ uts/objects/unit/realtime_object.md | 71 ++++++++----------- 7 files changed, 106 insertions(+), 40 deletions(-) diff --git a/specifications/objects-features.md b/specifications/objects-features.md index 194703f19..a3236fedc 100644 --- a/specifications/objects-features.md +++ b/specifications/objects-features.md @@ -292,6 +292,31 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO18b2)` `SYNCED`: Indicates that the [RTO17](#RTO17) sync state has transitioned to `SYNCED` - `(RTO18c)` Registers the provided listener for the specified event - `(RTO18d)` If `on` is called more than once with the same listener and event, the listener is added multiple times to the listener registry. As such, if the same listener is registered twice and an event is emitted once, the listener will be invoked twice + > **EDITOR'S NOTE — SDKs diverge on this by design; it is not universally testable.** RTO18d + > restates the general `EventEmitter#on` contract **RTE4** (`features.md`, mirrored in + > `api-docstrings.md`). Registering the **same listener reference** twice for the same event is + > handled differently across Ably's own SDKs (verified in source): + > - **ably-js** (`eventemitter.ts`): `on` does a plain `listeners.push(listener)` with **no + > identity check** → **fires twice** (matches RTE4/RTO18d). + > - **ably-cocoa** (`ARTEventEmitter.m`): `on:` mints a *fresh* `ARTEventListener` wrapping the + > block on every call, and the registry's "AsSet" dedup is pointer-identity on that wrapper (the + > block itself is never the key), so two calls never collide → **fires twice**. + > - **ably-java** (`EventEmitter.java`): `on(event, Listener)` does `filters.put(listener, …)` into + > a map keyed by the listener *instance* (and the all-events `on(Listener)` is `contains`-guarded) + > → **de-duplicates → fires once**. ably-java's own Javadoc flags this as a deliberate, SDK-wide + > deviation from RTE4 — for a state-change listener it avoids the double-side-effect footgun the + > WHATWG DOM `addEventListener` also avoids by de-duplicating. + > + > This is a **design choice, not a language limitation**: the test passes the *same reference*, and + > reference identity is available in every one of these languages (JS `===`, ObjC pointer, Java + > identity) — js/cocoa simply choose to append, java chooses to dedupe. Because the SDKs genuinely + > diverge, the assertion cannot hold identically everywhere, so the corresponding UTS test + > (`RTO18d/duplicate-listener-0`) has been **removed** from the objects UTS suite. RTE4's "fires + > twice" is retained as the reference behaviour; SDKs that de-duplicate document it locally + > (ably-java tracks it in its `deviations.md`), and any SDK may pin its own behaviour in an + > SDK-local (non-UTS) test. + > Any move to make this *uniform* would be an ecosystem-wide RTE4 change (redefining `off`'s + > remove-one-vs-all semantics and closure identity) — out of scope here. - `(RTO18e)` When an event is emitted, all registered listeners for that event must be called with no arguments - `(RTO18f)` The client library may return a subscription object (or the idiomatic equivalent for the language) as a result of this operation: - `(RTO18f1)` The subscription object includes an `off` function diff --git a/uts/objects/unit/internal_live_counter.md b/uts/objects/unit/internal_live_counter.md index 73a36b46d..080db29b4 100644 --- a/uts/objects/unit/internal_live_counter.md +++ b/uts/objects/unit/internal_live_counter.md @@ -11,6 +11,15 @@ Tests the `InternalLiveCounter` CRDT data structure. InternalLiveCounter holds a Tests operate directly on InternalLiveCounter by calling `applyOperation()` and `replaceData()` with constructed messages. No channel or connection infrastructure is needed. +> **SDK portability note — op-path update return value.** Several tests below assert on the +> `LiveCounterUpdate` **returned by `applyOperation(msg, source)`** (`noop`, `update.amount`, +> `tombstone`, `objectMessage`). This models SDKs whose operation application returns the update +> synchronously. An SDK whose operation path returns only a success/failure boolean and instead +> delivers the update via the object's subscription/emit path (e.g. a statically-typed SDK — see +> `objects-features.md` RTTS) may satisfy the same requirement by asserting the **emitted update +> event** and/or the **resulting object state** — these are equivalent observables of the same +> `LiveCounterUpdate`. The sync path (`replaceData()`) returns the update in all SDKs. + ## Shared Helpers See `helpers/standard_test_pool.md` for `build_counter_inc`, `build_counter_create`, `build_object_delete`, `build_object_state`. diff --git a/uts/objects/unit/internal_live_counter_api.md b/uts/objects/unit/internal_live_counter_api.md index 3cf71d535..53f2dc7ac 100644 --- a/uts/objects/unit/internal_live_counter_api.md +++ b/uts/objects/unit/internal_live_counter_api.md @@ -237,6 +237,9 @@ The `null` row applies only where `null` is passable and distinguishable from an argument (e.g. a boxed `Integer` in Java). In SDKs whose signature makes null equivalent to "omitted" (e.g. `increment(amount?: number)` in JavaScript, where a nullish amount takes the default of 1), the row is not applicable — see the pseudocode conventions in `uts/README.md`. +Such SDKs should instead assert the equivalence directly — `increment(null)` succeeds and +increments by the default of 1 — pinning the null-means-omitted contract so a later signature +or coalescing change surfaces as a conscious decision. ### Setup ```pseudo diff --git a/uts/objects/unit/internal_live_map.md b/uts/objects/unit/internal_live_map.md index 348601398..08fcd07d8 100644 --- a/uts/objects/unit/internal_live_map.md +++ b/uts/objects/unit/internal_live_map.md @@ -11,6 +11,16 @@ Tests the `InternalLiveMap` LWW-map CRDT data structure. InternalLiveMap holds a Tests operate directly on InternalLiveMap by calling `applyOperation()` and `replaceData()` with constructed messages. +> **SDK portability note — op-path update return value.** Several tests below assert on the +> `LiveMapUpdate` **returned by `applyOperation(msg, source)`** (`noop`, the per-key `update` diff +> such as `{ "name": "updated"/"removed" }`, `tombstone`, `objectMessage`). This models SDKs whose +> operation application returns the update synchronously. An SDK whose operation path returns only a +> success/failure boolean and instead delivers the update via the object's subscription/emit path +> (e.g. a statically-typed SDK — see `objects-features.md` RTTS) may satisfy the same requirement by +> asserting the **emitted update event** and/or the **resulting map state** — these are equivalent +> observables of the same `LiveMapUpdate`. The sync path (`replaceData()`) returns the update in all +> SDKs. + ## Shared Helpers See `helpers/standard_test_pool.md` for builder functions. diff --git a/uts/objects/unit/objects_pool.md b/uts/objects/unit/objects_pool.md index ada2c79c9..d0a1d3662 100644 --- a/uts/objects/unit/objects_pool.md +++ b/uts/objects/unit/objects_pool.md @@ -119,6 +119,24 @@ ASSERT updates[0].objectMessage IS null --- +> **NOTE — channel DETACHED/FAILED clears objects data (SDK-internal; intentionally not a UTS test).** +> When the channel transitions to DETACHED or FAILED, the objects data is cleared **without emitting +> update events** (the current data is unknown in those states); a SUSPENDED channel **retains** its +> data. This is implemented in both reference SDKs (ably-js `RealtimeObject.actOnChannelState` → +> `objectsPool.clearObjectsData(false)` + clear SyncObjectsPool; ably-java +> `DefaultRealtimeObject.handleStateChange`), but it is **deliberately not a shared UTS requirement**: +> it has no normative `RTO` point, and it is not observable through the public API — access on a +> DETACHED channel throws (RTO25b), and on re-attach the RTO5c sync **replaces** the pool regardless of +> whether detach cleared it, so an SDK that skipped the clear would still be observably correct. It is a +> defensive internal cleanup, so each SDK guards it with its own **SDK-local** test rather than a UTS +> spec test. ably-java pins it in `DefaultRealtimeObjectChannelStateTest` (asserts the pool is cleared +> after DETACHED/FAILED and retained after SUSPENDED, driving `handleStateChange` and inspecting the +> pool directly); other SDKs may add an equivalent local test (e.g. ably-js in +> `test/realtime/liveobjects.test.js`). Referenced from the "no re-attach-after-detach scenario" note +> in `realtime_object.md`. + +--- + ## RTO5 - OBJECT_SYNC complete sequence **Test ID**: `objects/unit/RTO5/sync-complete-sequence-0` diff --git a/uts/objects/unit/path_object.md b/uts/objects/unit/path_object.md index 8a4000a4e..1a55aed8d 100644 --- a/uts/objects/unit/path_object.md +++ b/uts/objects/unit/path_object.md @@ -574,6 +574,11 @@ ASSERT result["profile"]["prefs"]["theme"] == "dark" mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) + +# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages +# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's +# read-after-send conventions otherwise cover only negative assertions.) +poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` ### Test Steps @@ -626,6 +631,11 @@ ASSERT root.get("score").compact() == 100 mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) + +# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages +# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's +# read-after-send conventions otherwise cover only negative assertions.) +poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` ### Test Steps diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 411fd9126..5df537725 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -545,35 +545,25 @@ ASSERT events CONTAINS_IN_ORDER ["SYNCING", "SYNCED"] --- -## RTO18d - Duplicate listener registered twice fires twice - -**Test ID**: `objects/unit/RTO18d/duplicate-listener-0` - -**Spec requirement:** If same listener registered twice, it is invoked twice per event. - -### Setup -```pseudo -{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") -call_count = 0 -listener = () => { call_count++ } -channel.object.on(SYNCED, listener) -channel.object.on(SYNCED, listener) -``` - -### Test Steps -```pseudo -mock_ws.send_to_client(ProtocolMessage( - action: ATTACHED, channel: "test", channelSerial: "sync2:cursor", flags: HAS_OBJECTS -)) -mock_ws.send_to_client(build_object_sync_message("test", "sync2:", STANDARD_POOL_OBJECTS)) - -poll_until(call_count >= 2, timeout: 5s) -``` - -### Assertions -```pseudo -ASSERT call_count == 2 -``` +## RTO18d - Duplicate listener registration — REMOVED (platform-idiomatic, not universally testable) + +The former test `objects/unit/RTO18d/duplicate-listener-0` (registering the same listener twice → +invoked twice) has been **removed** from the UTS suite: the behaviour is contingent on each SDK's +listener-registration idiom, so it cannot pass identically across SDKs (the UTS contract). + +Registering the **same listener reference** twice for the same event diverges by design across SDKs +(verified in source): +- **ably-js:** `on` does a plain `listeners.push(listener)` with no identity check → fires **twice** + (matches RTE4 / RTO18d). +- **ably-cocoa:** `on:` mints a fresh `ARTEventListener` per call; the "AsSet" dedup is pointer-identity + on that wrapper (the block is never the key) → fires **twice**. +- **ably-java:** `on(event, Listener)` keys the registry by the listener *instance* → de-duplicates → + fires **once** (a deliberate, SDK-wide choice its `EventEmitter` documents as a spec deviation). + +This is a design choice, not a language limitation — the same reference is detectable in all three +languages; js/cocoa append, java dedupes. Because the SDKs genuinely diverge, the assertion can't hold +identically everywhere. See the RTO18d editor's note in `objects-features.md`; an SDK that wants to pin +its own behaviour should do so in an SDK-local (non-UTS) test. --- @@ -1538,17 +1528,18 @@ scenarios = [ }, expected_events: ["SYNCING", "SYNCED"] }, - { - name: "re-attach after detach", - trigger: () => { - mock_ws.send_to_client(ProtocolMessage(action: DETACHED, channel: "test")) - mock_ws.send_to_client(ProtocolMessage( - action: ATTACHED, channel: "test", channelSerial: "sync2:cursor", flags: HAS_OBJECTS - )) - mock_ws.send_to_client(build_object_sync_message("test", "sync2:", STANDARD_POOL_OBJECTS)) - }, - expected_events: ["SYNCING", "SYNCED"] - }, + // NOTE: there is intentionally NO "re-attach after detach" scenario here. At the objects layer, + // a detach emits no sync events (the detached/failed handler clears objects data WITHOUT + // emitting), and the re-attach drives the SAME onAttached -> new-sync path (RTO4c) as the + // "re-sync on new ATTACHED" scenario below — so it is redundant for a sync-EVENT test. It is also + // not portably expressible: in some SDKs an unsolicited server DETACHED transitions the channel to + // SUSPENDED (not DETACHED), so the fixture cannot be written identically everywhere. + // + // A SEPARATE, SDK-internal behaviour lives near here: on a channel DETACHED/FAILED the objects data + // is cleared (no events emitted), while SUSPENDED retains it. It is deliberately NOT a UTS test (no + // normative RTO point; not observable through the public API — access on DETACHED throws per RTO25b, + // and re-sync replaces the pool regardless), so each SDK guards it with an SDK-local test. See the + // NOTE in objects_pool.md. { name: "re-sync on new ATTACHED", trigger: () => { From 65e6dd5ec756ddfa13779f1c659dc5fc57e5274e Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 29 Jul 2026 18:29:58 +0530 Subject: [PATCH 2/6] =?UTF-8?q?objects:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20restore=20RTO18d=20test,=20add=20RTO27=20spec=20poi?= =?UTF-8?q?nt=20+=20UTS=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR review on the objects UTS cross-SDK corrections. - RTO18d (realtime_object.md, objects-features.md): restore the duplicate-listener UTS test (asserting the RTE4 "fires twice" contract) instead of removing it, and drop the editor's note from the spec. The UTS tests the spec's assertion; SDKs that de-duplicate listeners by instance (e.g. ably-java) may assert fires-once via an optional note on the test — the deviation stays with the SDK, not the UTS. - RTO27 (objects-features.md, NEW): add an explicit normative point for the objects-data lifecycle on channel state transitions — on DETACHED/FAILED clear every pooled object's data without emitting update events (and clear the SyncObjectsPool); on SUSPENDED retain it. Placed in the channel-lifecycle cluster (after RTO5) and cross-referenced from RTO20e1. Closes a previously-unspecified gap both reference SDKs already implement (ably-js RealtimeObject.actOnChannelState, ably-java DefaultRealtimeObject.handleStateChange). - RTO27 UTS test (realtime_object.md): add a white-box test that drives the internal channel-state handler directly and inspects the ObjectsPool (mirroring the internal_live_counter / internal_live_map tests), so the clear (not observable via the public API) and SUSPENDED (a connection-level state) are both testable at the unit tier. Reframe the objects_pool.md / realtime_object.md notes to reference RTO27. - path_object.md: reword the RTPO13c5 / RTPO14 read-after-send quiescence barrier to clarify it guards an INBOUND server OBJECT message (no apply-on-ack point to await), not a local publish. --- specifications/objects-features.md | 30 ++----- uts/objects/unit/objects_pool.md | 24 +++--- uts/objects/unit/path_object.md | 14 ++-- uts/objects/unit/realtime_object.md | 119 ++++++++++++++++++++++------ 4 files changed, 117 insertions(+), 70 deletions(-) diff --git a/specifications/objects-features.md b/specifications/objects-features.md index a3236fedc..9a078ef64 100644 --- a/specifications/objects-features.md +++ b/specifications/objects-features.md @@ -192,6 +192,11 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO5c5)` The `bufferedObjectOperations` list must be cleared - `(RTO5c9)` The `appliedOnAckSerials` set ([RTO7b](#RTO7b)) must be cleared. A state sync causes the channel's LiveObjects data to be replaced, so after a state sync the `appliedOnAckSerials` no longer accurately describes which operations have been applied to the channel's LiveObjects data - `(RTO5c8)` The [RTO17](#RTO17) sync state must transition to `SYNCED` +- `(RTO27)` When the channel transitions to a state other than `ATTACHED`, the client library must manage the stored objects data as follows (for the effect of these transitions on an in-progress `publishAndApply`, see [RTO20e1](#RTO20e1)): + - `(RTO27a)` When the channel transitions to the `DETACHED` or `FAILED` state, the current state of the objects data can no longer be known, so the client library must: + - `(RTO27a1)` For every object in the internal `ObjectsPool`, clear its internal data, resetting it to the zero value for its type (an empty map, or a counter with value `0`), without emitting any `LiveObjectUpdate` events. The objects themselves remain in the `ObjectsPool`; only their data is cleared + - `(RTO27a2)` The `SyncObjectsPool` must be cleared + - `(RTO27b)` When the channel transitions to the `SUSPENDED` state, the client library must retain the stored objects data unchanged, since the connection may still recover and the retained data remains a valid best-effort local copy - `(RTO6)` Certain object operations may require creating a new object if one does not already exist in the internal `ObjectsPool` for the given `objectId`. This can be done as follows: - `(RTO6a)` If an object with `objectId` exists in `ObjectsPool`, do not create a new object - `(RTO6b)` The expected type of the object can be inferred from the provided `objectId`: @@ -292,31 +297,6 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO18b2)` `SYNCED`: Indicates that the [RTO17](#RTO17) sync state has transitioned to `SYNCED` - `(RTO18c)` Registers the provided listener for the specified event - `(RTO18d)` If `on` is called more than once with the same listener and event, the listener is added multiple times to the listener registry. As such, if the same listener is registered twice and an event is emitted once, the listener will be invoked twice - > **EDITOR'S NOTE — SDKs diverge on this by design; it is not universally testable.** RTO18d - > restates the general `EventEmitter#on` contract **RTE4** (`features.md`, mirrored in - > `api-docstrings.md`). Registering the **same listener reference** twice for the same event is - > handled differently across Ably's own SDKs (verified in source): - > - **ably-js** (`eventemitter.ts`): `on` does a plain `listeners.push(listener)` with **no - > identity check** → **fires twice** (matches RTE4/RTO18d). - > - **ably-cocoa** (`ARTEventEmitter.m`): `on:` mints a *fresh* `ARTEventListener` wrapping the - > block on every call, and the registry's "AsSet" dedup is pointer-identity on that wrapper (the - > block itself is never the key), so two calls never collide → **fires twice**. - > - **ably-java** (`EventEmitter.java`): `on(event, Listener)` does `filters.put(listener, …)` into - > a map keyed by the listener *instance* (and the all-events `on(Listener)` is `contains`-guarded) - > → **de-duplicates → fires once**. ably-java's own Javadoc flags this as a deliberate, SDK-wide - > deviation from RTE4 — for a state-change listener it avoids the double-side-effect footgun the - > WHATWG DOM `addEventListener` also avoids by de-duplicating. - > - > This is a **design choice, not a language limitation**: the test passes the *same reference*, and - > reference identity is available in every one of these languages (JS `===`, ObjC pointer, Java - > identity) — js/cocoa simply choose to append, java chooses to dedupe. Because the SDKs genuinely - > diverge, the assertion cannot hold identically everywhere, so the corresponding UTS test - > (`RTO18d/duplicate-listener-0`) has been **removed** from the objects UTS suite. RTE4's "fires - > twice" is retained as the reference behaviour; SDKs that de-duplicate document it locally - > (ably-java tracks it in its `deviations.md`), and any SDK may pin its own behaviour in an - > SDK-local (non-UTS) test. - > Any move to make this *uniform* would be an ecosystem-wide RTE4 change (redefining `off`'s - > remove-one-vs-all semantics and closure identity) — out of scope here. - `(RTO18e)` When an event is emitted, all registered listeners for that event must be called with no arguments - `(RTO18f)` The client library may return a subscription object (or the idiomatic equivalent for the language) as a result of this operation: - `(RTO18f1)` The subscription object includes an `off` function diff --git a/uts/objects/unit/objects_pool.md b/uts/objects/unit/objects_pool.md index d0a1d3662..1f77c4412 100644 --- a/uts/objects/unit/objects_pool.md +++ b/uts/objects/unit/objects_pool.md @@ -119,21 +119,15 @@ ASSERT updates[0].objectMessage IS null --- -> **NOTE — channel DETACHED/FAILED clears objects data (SDK-internal; intentionally not a UTS test).** -> When the channel transitions to DETACHED or FAILED, the objects data is cleared **without emitting -> update events** (the current data is unknown in those states); a SUSPENDED channel **retains** its -> data. This is implemented in both reference SDKs (ably-js `RealtimeObject.actOnChannelState` → -> `objectsPool.clearObjectsData(false)` + clear SyncObjectsPool; ably-java -> `DefaultRealtimeObject.handleStateChange`), but it is **deliberately not a shared UTS requirement**: -> it has no normative `RTO` point, and it is not observable through the public API — access on a -> DETACHED channel throws (RTO25b), and on re-attach the RTO5c sync **replaces** the pool regardless of -> whether detach cleared it, so an SDK that skipped the clear would still be observably correct. It is a -> defensive internal cleanup, so each SDK guards it with its own **SDK-local** test rather than a UTS -> spec test. ably-java pins it in `DefaultRealtimeObjectChannelStateTest` (asserts the pool is cleared -> after DETACHED/FAILED and retained after SUSPENDED, driving `handleStateChange` and inspecting the -> pool directly); other SDKs may add an equivalent local test (e.g. ably-js in -> `test/realtime/liveobjects.test.js`). Referenced from the "no re-attach-after-detach scenario" note -> in `realtime_object.md`. +> **NOTE — channel DETACHED/FAILED clears objects data (normative: [RTO27](../../../specifications/objects-features.md#RTO27)).** +> On a channel transition to DETACHED or FAILED the objects data is cleared **without emitting update +> events** (RTO27a); a SUSPENDED channel **retains** its data (RTO27b) (ably-js +> `RealtimeObject.actOnChannelState` → `objectsPool.clearObjectsData(false)`; ably-java +> `DefaultRealtimeObject.handleStateChange`). This is exercised **white-box** by the **RTO27** test in +> `realtime_object.md`: it drives the internal channel-state handler directly and inspects the internal +> `ObjectsPool`, because the behaviour is not reachable black-box (access after DETACHED throws per +> RTO25b, and SUSPENDED is a connection-level state a channel-level mock cannot drive). SDKs may +> additionally pin it in a local test (ably-java: `DefaultRealtimeObjectChannelStateTest`). --- diff --git a/uts/objects/unit/path_object.md b/uts/objects/unit/path_object.md index 1a55aed8d..b7c46e003 100644 --- a/uts/objects/unit/path_object.md +++ b/uts/objects/unit/path_object.md @@ -575,9 +575,10 @@ mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) -# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages -# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's -# read-after-send conventions otherwise cover only negative assertions.) +# Quiescence barrier: this is an INBOUND server OBJECT message (send_to_client), not a locally +# published operation — so there is no publish-completion / apply-on-ack point to await here. SDKs +# may apply inbound OBJECT messages asynchronously, so wait until the MAP_SET has been applied before +# this POSITIVE read. (The spec's read-after-send conventions otherwise cover only negative assertions.) poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` @@ -632,9 +633,10 @@ mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) -# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages -# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's -# read-after-send conventions otherwise cover only negative assertions.) +# Quiescence barrier: this is an INBOUND server OBJECT message (send_to_client), not a locally +# published operation — so there is no publish-completion / apply-on-ack point to await here. SDKs +# may apply inbound OBJECT messages asynchronously, so wait until the MAP_SET has been applied before +# this POSITIVE read. (The spec's read-after-send conventions otherwise cover only negative assertions.) poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 5df537725..20382f948 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -545,25 +545,42 @@ ASSERT events CONTAINS_IN_ORDER ["SYNCING", "SYNCED"] --- -## RTO18d - Duplicate listener registration — REMOVED (platform-idiomatic, not universally testable) - -The former test `objects/unit/RTO18d/duplicate-listener-0` (registering the same listener twice → -invoked twice) has been **removed** from the UTS suite: the behaviour is contingent on each SDK's -listener-registration idiom, so it cannot pass identically across SDKs (the UTS contract). - -Registering the **same listener reference** twice for the same event diverges by design across SDKs -(verified in source): -- **ably-js:** `on` does a plain `listeners.push(listener)` with no identity check → fires **twice** - (matches RTE4 / RTO18d). -- **ably-cocoa:** `on:` mints a fresh `ARTEventListener` per call; the "AsSet" dedup is pointer-identity - on that wrapper (the block is never the key) → fires **twice**. -- **ably-java:** `on(event, Listener)` keys the registry by the listener *instance* → de-duplicates → - fires **once** (a deliberate, SDK-wide choice its `EventEmitter` documents as a spec deviation). - -This is a design choice, not a language limitation — the same reference is detectable in all three -languages; js/cocoa append, java dedupes. Because the SDKs genuinely diverge, the assertion can't hold -identically everywhere. See the RTO18d editor's note in `objects-features.md`; an SDK that wants to pin -its own behaviour should do so in an SDK-local (non-UTS) test. +## RTO18d - Duplicate listener registered twice fires twice + +**Test ID**: `objects/unit/RTO18d/duplicate-listener-0` + +**Spec requirement:** If same listener registered twice, it is invoked twice per event. + +> **OPTIONAL for SDKs that de-duplicate listeners by instance.** RTO18d restates the general +> `EventEmitter#on` contract (RTE4): registering the same listener reference twice adds it twice, so it +> fires twice. Some SDKs deliberately de-duplicate listeners keyed by instance (e.g. ably-java's +> `EventEmitter` documents this as an SDK-wide deviation from RTE4). Such an SDK may instead assert +> `call_count == 1` here — or skip this test — and pin its own behaviour; the assertion below is the +> RTE4 reference behaviour. + +### Setup +```pseudo +{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") +call_count = 0 +listener = () => { call_count++ } +channel.object.on(SYNCED, listener) +channel.object.on(SYNCED, listener) +``` + +### Test Steps +```pseudo +mock_ws.send_to_client(ProtocolMessage( + action: ATTACHED, channel: "test", channelSerial: "sync2:cursor", flags: HAS_OBJECTS +)) +mock_ws.send_to_client(build_object_sync_message("test", "sync2:", STANDARD_POOL_OBJECTS)) + +poll_until(call_count >= 2, timeout: 5s) +``` + +### Assertions +```pseudo +ASSERT call_count == 2 +``` --- @@ -1535,11 +1552,10 @@ scenarios = [ // not portably expressible: in some SDKs an unsolicited server DETACHED transitions the channel to // SUSPENDED (not DETACHED), so the fixture cannot be written identically everywhere. // - // A SEPARATE, SDK-internal behaviour lives near here: on a channel DETACHED/FAILED the objects data - // is cleared (no events emitted), while SUSPENDED retains it. It is deliberately NOT a UTS test (no - // normative RTO point; not observable through the public API — access on DETACHED throws per RTO25b, - // and re-sync replaces the pool regardless), so each SDK guards it with an SDK-local test. See the - // NOTE in objects_pool.md. + // A SEPARATE behaviour lives near here: on a channel DETACHED/FAILED the objects data is cleared + // (no events emitted), while SUSPENDED retains it. This is normative (RTO27) and is covered by the + // RTO27 white-box test at the end of this file (it drives the internal channel-state handler directly + // and inspects the pool, since the behaviour is not reachable black-box). See objects_pool.md. { name: "re-sync on new ATTACHED", trigger: () => { @@ -1580,3 +1596,58 @@ FOR scenario IN scenarios: ASSERT events == scenario.expected_events ``` + +--- + +## RTO27 - Objects data lifecycle on channel state transitions + +**Test ID**: `objects/unit/RTO27/channel-state-data-lifecycle-0` + +**Spec requirement:** On a channel transition to `DETACHED`/`FAILED`, every object's data is cleared +without emitting update events (RTO27a); on `SUSPENDED`, the data is retained (RTO27b). + +**White-box test.** Like the `internal_live_counter` / `internal_live_map` tests (which call +`applyOperation` directly), this drives the RealtimeObject's internal channel-state handler directly +and inspects the internal `ObjectsPool`. A direct call is required because the behaviour is not +reachable black-box: access after `DETACHED`/`FAILED` throws (RTO25b), and `SUSPENDED` is a +connection-level state that a channel-level mock cannot drive. The abstract symbols map as follows: +- `channel.object.handle_channel_state(state)` → the SDK's channel-state handler (ably-js + `RealtimeObject.actOnChannelState(state)`, ably-java `DefaultRealtimeObject.handleStateChange(state, false)`). +- `channel.object.objects_pool` → the internal `ObjectsPool` (ably-js `_objectsPool`, ably-java `objectsPool`). + +### RTO27a - DETACHED / FAILED clears data without emitting update events + +```pseudo +FOR state IN [DETACHED, FAILED]: + { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") + pool = channel.object.objects_pool # internal ObjectsPool (RTO3) + + # sanity: STANDARD_POOL_OBJECTS has been materialised + ASSERT "name" IN pool["root"].data + ASSERT pool["counter:score@1000"].value == 100 + + # RTO27a1: no update events must be emitted during the clear + updates = [] + pool["root"].subscribe((update) => updates.append(update)) + + channel.object.handle_channel_state(state) # internal channel-state handler (RTO27) + + # RTO27a1: every object's data is cleared; the objects themselves remain in the pool + ASSERT pool["root"].data == {} + ASSERT "counter:score@1000" IN pool + ASSERT pool["counter:score@1000"].value == 0 + ASSERT updates.length == 0 +``` + +### RTO27b - SUSPENDED retains data + +```pseudo +{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") +pool = channel.object.objects_pool + +channel.object.handle_channel_state(SUSPENDED) # RTO27b: no-op for objects data + +# RTO27b: data retained unchanged +ASSERT "name" IN pool["root"].data +ASSERT pool["counter:score@1000"].value == 100 +``` From 85b47680bf42a378ba84dbb3ff34fa0b5b4f2bbd Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 29 Jul 2026 20:44:35 +0530 Subject: [PATCH 3/6] objects/uts: drop redundant detach-clear note, document identifier-naming conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the RTO27 work and the PR review. - objects_pool.md: remove the DETACHED/FAILED detach-clear NOTE. It was a stopgap for a missing spec point; now that RTO27 is normative and has its own white-box test in realtime_object.md, the note is redundant (and was a RealtimeObject concern, not a bare ObjectsPool one). - realtime_object.md: rename the RTO27 test's abstract white-box symbols to the UTS camelCase convention (objects_pool -> objectsPool, handle_channel_state -> processChannelState); drop the dangling "See objects_pool.md" pointer and the redundant RTO27 cross-reference from the no-re-attach-after-detach note, trimming that note to its two load-bearing reasons (redundant + not portably expressible); extend the header spec points to RTO22-RTO27 to match the file's contents. - docs/writing-test-specs.md, README.md: add an "Identifier Naming" convention section (spec surface mirrors the specification's names — PascalCase types, camelCase fields/methods; behaviours the spec describes but doesn't name get a camelCase coinage; snake_case is reserved for test-harness constructs; wire/enum values keep protocol/spec casing). Validated against objects/realtime/rest UTS specs and the features.md/protocol.md parent specs. --- uts/README.md | 8 ++++ uts/docs/writing-test-specs.md | 67 +++++++++++++++++++++++++++++ uts/objects/unit/objects_pool.md | 12 ------ uts/objects/unit/realtime_object.md | 28 +++++------- 4 files changed, 85 insertions(+), 30 deletions(-) diff --git a/uts/README.md b/uts/README.md index 4b8d6bc8c..8cc4d9b9d 100644 --- a/uts/README.md +++ b/uts/README.md @@ -87,6 +87,14 @@ ASSERT error.code == 40160 AWAIT_STATE client.connection.state == ConnectionState.connected ``` +**Identifier naming.** Identifiers that name SDK/spec surface mirror the specification's own names and +casing — `PascalCase` types (`ObjectsPool`), `camelCase` fields and methods (`bufferedObjectOperations`, +`applyOperation`) — so they grep straight back to the spec. Behaviours the spec describes but does not +name get a `camelCase` coinage in the same style (`processAttached`, `processChannelState`), never +`snake_case`. `snake_case` is reserved for test-harness constructs (`setup_synced_channel`, `mock_ws`, +`send_to_client`, `build_*`, `poll_until`). Spec *file* names are `snake_case` (`objects_pool.md`) — +unrelated to symbol casing. Full rules: [docs/writing-test-specs.md](docs/writing-test-specs.md#identifier-naming). + Pseudocode maps to language idioms rather than prescribing exact syntax: - **Absent values**: `== null` means the language-appropriate "absent" value — `undefined` in diff --git a/uts/docs/writing-test-specs.md b/uts/docs/writing-test-specs.md index e60f6fd67..aa403e6b2 100644 --- a/uts/docs/writing-test-specs.md +++ b/uts/docs/writing-test-specs.md @@ -508,6 +508,73 @@ Tests that all REST requests include the `Ably-Agent` header with correct format ## Pseudocode Conventions +### Identifier Naming + +Every identifier in pseudocode is either **spec surface** (an SDK/spec name, cases 1–2 below) or +**test harness** (scaffolding, case 3), and its **casing is the visual signal** of which. Getting this +right keeps the spec surface greppable straight back to the specification and keeps test scaffolding +visually distinct from it. + +**1. Spec surface — mirror the specification's own names exactly.** Anything that names an SDK type, +field, property, or method that a feature specification (`features.md`, `objects-features.md`, …) +already defines must use the **identical name and casing** the spec uses: + +- **Types / concepts → `PascalCase`**: `ObjectsPool`, `SyncObjectsPool`, `InternalLiveMap`, + `InternalLiveCounter`, `Connection`. +- **Fields / properties → `camelCase`**: `bufferedObjectOperations`, `appliedOnAckSerials`, + `clearTimeserial`, `siteTimeserials`, `createOperationIsMerged`. +- **Methods → the spec's member, written as a call**: the spec's `Type#method` form (e.g. + `InternalLiveMap#set`, `RealtimeObject#on`) is written `object.method(...)`; bare method names + (`applyOperation`, `replaceData`) match the spec verbatim. + +Do not rename, abbreviate, or re-case a spec identifier — a reader must be able to grep it out of the +specification unchanged. + +**2. Behaviour the specification describes but does not name.** Some clauses describe an action without +giving it a method name (e.g. RTO4: *"when a channel `ATTACHED` `ProtocolMessage` is received, the +client library must …"* — no method is named). When a white-box test needs to invoke that behaviour +directly, coin a name **in the specification's own style — `camelCase`, never `snake_case`** — and use +it consistently across the suite. Established coinages: `processAttached`, `processObjectSync`, +`processObjectMessage`, `syncState`, `processChannelState`. These are UTS conventions rather than spec +names, but they read as though the spec could have named them. + +**3. Test-harness constructs → `snake_case`.** Everything that is scaffolding rather than SDK/spec +surface — fixtures, mocks, builders, polling/flush helpers — is `snake_case`, so it is instantly +distinguishable from the spec surface: `setup_synced_channel`, `mock_ws`, `send_to_client`, +`build_object_message`, `build_counter_inc`, `install_mock`, `respond_with_success`, `poll_until`, +`process_pending_events`, `encode_uri_component`. + +**Internal (white-box) access** uses the spec's internal name, `camelCase`d: the internal `ObjectsPool` +(RTO3) is reached as `object.objectsPool`; internal fields keep their spec names +(`bufferedObjectOperations`, `appliedOnAckSerials`). How each SDK exposes these is covered in the +derived-test mapping docs. + +**Wire and enum values follow the protocol/spec — not these two buckets.** Identifiers that name +wire-format data or enum members come from `protocol.md` / `features.md` and are matched verbatim; they +are **not** harness, even when `snake_case`: + +- **Wire fields & query parameters** — mostly `camelCase` (`channelSerial`, `connectionId`, + `msgSerial`), with a few `snake_case` (the `request_id` query parameter, RSC7c). +- **Enum members** — the spec's enum casing, typically `SCREAMING_SNAKE_CASE` + (the `ChannelMode.PRESENCE_SUBSCRIBE` enum member / `TR3` flag, also written `presence_subscribe` in + some prose; `SYNCED`; `DETACHED`). Render these per the *Enum values* note in the UTS README. + +**File names are not identifiers.** Spec *files* are `snake_case` `.md` (`objects_pool.md`, +`realtime_object.md`); this is unrelated to symbol casing. The `ObjectsPool` type stays `PascalCase` +even though it is documented in `objects_pool.md`. + +```pseudo +# GOOD — spec surface mirrors the spec; harness is snake_case +{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") # harness → snake_case +pool = channel.object.objectsPool # ObjectsPool (RTO3) → camelCase accessor +update = counter.applyOperation(msg, source: CHANNEL) # spec method, verbatim +ASSERT counter.bufferedObjectOperations.length == 0 # spec field, verbatim + +# BAD — spec surface in snake_case: looks like harness, no longer greps back to the spec +pool = channel.object.objects_pool +update = counter.apply_operation(msg) +``` + ### URI Path Component Encoding Use `encode_uri_component()` for any variable path segment or query parameter in URL assertions. This is defined in the UTS README. Always use exact equality (`==`) for path assertions, not `CONTAINS`. diff --git a/uts/objects/unit/objects_pool.md b/uts/objects/unit/objects_pool.md index 1f77c4412..ada2c79c9 100644 --- a/uts/objects/unit/objects_pool.md +++ b/uts/objects/unit/objects_pool.md @@ -119,18 +119,6 @@ ASSERT updates[0].objectMessage IS null --- -> **NOTE — channel DETACHED/FAILED clears objects data (normative: [RTO27](../../../specifications/objects-features.md#RTO27)).** -> On a channel transition to DETACHED or FAILED the objects data is cleared **without emitting update -> events** (RTO27a); a SUSPENDED channel **retains** its data (RTO27b) (ably-js -> `RealtimeObject.actOnChannelState` → `objectsPool.clearObjectsData(false)`; ably-java -> `DefaultRealtimeObject.handleStateChange`). This is exercised **white-box** by the **RTO27** test in -> `realtime_object.md`: it drives the internal channel-state handler directly and inspects the internal -> `ObjectsPool`, because the behaviour is not reachable black-box (access after DETACHED throws per -> RTO25b, and SUSPENDED is a connection-level state a channel-level mock cannot drive). SDKs may -> additionally pin it in a local test (ably-java: `DefaultRealtimeObjectChannelStateTest`). - ---- - ## RTO5 - OBJECT_SYNC complete sequence **Test ID**: `objects/unit/RTO5/sync-complete-sequence-0` diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 20382f948..60ffaef8c 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -1,6 +1,6 @@ # RealtimeObject Tests -Spec points: `RTO2`, `RTO10`, `RTO15`, `RTO17`–`RTO20`, `RTO22`–`RTO26` +Spec points: `RTO2`, `RTO10`, `RTO15`, `RTO17`–`RTO20`, `RTO22`–`RTO27` ## Test Type Unit test with mocked WebSocket client @@ -1545,17 +1545,9 @@ scenarios = [ }, expected_events: ["SYNCING", "SYNCED"] }, - // NOTE: there is intentionally NO "re-attach after detach" scenario here. At the objects layer, - // a detach emits no sync events (the detached/failed handler clears objects data WITHOUT - // emitting), and the re-attach drives the SAME onAttached -> new-sync path (RTO4c) as the - // "re-sync on new ATTACHED" scenario below — so it is redundant for a sync-EVENT test. It is also - // not portably expressible: in some SDKs an unsolicited server DETACHED transitions the channel to - // SUSPENDED (not DETACHED), so the fixture cannot be written identically everywhere. - // - // A SEPARATE behaviour lives near here: on a channel DETACHED/FAILED the objects data is cleared - // (no events emitted), while SUSPENDED retains it. This is normative (RTO27) and is covered by the - // RTO27 white-box test at the end of this file (it drives the internal channel-state handler directly - // and inspects the pool, since the behaviour is not reachable black-box). See objects_pool.md. + // No "re-attach after detach" scenario, deliberately: it is redundant with "re-sync on new + // ATTACHED" below (same onAttached -> new-sync path, RTO4c), and not portably expressible — an + // unsolicited server DETACHED transitions to SUSPENDED (not DETACHED) in some SDKs. { name: "re-sync on new ATTACHED", trigger: () => { @@ -1611,16 +1603,16 @@ without emitting update events (RTO27a); on `SUSPENDED`, the data is retained (R and inspects the internal `ObjectsPool`. A direct call is required because the behaviour is not reachable black-box: access after `DETACHED`/`FAILED` throws (RTO25b), and `SUSPENDED` is a connection-level state that a channel-level mock cannot drive. The abstract symbols map as follows: -- `channel.object.handle_channel_state(state)` → the SDK's channel-state handler (ably-js +- `channel.object.processChannelState(state)` → the SDK's channel-state handler (ably-js `RealtimeObject.actOnChannelState(state)`, ably-java `DefaultRealtimeObject.handleStateChange(state, false)`). -- `channel.object.objects_pool` → the internal `ObjectsPool` (ably-js `_objectsPool`, ably-java `objectsPool`). +- `channel.object.objectsPool` → the internal `ObjectsPool` (RTO3) (ably-js `_objectsPool`, ably-java `objectsPool`). ### RTO27a - DETACHED / FAILED clears data without emitting update events ```pseudo FOR state IN [DETACHED, FAILED]: { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") - pool = channel.object.objects_pool # internal ObjectsPool (RTO3) + pool = channel.object.objectsPool # internal ObjectsPool (RTO3) # sanity: STANDARD_POOL_OBJECTS has been materialised ASSERT "name" IN pool["root"].data @@ -1630,7 +1622,7 @@ FOR state IN [DETACHED, FAILED]: updates = [] pool["root"].subscribe((update) => updates.append(update)) - channel.object.handle_channel_state(state) # internal channel-state handler (RTO27) + channel.object.processChannelState(state) # internal channel-state handler (RTO27) # RTO27a1: every object's data is cleared; the objects themselves remain in the pool ASSERT pool["root"].data == {} @@ -1643,9 +1635,9 @@ FOR state IN [DETACHED, FAILED]: ```pseudo { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") -pool = channel.object.objects_pool +pool = channel.object.objectsPool -channel.object.handle_channel_state(SUSPENDED) # RTO27b: no-op for objects data +channel.object.processChannelState(SUSPENDED) # RTO27b: no-op for objects data # RTO27b: data retained unchanged ASSERT "name" IN pool["root"].data From 5b26b530b75ec35475c06863ff5808963fb994b0 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Thu, 30 Jul 2026 14:48:21 +0530 Subject: [PATCH 4/6] objects/uts: resolve op-path portability (comment #2) + document deviation recording conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - internal_live_counter.md, internal_live_map.md: remove the "SDK portability note" blocks. ably-java's op path now returns the LiveObjectUpdate (RTLC9g/RTLM7f), so the boolean-return divergence they described no longer exists; the UTS simply asserts the return contract, and any SDK that diverges records it in its own deviations.md rather than in the shared spec. These notes were added earlier in this PR, so they net out of the final diff. - docs/writing-derived-tests.md (§ Recording deviations, § Test patterns, decision tree): make the deviation-handling guidance explicit and internally consistent so it can be followed without ambiguity: - the SDK-deviation choice is disposition-based — env-gated skip for a bug you expect to fix, adapted assertion (preferred) for a permanent/intentional divergence with stable behaviour; §2c and the decision tree now point at both patterns instead of "adapt" unconditionally. - new "Idiomatic translation vs genuine deviations" subsection: idiomatic public-API naming is not a deviation (just translate it); public-behaviour differences are deviations at any tier; internal-API shape differences are unit-tier-only adaptations that must preserve coverage. - keep all four section headings present in every deviations.md (mark empty ones "(none)"); add Status/Resolution entry fields and the shape-deviation vocabulary convention. - fixes: "four advantages" count, and "Test impact" wording no longer assumes adaptation. --- uts/docs/writing-derived-tests.md | 44 +++++++++++++++++------ uts/objects/unit/internal_live_counter.md | 9 ----- uts/objects/unit/internal_live_map.md | 10 ------ 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/uts/docs/writing-derived-tests.md b/uts/docs/writing-derived-tests.md index 4ad9f3ce5..a47b8bdb9 100644 --- a/uts/docs/writing-derived-tests.md +++ b/uts/docs/writing-derived-tests.md @@ -107,7 +107,7 @@ If the translation is wrong, fix the test. No deviation entry needed. If the UTS spec is correct per the features spec, and the test accurately translates it, then the SDK has a deviation. In this case: -- Keep the test, but adapt it to pass against the SDK's current behaviour +- Keep the test and handle it as an SDK deviation using one of the two patterns in section 3 — an **env-gated skip** or an **adapted assertion** (see there for which to choose) - Document exactly what the spec requires vs what the SDK does - Record it in the deviations file @@ -115,7 +115,7 @@ If the UTS spec is correct per the features spec, and the test accurately transl Once you've diagnosed a failure (section 2), there are three patterns. Two are for an **SDK deviation** (section 2c): **env-gated skip** keeps the spec-correct assertion but skips it unless explicitly enabled, and **adapted assertion** asserts the SDK's actual behaviour with the spec expectation in a comment. The third, **spec-error fail-fast**, is for a **UTS spec error** (section 2a): the spec itself is wrong, so there are no spec-correct assertions to write — the test fails fast and points at the fix instead. (A translation bug, section 2b, isn't a pattern — you just fix the test.) -**Env-gated skip** (preferred) — the test contains the correct spec assertion but is skipped by default. An environment variable enables it on demand: +**Env-gated skip** (preferred *for a deviation you expect to be fixed*) — the test contains the correct spec assertion but is skipped by default. An environment variable enables it on demand: ``` it("RSA7b - clientId from TokenDetails", function() { // DEVIATION: see deviations.md @@ -125,7 +125,7 @@ it("RSA7b - clientId from TokenDetails", function() { assert client.auth.clientId == "token-client-id" }) ``` -This has three advantages: +This has four advantages: - Normal test runs stay green (deviations are skipped) - Each deviation is individually reproducible: `RUN_DEVIATIONS=1 --grep "RSA7b"` - Issues filed against the SDK can link to a concrete reproduction command @@ -141,9 +141,9 @@ it("RSC1b - no credentials raises error", function() { assert error.code == 40160 }) ``` -Use this pattern when the SDK does *something* (just not the right thing) and you want to assert on the actual behaviour to prevent regressions. These tests pass in normal runs. +Use this pattern when the SDK does *something* (just not the right thing) and you want to assert on the actual behaviour to prevent regressions. These tests pass in normal runs. **Prefer this over an env-gated skip when the divergence is permanent or intentional** (the actual behaviour is stable and worth guarding) — a test that runs and asserts real behaviour catches regressions, whereas a spec-correct assertion that is skipped indefinitely verifies nothing. (Distinguish this from *idiomatic naming* — a differently-spelled public API is not a deviation at all — and from *internal-API shape* adaptation in white-box unit tests; see [Idiomatic translation vs genuine deviations](#idiomatic-translation-vs-genuine-deviations) under Recording deviations.) -**Spec-error fail-fast** — for a **UTS spec error** (section 2a), *not* an SDK deviation. An SDK deviation is a permanent fact, so its test stays green (env-gated) or asserts actual behaviour. A spec error is a bug in the source of truth that is fixable, so the opposite applies: the test must **fail immediately** with a message pointing to the deviations entry, forcing the spec to be corrected first rather than papered over: +**Spec-error fail-fast** — for a **UTS spec error** (section 2a), *not* an SDK deviation. An SDK deviation is a fact about the SDK as it stands, so its test stays green — skipped (env-gated) or asserting actual behaviour — and never blocks the suite. A spec error is a bug in the source of truth, so the opposite applies: the test must **fail immediately** with a message pointing to the deviations entry, forcing the spec to be corrected first rather than papered over: ``` it("RTLC7c2 - LOCAL source does not write siteTimeserials", function() { // SPEC ERROR RTLC7c2: replayed ACK serial "t:1:0" contradicts the harness ("ack-0:0"). @@ -172,7 +172,7 @@ Test fails | | | NO --> Fix the test | | - | YES --> SDK deviation. Adapt test, record in deviations file + | YES --> SDK deviation. Env-gated skip or adapted assertion (section 3); record in deviations file ``` --- @@ -185,16 +185,40 @@ When evaluating against an existing implementation, maintain a deviations file ( 2. **What the spec says** — quote or paraphrase the features spec 3. **What the SDK does** — concrete observable behaviour 4. **Root cause** (if known) — file, function, mechanism -5. **Test impact** — which test(s) are affected and how they were adapted +5. **Test impact** — which test(s) are affected and how each was handled (env-gated skip, adapted assertion, fail-fast, or skipped stub) -Deviations are grouped into four sections: +Also record, on any deviation whose disposition or outcome isn't already obvious: + +6. **Status** — *open bug* (to be fixed) vs *intentional / SDK-wide deviation* (and why it won't be), so it's clear whether an issue should be filed +7. **Resolution** — the outcome once the deviation is resolved by analysis or a spec change (e.g. reclassified as a cross-SDK design divergence, or a spec point revised upstream); omit until there is one + +Where a suite has recurring *internal-shape* differences (see **Adapted Tests** below), define a short **shape-deviation vocabulary** once — e.g. `S-1…S-n`, as the objects unit suite does in its mapping reference — and cite the tag from each affected entry rather than re-explaining the same structural difference per test. + +Deviations are grouped into four sections. **Keep all four headings present in every `deviations.md`, in this order** — and mark an empty category `*(none)*` rather than deleting its heading. This way every entry sits unambiguously under one category, and a reader can tell an empty category (nothing found) from a forgotten one: - **UTS Spec Errors** — the UTS spec itself is wrong (contradicts the features spec, or is internally inconsistent). The test **fails fast** pointing here (see "Spec-error fail-fast"); the fix belongs in the spec, not the SDK. Unlike the categories below, this is not an SDK problem. - **Failing Tests** — SDK non-compliance where the spec-correct test is present but skipped (env-gated). These are the primary output — each maps to a potential issue to file. -- **Adapted Tests** — SDK non-compliance where the test was adapted to assert actual behaviour. The test passes but documents a genuine deviation. +- **Adapted Tests** — SDK non-compliance where the test asserts the SDK's *actual* behaviour instead of the spec's. It passes, guards against regressions, and documents the deviation. **Whenever the actual behaviour is stable and assertable, prefer this and record the test here** — a running adapted test is worth more than a spec-correct assertion skipped indefinitely. (Reserve *Failing Tests* for deviations you expect to be fixed, where preserving the spec-correct assertion as the target matters.) - **Mock Infrastructure Limitations** — tests that can't be implemented due to missing mock capabilities (e.g. msgpack support). These are skipped stubs, not SDK deviations. This file is valuable output. It gives the SDK team a precise catalogue of spec gaps, each with a failing test that can be turned on once the fix lands. +### Idiomatic translation vs genuine deviations + +A test that doesn't read word-for-word like the pseudocode is **not** automatically a deviation. Before recording anything, decide which of three situations you're in — only the last two are deviations: + +**1. Idiomatic naming / rendering — not a deviation. Just translate it; record nothing.** +The public API is legitimately spelled differently in this language: a different method name, a property or getter where the spec writes a call, an idiomatic enum value (`MapSemantics.LWW` vs the wire string `'lww'`), `toJson()` rendered as `toMap()` / `to_dict()`, or a keyword that must be escaped (`channel.object` → `` channel.`object` `` in Kotlin). This is ordinary translation — see Phase 1 § *Map pseudocode to language idioms*, the naming rules in `writing-test-specs.md` § *Identifier Naming*, and the SDK's own mapping reference (e.g. ably-java's `objects-mapping.md`). The behaviour is identical; only the spelling changed, so there is nothing to record. + +**2. Public behaviour differs — a deviation, at any tier.** +The SDK produces an observably different *value or effect* through its public surface — e.g. a different error code (`40106` vs `40160`). Assert the actual behaviour with the spec expectation in a comment (the *adapted assertion* pattern above), or env-gate the spec-correct assertion (a *Failing Test*). This is the ordinary deviation case and applies at unit, integration and proxy tiers alike. + +**3. Internal-API shape differs — a deviation, at the unit tier only.** +Some *unit* specs are deliberately white-box: they drive internal classes and methods directly (an internal apply/operation method, the object pool, an internal "update" object). Unlike the public API, internal APIs are **not** standardised across SDKs — a boolean return where the spec models a returned object, an event where it models a return value, a missing accessor — so a white-box unit test adapts to the SDK's internal surface. Two rules keep it honest: + - **Preserve the coverage** — assert the equivalent *observable* (the resulting object state, the emitted event, the tombstone flag, …). Adapting the *shape* of the check is fine; **dropping** it is not. If an internal difference genuinely removes an observable, that is a real deviation, not an adaptation. + - **Name recurring differences once** as a shape-deviation vocabulary (`S-1…S-n`) and cite the tag per test, so each entry stays about *that* test rather than re-explaining the mechanism. + +Because **integration and proxy specs exercise only the public API**, case 3 never applies at those tiers — such a test must not reach into internal APIs to make itself pass. If one appears to need that, it is really case 2 (a public-behaviour deviation) or a translation bug. And never log case 1 (idiomatic naming) as a deviation — the deviations file is for behaviour the SDK gets *wrong*, not for how its API is spelled. + ### Filing issues from deviations Once the test suite is complete, classify the deviations into distinct issues grouped by root cause or theme — not one issue per test. For example, five tests that all fail because `auth.clientId` isn't derived from token details are one issue, not five. @@ -224,7 +248,7 @@ When writing tests before an implementation exists: ### Check the SDK's API surface -Not everything in the UTS pseudocode maps 1:1 to every SDK. Before writing tests, verify that the API exists. If an API is missing or named differently, note it and adapt the test. +Not everything in the UTS pseudocode maps 1:1 to every SDK. Before writing tests, verify that the API exists. An API that is merely *named* differently is idiomatic translation — use the SDK's name and move on; there is nothing to record (see [Idiomatic translation vs genuine deviations](#idiomatic-translation-vs-genuine-deviations)). An API that is genuinely *missing* is different: note it — the test may be not-applicable to this SDK, or the absence may itself be a deviation to record. ### Required options vary by SDK diff --git a/uts/objects/unit/internal_live_counter.md b/uts/objects/unit/internal_live_counter.md index 080db29b4..73a36b46d 100644 --- a/uts/objects/unit/internal_live_counter.md +++ b/uts/objects/unit/internal_live_counter.md @@ -11,15 +11,6 @@ Tests the `InternalLiveCounter` CRDT data structure. InternalLiveCounter holds a Tests operate directly on InternalLiveCounter by calling `applyOperation()` and `replaceData()` with constructed messages. No channel or connection infrastructure is needed. -> **SDK portability note — op-path update return value.** Several tests below assert on the -> `LiveCounterUpdate` **returned by `applyOperation(msg, source)`** (`noop`, `update.amount`, -> `tombstone`, `objectMessage`). This models SDKs whose operation application returns the update -> synchronously. An SDK whose operation path returns only a success/failure boolean and instead -> delivers the update via the object's subscription/emit path (e.g. a statically-typed SDK — see -> `objects-features.md` RTTS) may satisfy the same requirement by asserting the **emitted update -> event** and/or the **resulting object state** — these are equivalent observables of the same -> `LiveCounterUpdate`. The sync path (`replaceData()`) returns the update in all SDKs. - ## Shared Helpers See `helpers/standard_test_pool.md` for `build_counter_inc`, `build_counter_create`, `build_object_delete`, `build_object_state`. diff --git a/uts/objects/unit/internal_live_map.md b/uts/objects/unit/internal_live_map.md index 08fcd07d8..348601398 100644 --- a/uts/objects/unit/internal_live_map.md +++ b/uts/objects/unit/internal_live_map.md @@ -11,16 +11,6 @@ Tests the `InternalLiveMap` LWW-map CRDT data structure. InternalLiveMap holds a Tests operate directly on InternalLiveMap by calling `applyOperation()` and `replaceData()` with constructed messages. -> **SDK portability note — op-path update return value.** Several tests below assert on the -> `LiveMapUpdate` **returned by `applyOperation(msg, source)`** (`noop`, the per-key `update` diff -> such as `{ "name": "updated"/"removed" }`, `tombstone`, `objectMessage`). This models SDKs whose -> operation application returns the update synchronously. An SDK whose operation path returns only a -> success/failure boolean and instead delivers the update via the object's subscription/emit path -> (e.g. a statically-typed SDK — see `objects-features.md` RTTS) may satisfy the same requirement by -> asserting the **emitted update event** and/or the **resulting map state** — these are equivalent -> observables of the same `LiveMapUpdate`. The sync path (`replaceData()`) returns the update in all -> SDKs. - ## Shared Helpers See `helpers/standard_test_pool.md` for builder functions. From 1664abd7f2b54912b050987fc2e90460254e2949 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Thu, 30 Jul 2026 15:54:44 +0530 Subject: [PATCH 5/6] objects/uts: RTO27 nested-map coverage + tighten deviation entry-field wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - realtime_object.md (RTO27): assert the nested pooled map map:profile@1000 independently — cleared on DETACHED/FAILED, retained on SUSPENDED — checked via the pool (not via root navigation) so a nested-object regression can't be hidden by the root clear. Addresses the CodeRabbit review on ably-js#2278; mirrored in the ably-java and ably-js derived tests. - writing-derived-tests.md: drop the redundant "Also record…" lead-in so the deviation entry fields read as a single 1–7 list, and qualify the two non-core fields — Status "(for an SDK deviation)" and Resolution "(once resolved)" — so they don't read as mandatory on every entry (a UTS Spec Error / Mock Infra entry has no such disposition; Resolution is omitted until there is one). --- uts/docs/writing-derived-tests.md | 7 ++----- uts/objects/unit/realtime_object.md | 12 +++++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/uts/docs/writing-derived-tests.md b/uts/docs/writing-derived-tests.md index a47b8bdb9..e5dd1f321 100644 --- a/uts/docs/writing-derived-tests.md +++ b/uts/docs/writing-derived-tests.md @@ -186,11 +186,8 @@ When evaluating against an existing implementation, maintain a deviations file ( 3. **What the SDK does** — concrete observable behaviour 4. **Root cause** (if known) — file, function, mechanism 5. **Test impact** — which test(s) are affected and how each was handled (env-gated skip, adapted assertion, fail-fast, or skipped stub) - -Also record, on any deviation whose disposition or outcome isn't already obvious: - -6. **Status** — *open bug* (to be fixed) vs *intentional / SDK-wide deviation* (and why it won't be), so it's clear whether an issue should be filed -7. **Resolution** — the outcome once the deviation is resolved by analysis or a spec change (e.g. reclassified as a cross-SDK design divergence, or a spec point revised upstream); omit until there is one +6. **Status** *(for an SDK deviation)* — *open bug* (to be fixed) vs *intentional / SDK-wide deviation* (and why it won't be), so it's clear whether an issue should be filed +7. **Resolution** *(once resolved)* — the outcome after the deviation is resolved by analysis or a spec change (e.g. reclassified as a cross-SDK design divergence, or a spec point revised upstream); omit until there is one Where a suite has recurring *internal-shape* differences (see **Adapted Tests** below), define a short **shape-deviation vocabulary** once — e.g. `S-1…S-n`, as the objects unit suite does in its mapping reference — and cite the tag from each affected entry rather than re-explaining the same structural difference per test. diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 60ffaef8c..524724033 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -1614,9 +1614,10 @@ FOR state IN [DETACHED, FAILED]: { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") pool = channel.object.objectsPool # internal ObjectsPool (RTO3) - # sanity: STANDARD_POOL_OBJECTS has been materialised + # sanity: STANDARD_POOL_OBJECTS has been materialised — root map, a counter, and a NESTED map ASSERT "name" IN pool["root"].data ASSERT pool["counter:score@1000"].value == 100 + ASSERT "email" IN pool["map:profile@1000"].data # RTO27a1: no update events must be emitted during the clear updates = [] @@ -1624,10 +1625,14 @@ FOR state IN [DETACHED, FAILED]: channel.object.processChannelState(state) # internal channel-state handler (RTO27) - # RTO27a1: every object's data is cleared; the objects themselves remain in the pool + # RTO27a1: EVERY object's data is cleared — root map, the counter, AND the nested map. The nested + # map is checked independently (via the pool, not via root navigation) so a nested-object + # regression cannot be hidden by the root clear. The objects themselves remain in the pool. ASSERT pool["root"].data == {} ASSERT "counter:score@1000" IN pool ASSERT pool["counter:score@1000"].value == 0 + ASSERT "map:profile@1000" IN pool + ASSERT pool["map:profile@1000"].data == {} ASSERT updates.length == 0 ``` @@ -1639,7 +1644,8 @@ pool = channel.object.objectsPool channel.object.processChannelState(SUSPENDED) # RTO27b: no-op for objects data -# RTO27b: data retained unchanged +# RTO27b: data retained unchanged — root, the counter, and the nested map ASSERT "name" IN pool["root"].data ASSERT pool["counter:score@1000"].value == 100 +ASSERT "email" IN pool["map:profile@1000"].data ``` From 0aba67fdf67358bba9d6a55a162516504dff6ea2 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 31 Jul 2026 14:27:57 +0530 Subject: [PATCH 6/6] objects: clarify RTO27 channel-state scope, cross-ref RTO4, fix increment note Addresses the two review comments on PR #512: - RTO27b generalized from SUSPENDED-only to a catch-all: SUSPENDED plus every other non-{DETACHED,FAILED} state (INITIALIZED/ATTACHING/DETACHING) retains the stored objects data unchanged, so RTO27a+RTO27b are now total over all non-ATTACHED transitions (no behaviour change in ably-js/ably-java, which already fall through for those states). - Add reciprocal cross-references between RTO4 (ATTACHED) and RTO27 (non-ATTACHED) so the two halves of the channel-state lifecycle are mutually discoverable. - internal_live_counter_api.md: reword the null-row note around the omitted-argument call increment() rather than increment(null), keeping the null-means-omitted aside scoped to languages that treat null and the absent value as equivalently nullish, and explicitly not endorsing null as a valid amount. --- specifications/objects-features.md | 6 +++--- uts/objects/unit/internal_live_counter_api.md | 8 +++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/specifications/objects-features.md b/specifications/objects-features.md index 9a078ef64..227d6f159 100644 --- a/specifications/objects-features.md +++ b/specifications/objects-features.md @@ -139,7 +139,7 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO3a)` `ObjectsPool` is a `Dict` - a map of `LiveObject`s keyed by [`objectId`](../features#OST2a) string - `(RTO3b)` It must always contain an `InternalLiveMap` object with id `root` - `(RTO3b1)` Upon initialization of the `ObjectsPool`, create a new `InternalLiveMap` per [RTLM4](#RTLM4) with `objectId` set to `root` and add it to the `ObjectsPool` -- `(RTO4)` When a channel `ATTACHED` `ProtocolMessage` is received, the client library must perform the following actions in order. The `ProtocolMessage` may contain a `HAS_OBJECTS` bit flag (see [TR3](../features#TR3)); note that some of the following actions are conditional on this flag. +- `(RTO4)` When a channel `ATTACHED` `ProtocolMessage` is received, the client library must perform the following actions in order. The `ProtocolMessage` may contain a `HAS_OBJECTS` bit flag (see [TR3](../features#TR3)); note that some of the following actions are conditional on this flag. For transitions to channel states other than `ATTACHED`, see [RTO27](#RTO27). - `(RTO4c)` The [RTO17](#RTO17) sync state must transition to `SYNCING` if not already `SYNCING` - `(RTO4d)` The `bufferedObjectOperations` list must be cleared without applying any buffered operations - `(RTO4a)` If the `HAS_OBJECTS` flag is 1, the server will shortly perform an `OBJECT_SYNC` sequence as described in [RTO5](#RTO5). Note that this does not imply that objects are definitely present on the channel, only that there may be; the `OBJECT_SYNC` message may be empty @@ -192,11 +192,11 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO5c5)` The `bufferedObjectOperations` list must be cleared - `(RTO5c9)` The `appliedOnAckSerials` set ([RTO7b](#RTO7b)) must be cleared. A state sync causes the channel's LiveObjects data to be replaced, so after a state sync the `appliedOnAckSerials` no longer accurately describes which operations have been applied to the channel's LiveObjects data - `(RTO5c8)` The [RTO17](#RTO17) sync state must transition to `SYNCED` -- `(RTO27)` When the channel transitions to a state other than `ATTACHED`, the client library must manage the stored objects data as follows (for the effect of these transitions on an in-progress `publishAndApply`, see [RTO20e1](#RTO20e1)): +- `(RTO27)` When the channel transitions to a state other than `ATTACHED`, the client library must manage the stored objects data as follows (the `ATTACHED` transition is handled by [RTO4](#RTO4); for the effect of these transitions on an in-progress `publishAndApply`, see [RTO20e1](#RTO20e1)): - `(RTO27a)` When the channel transitions to the `DETACHED` or `FAILED` state, the current state of the objects data can no longer be known, so the client library must: - `(RTO27a1)` For every object in the internal `ObjectsPool`, clear its internal data, resetting it to the zero value for its type (an empty map, or a counter with value `0`), without emitting any `LiveObjectUpdate` events. The objects themselves remain in the `ObjectsPool`; only their data is cleared - `(RTO27a2)` The `SyncObjectsPool` must be cleared - - `(RTO27b)` When the channel transitions to the `SUSPENDED` state, the client library must retain the stored objects data unchanged, since the connection may still recover and the retained data remains a valid best-effort local copy + - `(RTO27b)` When the channel transitions to any other state (for example `SUSPENDED`, `INITIALIZED`, `ATTACHING`, or `DETACHING`), the client library must retain the stored objects data unchanged. In the `SUSPENDED` case in particular, the connection may still recover and the retained data remains a valid best-effort local copy - `(RTO6)` Certain object operations may require creating a new object if one does not already exist in the internal `ObjectsPool` for the given `objectId`. This can be done as follows: - `(RTO6a)` If an object with `objectId` exists in `ObjectsPool`, do not create a new object - `(RTO6b)` The expected type of the object can be inferred from the provided `objectId`: diff --git a/uts/objects/unit/internal_live_counter_api.md b/uts/objects/unit/internal_live_counter_api.md index 53f2dc7ac..7955d48e4 100644 --- a/uts/objects/unit/internal_live_counter_api.md +++ b/uts/objects/unit/internal_live_counter_api.md @@ -237,9 +237,11 @@ The `null` row applies only where `null` is passable and distinguishable from an argument (e.g. a boxed `Integer` in Java). In SDKs whose signature makes null equivalent to "omitted" (e.g. `increment(amount?: number)` in JavaScript, where a nullish amount takes the default of 1), the row is not applicable — see the pseudocode conventions in `uts/README.md`. -Such SDKs should instead assert the equivalence directly — `increment(null)` succeeds and -increments by the default of 1 — pinning the null-means-omitted contract so a later signature -or coalescing change surfaces as a conscious decision. +Such SDKs should instead pin the default directly via the omitted-argument call — `increment()` +succeeds and increments by the default of 1 (and, where the language treats `null` and the +absent value as equivalently nullish, `increment(null)` behaves identically) — so a later +signature or coalescing change surfaces as a conscious decision, without implying `null` is a +valid amount. ### Setup ```pseudo