[ECO-5397] Temporarily add logging around all awaits in integration tests

[lawrence-forooghian]

Note: This PR is based on top of #451.

For investigating #295. Asked Claude to do this; the messages look fine at a glance but haven't looked at them in any detail.

Summary by CodeRabbit

• Tests
  • Added enhanced logging to integration tests to improve visibility into asynchronous operations, facilitating better debugging of timing-related issues during continuous integration runs.

[coderabbitai]

Walkthrough

Adds extensive timestamped logging throughout IntegrationTests via a new logAwait helper function. Inserts logging calls before and after async operations including API calls, room actions, messaging, presence, occupancy, typing indicators, subscriptions, and history fetches to aid debugging of intermittent test hangs in CI.

Changes

Cohort / File(s)	Summary
CI debugging logging Tests/AblyChatTests/IntegrationTests.swift	Introduces private logAwait helper for timestamped logging around async operations; adds approximately 100+ logAwait calls surrounding Sandbox API initialization, room/messaging setup, presence/occupancy/typing interactions, subscriptions, history loops, and resource cleanup to trace execution flow during hanging scenarios

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes are largely repetitive logging insertions following a consistent pattern. Review effort centers on verifying that logAwait calls are positioned correctly around each async operation and that the logging doesn't inadvertently alter timing behavior. High line count but low logic density and homogeneous nature of edits.

Possibly related issues

• Tests seem to be intermittently hanging in CI #295: Main changes add detailed timestamped logging inside IntegrationTests to diagnose intermittent CI test hangs, which directly targets the same hanging-tests problem.

Poem

🐰 A rabbit hops through async lands,
With timestamps clutched in tiny paws,
"Where do hangs hide?" the bunny demands,
Now logging writes the debug clause,
CI mysteries shall pause! 📝✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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-5397 Temporarily add logging around all awaits in integration tests" directly and specifically summarizes the main change in the changeset. It clearly indicates that logging is being added around await points in integration tests, includes the issue reference, and uses "temporarily" to signal this is a debugging measure rather than a permanent change. The title is concise, readable, and provides sufficient clarity for a teammate scanning the history to understand the primary objective.
Linked Issues Check	✅ Passed	The PR fulfills the core objective of linked issue ECO-5397, which is to collect diagnostic information through logging to investigate intermittent CI test hangs. The PR introduces a logAwait helper and inserts it around various asynchronous operations throughout the integration tests, creating a comprehensive diagnostic tool to trace execution flow and identify where hangs occur. While this is a diagnostic measure rather than a fix, it directly supports the issue's objective of gathering reproduction data and diagnostic logs to identify the root cause.
Out of Scope Changes Check	✅ Passed	All changes in the PR are in scope and directly related to the objective of investigating CI test hangs. The modifications consist solely of adding a logAwait helper function and inserting logging calls around await points—exactly what is needed to collect diagnostic information for the investigation. The summary confirms that overall behavior and control flow remain unchanged, with only runtime logging added, indicating no unrelated changes were introduced.

✨ 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 295-investigate-hanging-integration-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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

Tests/AblyChatTests/IntegrationTests.swift (1)

203-208: Consider adding loop boundary logging for for-await loops.

The current logging comprehensively covers individual await points. However, the for await loops don't have entry/exit logging. If a hang occurs within one of these loops, it might be harder to identify.

Consider adding Self.logAwait("ENTERING for-await loop ...") before and Self.logAwait("EXITING for-await loop ...") after each loop to help pinpoint hangs within iteration logic, without logging every single iteration which could be too verbose.

Example for the reaction summary events loop:

+        Self.logAwait("BEFORE for-await reactionSummaryEvents loop")
         for await event in rxMessageReactionsSubscription {
             reactionSummaryEvents.append(event)
             if reactionSummaryEvents.count == 4 {
                 break
             }
         }
+        Self.logAwait("AFTER for-await reactionSummaryEvents loop")

Also applies to: 256-261, 568-571, 578-581, 602-608

📜 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 de65183 and ffab2ee.

📒 Files selected for processing (1)

• Tests/AblyChatTests/IntegrationTests.swift (22 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.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/IntegrationTests.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/IntegrationTests.swift

Tests/AblyChatTests/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• Tests/AblyChatTests/IntegrationTests.swift

🧬 Code graph analysis (1)

Tests/AblyChatTests/IntegrationTests.swift (4)

Tests/AblyChatTests/Helpers/Sandbox.swift (1)

• createAPIKey (30-49)

Sources/AblyChat/Room.swift (6)

• onStatusChange (155-169)
• onStatusChange (172-174)
• onStatusChange (395-398)
• attach (377-379)
• detach (382-384)
• release (386-391)

Sources/AblyChat/DefaultMessages.swift (3)

• send (122-124)
• subscribe (37-115)
• get (135-137)

Sources/AblyChat/Rooms.swift (3)

• get (60-63)
• get (164-269)
• release (319-357)

🔇 Additional comments (2)

Tests/AblyChatTests/IntegrationTests.swift (2)

1-9: Well-documented temporary debugging approach.

The maintainer note clearly explains the purpose and scope of the logging instrumentation, which will help future contributors understand why this extensive logging exists and maintain consistency during the investigation period.

---

25-29: Effective helper for debugging async hangs.

The logAwait helper provides good context with timestamps and source location information. Using Unix timestamp format is practical for calculating durations between log entries when analyzing hang behavior.