Skip to content

Conversation

@maratal
Copy link
Collaborator

@maratal maratal commented Oct 26, 2025

Closes #396

Summary by CodeRabbit

  • Tests
    • Added a new test validating subscription to presence events and correct event sequencing (enter, update, leave) including unsubscribe behavior.
    • Improved mock presence/channel behavior to support real subscription, emission and teardown of presence events for more reliable test coverage.

@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025

Walkthrough

Mocks were enhanced to store and emit presence callbacks; MockRealtimeChannel now initializes presence in init and delegates emissions. Tests were updated to use synchronous initializations, annotated with @mainactor, and a new test verifies subscribe/unsubscribe and enter/update/leave event flow through the channel.

Changes

Cohort / File(s) Summary
Mock: Presence implementation
Tests/AblyChatTests/Mocks/MockRealtimePresence.swift
Added private subscriptions storage; implemented subscribe(_:) and subscribe(_:callback:) to store callbacks and return listeners; made unsubscribe(_:) clear subscriptions; added emitMessage(_:) to notify all registered callbacks.
Mock: Channel delegation & init
Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift
Changed presence to a typed property initialized in init(...); added emitPresenceMessage(_:) that delegates to presence.emitMessage(_:).
Tests: Presence behavior
Tests/AblyChatTests/DefaultPresenceTests.swift
Added @MainActor annotation; replaced several await initializations with synchronous ones; added usersMaySubscribeToAllPresenceEvents test that subscribes to presence events, simulates enter/update/leave via channel, verifies event sequence, and checks no events after unsubscribe.

Sequence Diagram(s)

sequenceDiagram
    participant Test
    participant DefaultPresence
    participant MockRealtimeChannel
    participant MockRealtimePresence
    participant Callback

    Test->>DefaultPresence: subscribe(callback)
    DefaultPresence->>MockRealtimePresence: subscribe(callback)
    MockRealtimePresence->>MockRealtimePresence: store callback
    MockRealtimePresence-->>DefaultPresence: ARTEventListener

    Test->>MockRealtimeChannel: emitPresenceMessage(enter)
    MockRealtimeChannel->>MockRealtimePresence: emitMessage(enter)
    MockRealtimePresence->>Callback: invoke with enter
    Callback-->>Test: record enter

    Test->>MockRealtimeChannel: emitPresenceMessage(update)
    MockRealtimeChannel->>MockRealtimePresence: emitMessage(update)
    MockRealtimePresence->>Callback: invoke with update
    Callback-->>Test: record update

    Test->>MockRealtimeChannel: emitPresenceMessage(leave)
    MockRealtimeChannel->>MockRealtimePresence: emitMessage(leave)
    MockRealtimePresence->>Callback: invoke with leave
    Callback-->>Test: record leave

    Test->>DefaultPresence: unsubscribe(listener)
    DefaultPresence->>MockRealtimePresence: unsubscribe(listener)
    MockRealtimePresence->>MockRealtimePresence: clear callbacks
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review MockRealtimePresence: verify subscription storage, correct invocation of stored callbacks, and unsubscribe behavior.
  • Confirm MockRealtimeChannel init change and emitPresenceMessage correctly delegates.
  • Check the new test for correct use of @mainactor and synchronous initializations and that it reliably asserts event ordering and no events after unsubscribe.

Poem

🐰 I sat and listened where presences play,
Mocks learned to remember each message relay,
Enter, update, leave—one tidy parade,
Subscribed, then freed, the callbacks parade,
Hops of test-green joy, a well-crafted day. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 69.57% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "ECO-5596 Presence subscribe tests" directly and clearly relates to the main change in the changeset. The changes focus on introducing a new test named usersMaySubscribeToAllPresenceEvents that validates presence subscription behavior, which is precisely what the title indicates. The title is concise, specific, and includes the Jira issue identifier, making it clear for someone scanning the commit history that this PR addresses presence subscription testing requirements.
Linked Issues Check ✅ Passed The code changes successfully fulfill the linked issue requirements [#396, ECO-5596]. The new test usersMaySubscribeToAllPresenceEvents in DefaultPresenceTests validates presence subscription behavior by emitting presence messages through the channel (enter, update, leave events) rather than bypassing it. Supporting infrastructure was enhanced across MockRealtimeChannel and MockRealtimePresence to enable proper event emission and subscription tracking, allowing tests to validate the complete presence subscription flow from channel to subscriber. These changes align with the stated objective to "reintroduce and correctly implement tests" that validate "behavior when a presence event is emitted by the channel."
Out of Scope Changes Check ✅ Passed All changes in this PR remain within scope of the linked issues [#396, ECO-5596]. The modifications to DefaultPresenceTests.swift (adding the subscription test and adjusting initializations), MockRealtimeChannel.swift (adding emitPresenceMessage method and refactoring presence initialization), and MockRealtimePresence.swift (implementing functional subscription storage and emit mechanisms) are all directly necessary to support proper testing of Presence.subscribe through the channel. No unrelated features, refactoring, or infrastructure changes are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 396-presence-subscribe-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0436cfc and efcd5cf.

📒 Files selected for processing (3)
  • Tests/AblyChatTests/DefaultPresenceTests.swift (17 hunks)
  • Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (3 hunks)
  • Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • Tests/AblyChatTests/DefaultPresenceTests.swift
  • Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.swift: Use protocol-based design; expose SDK functionality via protocols and prefer associated types with opaque return types (some Protocol) instead of existentials (any Protocol)
Isolate all mutable state to the main actor; mark stateful objects with @mainactor
Public API must use typed throws with ErrorInfo; use InternalError internally and convert at the public API boundary
For public structs emitted by the API, provide an explicit public memberwise initializer
When using AsyncSequence operators in @mainactor contexts, mark operator closures as @sendable
Task, CheckedContinuation, and AsyncThrowingStream do not support typed errors; use Result and call .get() to surface typed errors
Do not use Dictionary.mapValues for typed throws; use ablyChat_mapValuesWithTypedThrow instead
When the compiler struggles with typed throws, explicitly declare the error type on do blocks (e.g., do throws(InternalError))
Specify error types in closures using the throws(ErrorType) syntax (e.g., try items.map { jsonValue throws(InternalError) in ... })
Mark any test-only APIs with testsOnly_ prefix and wrap them in #if DEBUG

Files:

  • Tests/AblyChatTests/Mocks/MockRealtimePresence.swift
Tests/AblyChatTests/**/*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

Tests/AblyChatTests/**/*.swift: Use test attribution tags to reference spec coverage: @SPEC, @specOneOf(m/n), @specPartial, @specUntested, @specNotApplicable
For Swift Testing #expect(throws:) with typed errors, move typed-throw code into a separate non-typed-throw function until Xcode 16.3+

Files:

  • Tests/AblyChatTests/Mocks/MockRealtimePresence.swift
Tests/AblyChatTests/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place tests under Tests/AblyChatTests/ as the root for test targets

Files:

  • Tests/AblyChatTests/Mocks/MockRealtimePresence.swift
Tests/AblyChatTests/Mocks/**/*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

Put mock implementations under Tests/AblyChatTests/Mocks/

Files:

  • Tests/AblyChatTests/Mocks/MockRealtimePresence.swift
🧬 Code graph analysis (1)
Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (5)
Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (3)
  • subscribe (142-144)
  • subscribe (147-153)
  • unsubscribe (161-163)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (9)
  • subscribe (279-281)
  • subscribe (283-285)
  • subscribe (383-385)
  • subscribe (387-389)
  • subscribe (501-503)
  • subscribe (505-507)
  • unsubscribe (287-289)
  • unsubscribe (391-393)
  • unsubscribe (497-499)
Sources/AblyChat/DefaultPresence.swift (1)
  • subscribe (175-200)
Sources/AblyChat/Presence.swift (2)
  • subscribe (126-140)
  • subscribe (143-145)
Sources/AblyChat/Subscription.swift (2)
  • unsubscribe (63-65)
  • unsubscribe (90-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Xcode, iOS (Xcode 26.0)

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for Presence.subscribe (CHA-PR7)

2 participants