Skip to content

Ten realtime/unit fixtures cannot produce the condition they set up — six of them treat connectionStateTtl as something a test can set, and four attach channels on clients that never connect #544

Description

@owenpearson

Summary

Ten fixtures in uts/realtime/unit set up a condition they cannot actually produce. The tests then assert on that condition, so a conforming SDK either fails them or hangs. They fall into four groups, and the largest one — six tests trying to reach a SUSPENDED connection — all comes from the same misunderstanding about where connectionStateTtl lives.

Line references are against d9a04ca; paths are relative to uts/.


1. connectionStateTtl treated as something the test can set — six tests

features.md:2085 (DF1a) makes the default 120 s, and says it "is overriden by connectionStateTtl, if specified in the ConnectionDetails of the CONNECTED ProtocolMessage". features.md:1760 (CD2f) makes it a ConnectionDetails field, and :2527 declares it there. It is not a ClientOptions field — the class ClientOptions block at features.md:2177 does not list it.

Six tests try to shorten it anyway, three different ways, and none of the three reaches the SDK.

1a. A local variable the SDK cannot see — RTN25 and RTN14e

realtime/unit/RTN25/error-reason-suspended-2 (realtime/unit/connection/error_reason_test.md:125):

# :134-140  — the mock refuses every attempt
mock_ws = MockWebSocket(
  onConnectionAttempt: (conn) => {
    # All connection attempts fail
    conn.respond_with_refused()
  }
)
# :148
DEFAULT_CONNECTION_STATE_TTL = 5000  # 5 seconds
# :163
ADVANCE_TIME(DEFAULT_CONNECTION_STATE_TTL + 100)
# :166-167
AWAIT_STATE client.connection.state == ConnectionState.suspended
  WITH timeout: 1 second

DEFAULT_CONNECTION_STATE_TTL is a variable in the test. Nothing passes it to the client. Because the mock refuses every attempt, no CONNECTED message and therefore no connectionDetails ever arrive, so the SDK is necessarily using DF1a's 120 s default. Advancing 5,100 ms leaves it 114,900 ms short, and the WITH timeout: 1 second wait cannot succeed.

realtime/unit/RTN14e/disconnected-to-suspended-0 (realtime/unit/connection/connection_open_failures_test.md:438) is the same shape, and its comments concede the point:

# :461-464
# Simulate short connectionStateTtl
# In real implementation, this comes from server in CONNECTED message
# For this test, we'll use a short default value
DEFAULT_CONNECTION_STATE_TTL = 5000  # 5 seconds

"In real implementation, this comes from server in CONNECTED message" is exactly right — and no mechanism is supplied for the test to do that.

1b. Passed as a ClientOptions field that does not exist — RTL6c4 and RTN7e

realtime/unit/channels/channel_publish.md:629 and :1362:

client = Realtime(options: ClientOptions(
  key: "appId.keyId:keySecret",
  autoConnect: false,
  disconnectedRetryTimeout: 1000,
  connectionStateTtl: 5000
))

Both then loop ADVANCE_TIME(2000) waiting for a SUSPENDED connection that a conforming SDK reaches only after 120 s. A strictly-typed SDK rejects the unknown option outright.

1c. Waiting on channelRetryTimeout instead — RTL4b

realtime/unit/RTL4b/fails-connection-suspended-2 (realtime/unit/channels/channel_attach.md:436):

# :450
channelRetryTimeout: 100  # Short timeout for testing
# :466
AWAIT_STATE client.connection.state == ConnectionState.suspended

channelRetryTimeout governs SUSPENDED channels (features.md:2177 block, and RTL13b); connection suspension is connectionStateTtl. This test calls no enable_fake_timers() anywhere, so it would block for two real minutes.

1d. A transport drop asserted to produce a SUSPENDED channel — RTP5f and RTL11

realtime/unit/RTP5f/suspended-maintains-presence-map-0 (realtime/unit/presence/realtime_presence_channel_state.md:427):

# :473-475
# Channel becomes SUSPENDED (e.g., connection transitions to SUSPENDED)
mock_ws.active_connection.simulate_disconnect()
AWAIT_STATE channel.state == ChannelState.suspended

