-
Notifications
You must be signed in to change notification settings - Fork 231
[CI] Resolve backend test issue #3923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughMoves JWT-expiry waiting responsibility from UserRobot to BackendRobot, removes StreamMockServer injection from UserRobot, initializes UserRobot eagerly in StreamTestCase, and updates tests to call Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant BackendRobot
participant MockServer
participant App
Test->>BackendRobot: request waitForJwtToExpire()
BackendRobot->>MockServer: wait/sleep (jwtTimeout + 2s)
MockServer-->>BackendRobot: JWT expired state reached
BackendRobot-->>Test: return
Test->>App: continue test steps observing token expiry behavior
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@StreamChatUITestsAppUITests/Tests/Base` TestCase/StreamTestCase.swift:
- Around line 46-50: tearDownWithError() currently calls mockServer.stop()
unconditionally but mockServer is only created when useMockServer is true, which
can crash when nil; update tearDownWithError() to safely handle the optional by
checking useMockServer or optional-binding mockServer before calling stop(), and
ensure backendRobot and participantRobot cleanup also only run if mockServer was
created (refer to symbols mockServer, useMockServer, tearDownWithError(),
BackendRobot, ParticipantRobot).
🧹 Nitpick comments (1)
TestTools/StreamChatTestMockServer/Robots/BackendRobot.swift (1)
59-65: Consider usingThread.sleep(forTimeInterval:)for consistency.The method correctly calculates the sleep time in microseconds. However,
usleep()is a lower-level C function. The codebase already usesThread.sleep(forTimeInterval:)at line 124 inwaitForMockServerToStart(). Using the same approach here would improve consistency:♻️ Suggested refactor for consistency
`@discardableResult` public func waitForJwtToExpire() -> Self { - let sleepTime = UInt32((StreamMockServer.jwtTimeout + 2) * 1_000_000) - usleep(sleepTime) + Thread.sleep(forTimeInterval: TimeInterval(StreamMockServer.jwtTimeout + 2)) return self }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
StreamChatUITestsAppUITests/Robots/UserRobot.swiftStreamChatUITestsAppUITests/Tests/Authentication_Tests.swiftStreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swiftTestTools/StreamChatTestMockServer/Robots/BackendRobot.swift
💤 Files with no reviewable changes (1)
- StreamChatUITestsAppUITests/Robots/UserRobot.swift
🧰 Additional context used
📓 Path-based instructions (1)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Write Swift code compatible with iOS deployment targets specified in Package.swift and podspec files; do not lower deployment targets without approval
Use SwiftLint with strict mode and respect .swiftlint.yml rules; justify and scope any exceptions rather than suppressing rules broadly
Run SwiftFormat for code formatting and respect repository-specific style conventions
Never commit API keys or customer data; use obvious placeholders (e.g., YOUR_STREAM_KEY) in example code
Follow the project's zero-warnings policy: fix new warnings and avoid introducing any
Files:
StreamChatUITestsAppUITests/Tests/Authentication_Tests.swiftTestTools/StreamChatTestMockServer/Robots/BackendRobot.swiftStreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swift
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Tests/**/*.swift : Add or extend tests in the matching module's Tests/ folder for changes to StreamChat or StreamChatUI code, covering core models, API surfaces, and view controller behaviors; use fakes/mocks from test helpers provided by the repo
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Maintain high test coverage when changing code in the Stream iOS Chat SDK
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Sources/StreamChat/**/*.swift : Ensure public API changes in StreamChat and StreamChatUI include inline documentation and migration notes
📚 Learning: 2025-11-25T12:17:18.459Z
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Tests/**/*.swift : Add or extend tests in the matching module's Tests/ folder for changes to StreamChat or StreamChatUI code, covering core models, API surfaces, and view controller behaviors; use fakes/mocks from test helpers provided by the repo
Applied to files:
StreamChatUITestsAppUITests/Tests/Authentication_Tests.swiftTestTools/StreamChatTestMockServer/Robots/BackendRobot.swiftStreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swift
📚 Learning: 2025-11-25T12:17:18.459Z
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Sources/StreamChatUI/**/*.swift : Ensure public API changes in StreamChatUI include inline documentation and migration notes
Applied to files:
TestTools/StreamChatTestMockServer/Robots/BackendRobot.swiftStreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swift
📚 Learning: 2025-11-25T12:17:18.459Z
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Sources/StreamChat/**/*.swift : Ensure public API changes in StreamChat and StreamChatUI include inline documentation and migration notes
Applied to files:
TestTools/StreamChatTestMockServer/Robots/BackendRobot.swiftStreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swift
📚 Learning: 2025-11-25T12:17:18.459Z
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Maintain high test coverage when changing code in the Stream iOS Chat SDK
Applied to files:
StreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swift
🧬 Code graph analysis (1)
StreamChatUITestsAppUITests/Tests/Authentication_Tests.swift (1)
TestTools/StreamChatTestMockServer/Robots/BackendRobot.swift (1)
waitForJwtToExpire(60-65)
⏰ 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). (5)
- GitHub Check: Test Backend Integration
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Automated Code Review
- GitHub Check: Metrics
🔇 Additional comments (4)
StreamChatUITestsAppUITests/Tests/Authentication_Tests.swift (3)
126-126: LGTM!Moving
waitForJwtToExpire()tobackendRobotis semantically correct—JWT expiry is a server-side concern rather than a user action. This aligns with how other JWT operations (revokeToken,invalidateToken, etc.) are already handled throughbackendRobot.
145-145: LGTM!Consistent update to use
backendRobot.waitForJwtToExpire().
167-167: LGTM!Consistent update to use
backendRobot.waitForJwtToExpire().StreamChatUITestsAppUITests/Tests/Base TestCase/StreamTestCase.swift (1)
11-11: LGTM!Changing from implicitly unwrapped optional to a concrete
letconstant is a safer pattern. This aligns with the removal of server injection fromUserRobot.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@StreamChatUITestsAppUITests/Tests/Performance/MessageListScrollTime.swift`:
- Around line 9-13: Replace the hardcoded API key in setUpWithError by reading
it from an environment variable: in the override func setUpWithError() used
symbols are useMockServer and switchApiKey — retrieve
ProcessInfo.processInfo.environment["STREAM_PERFORMANCE_TEST_API_KEY"] (or a
CI-injected secret) and assign it to switchApiKey, and if missing either throw
an error or skip the test with XCTSkip to prevent committing secrets; ensure you
update setUpWithError to validate the fetched value before calling try
super.setUpWithError().
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
StreamChatUITestsAppUITests/Tests/Performance/MessageListScrollTime.swift
🧰 Additional context used
📓 Path-based instructions (1)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Write Swift code compatible with iOS deployment targets specified in Package.swift and podspec files; do not lower deployment targets without approval
Use SwiftLint with strict mode and respect .swiftlint.yml rules; justify and scope any exceptions rather than suppressing rules broadly
Run SwiftFormat for code formatting and respect repository-specific style conventions
Never commit API keys or customer data; use obvious placeholders (e.g., YOUR_STREAM_KEY) in example code
Follow the project's zero-warnings policy: fix new warnings and avoid introducing any
Files:
StreamChatUITestsAppUITests/Tests/Performance/MessageListScrollTime.swift
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Tests/**/*.swift : Add or extend tests in the matching module's Tests/ folder for changes to StreamChat or StreamChatUI code, covering core models, API surfaces, and view controller behaviors; use fakes/mocks from test helpers provided by the repo
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Maintain high test coverage when changing code in the Stream iOS Chat SDK
📚 Learning: 2025-11-25T12:17:18.459Z
Learnt from: CR
Repo: GetStream/stream-chat-swift PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T12:17:18.459Z
Learning: Applies to Tests/**/*.swift : Add or extend tests in the matching module's Tests/ folder for changes to StreamChat or StreamChatUI code, covering core models, API surfaces, and view controller behaviors; use fakes/mocks from test helpers provided by the repo
Applied to files:
StreamChatUITestsAppUITests/Tests/Performance/MessageListScrollTime.swift
🪛 Gitleaks (8.30.0)
StreamChatUITestsAppUITests/Tests/Performance/MessageListScrollTime.swift
[high] 11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| override func setUpWithError() throws { | ||
| useMockServer = false | ||
| switchApiKey = "zcgvnykxsfm8" | ||
| try super.setUpWithError() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded API key should be externalized.
The switchApiKey value on line 11 contains a hardcoded API key "zcgvnykxsfm8". As per coding guidelines, API keys should not be committed to the repository. Even for test/sandbox keys, consider externalizing this to an environment variable or a CI-injected secret to avoid accidental exposure and align with security best practices.
🔧 Suggested approach
override func setUpWithError() throws {
useMockServer = false
- switchApiKey = "zcgvnykxsfm8"
+ switchApiKey = ProcessInfo.processInfo.environment["STREAM_PERFORMANCE_TEST_API_KEY"] ?? ""
try super.setUpWithError()
}Then configure STREAM_PERFORMANCE_TEST_API_KEY as a CI secret in your GitHub Actions workflow.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override func setUpWithError() throws { | |
| useMockServer = false | |
| switchApiKey = "zcgvnykxsfm8" | |
| try super.setUpWithError() | |
| } | |
| override func setUpWithError() throws { | |
| useMockServer = false | |
| switchApiKey = ProcessInfo.processInfo.environment["STREAM_PERFORMANCE_TEST_API_KEY"] ?? "" | |
| try super.setUpWithError() | |
| } |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In `@StreamChatUITestsAppUITests/Tests/Performance/MessageListScrollTime.swift`
around lines 9 - 13, Replace the hardcoded API key in setUpWithError by reading
it from an environment variable: in the override func setUpWithError() used
symbols are useMockServer and switchApiKey — retrieve
ProcessInfo.processInfo.environment["STREAM_PERFORMANCE_TEST_API_KEY"] (or a
CI-injected secret) and assign it to switchApiKey, and if missing either throw
an error or skip the test with XCTSkip to prevent committing secrets; ensure you
update setUpWithError to validate the fetched value before calling try
super.setUpWithError().
laevandus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Looks OK. At first I was thinking about why bool over optional property, but that would all the callsites ugly since most of the time we need it.
Summary
Test
Summary by CodeRabbit
Refactor
Tests
Performance
✏️ Tip: You can customize this high-level summary in your review settings.