realtime/unit/RTL11/queued-presence-fail-suspended-1 (:667) does the same at :703-705.

A transport drop yields a DISCONNECTED connection. features.md:696 (RTL3c) propagates SUSPENDED to a channel only from a SUSPENDED connection, which again needs connectionStateTtl to elapse. Neither test enables fake timers or advances the clock. RTP5f's own comment names the right route — "connection transitions to SUSPENDED" — and the step below it does not take it.

Proposed fix for group 1

Give the mock a way to deliver a short connectionStateTtl in the CONNECTED message's connectionDetails, and have these six tests use it. Several tests in the same tree already send connectionDetails: { connectionStateTtl: 120000 } (e.g. connection_open_failures_test.md:138, :389, :529), so the pattern exists — it just needs a small value and the tests that need it. RTN25's mock additionally has to accept one connection before refusing, or the CONNECTED message has nowhere to travel.


2. realtime/unit/RTN24/connection-details-override-2 re-identifies the connection

realtime/unit/connection/update_events_test.md:213. The first CONNECTED carries clientId: "client-original" (:234); the second carries clientId: "client-updated" # Changed (:273); and :294 asserts the connection is still CONNECTED.

The file's own spec-requirement line at :215 names the overridable fields as "operational parameters like maxIdleInterval, connectionStateTtl, maxMessageSize, and serverId". Silently re-identifying an already-identified connection is not an operational parameter, and RTN24 was not written to license it. (Whether an SDK is right to fail here is a separate question — features.md:223 (RSA7b3) is the governing point and does not mandate a FAILED transition. But the fixture should not be the thing that settles it.)

Fix: drop the clientId change from the second CONNECTED at :273. The test already changes serverId at :272, which exercises RTN24 without raising the identity question.


3. realtime/unit/RTP5a/detached-clears-presence-maps-0 reads the map back with a call that refills it

realtime/unit/presence/realtime_presence_channel_state.md:223. After AWAIT channel.detach() (:277):

# :287-288
members_after = channel.presence.get(waitForSync: false)
ASSERT members_after.length == 0

features.md:937 (RTP11e) has get run the ensure-active-channel procedure for any state other than SUSPENDED, and :839 (RTL33b) makes that an implicit attach from DETACHED. waitForSync: false (RTP11c1) only skips waiting for the SYNC to complete; it does not skip RTP11e. The fixture's own server answers the re-ATTACH with ATTACHED plus a SYNC carrying alice, so a compliant SDK returns 1 member, not 0.

Fix: assert on the presence map directly, or detach and assert without calling get.


4. Four tests whose client never connects, in files that install no mock at all

Each of these sets autoConnect: false, installs no mock, never calls connect(), then awaits an attach and asserts ATTACHED:

Test Site Client Attach + assert
realtime/unit/RTS3c1/error-reattach-params-0 channels/channel_options.md:208 :218 :222-223
realtime/unit/RTL16a/triggers-reattach-0 channels/channel_options.md:310 :320 :322-323
realtime/unit/RTS4a/release-detaches-attached-2 channels/channels_collection.md:286 :296 :303-304

Per features.md:707 (RTL4i), an attach on a non-CONNECTED connection parks the channel in ATTACHING and the AWAIT never returns; with no mock installed, a real socket would be attempted.

This is not a coincidence of three tests. Four files under uts/realtime/unit/channels/ contain no install_mock call anywhere — channel_annotations.md, channel_history.md, channel_options.md and channels_collection.md — while the other seven use it between 7 and 36 times each. (The first two construct a MockWebSocketClient that the mock contract does not define; that is a separate report.)

A fourth test in the same file leaves its premise as a comment. realtime/unit/RTS3c1/error-reattach-modes-1 (channels/channel_options.md:245) ends its setup at :258 with:

# Put channel in attaching state (implementation detail)

and nothing else. The one condition the test turns on is the one condition it does not establish — and the file has no mock to establish it with.

Fix: give these four setups a mock and a connected client, as the sibling sections of channel_attach.md and channel_state_events.md do.


Found while deriving uts/realtime/unit for ably-python (all 54 specs, 481 tests). Each of the ten was corrected in the derived test rather than followed, with a comment at the site naming the change, so a corrected specification can be diffed against what was actually run.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions