ci(repo): Add E2E message list tests - #2854
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes expand message-list integration testing with new page objects, fluent robot actions, assertions, lifecycle and URL-launch controls, and Allure metadata. They also update thread/channel UI error handling, persistence teardown, iOS simulator retries, and embedded framework configuration. ChangesMessage list integration testing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StreamTestEnv
participant UserRobot
participant MessageListPage
participant FakeUrlLauncher
StreamTestEnv->>FakeUrlLauncher: install URL-launch fake
UserRobot->>MessageListPage: tap link preview
MessageListPage->>FakeUrlLauncher: launch URL
FakeUrlLauncher-->>StreamTestEnv: record launched URL
StreamTestEnv-->>UserRobot: assert expected URL
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sample_app/integration_test/support/widget_test_extensions.dart`:
- Around line 47-93: Replace the unbounded pumpAndSettle() calls in tapFinder,
scrollMessageList, and scrollUpUntilVisible with the bounded settle() helper
already used by the visibility methods. Preserve the existing sequencing and
ensure scrollUpUntilVisible’s timeout can be enforced between iterations even
when perpetual animations are active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 20fd9c56-b7e3-4570-aada-4ceca4e1deac
📒 Files selected for processing (10)
sample_app/fastlane/Fastfilesample_app/integration_test/message_list_test.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/robots/participant_robot.dartsample_app/integration_test/robots/user_robot.dartsample_app/integration_test/robots/user_robot_message_list_asserts.dartsample_app/integration_test/support/stream_test_env.dartsample_app/integration_test/support/widget_test_extensions.dartsample_app/ios/Runner.xcodeproj/project.pbxprojsample_app/lib/pages/thread_page.dart
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2854 +/- ##
=======================================
Coverage 72.86% 72.86%
=======================================
Files 429 429
Lines 27698 27698
=======================================
Hits 20183 20183
Misses 7515 7515 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
# Conflicts: # sample_app/integration_test/support/stream_test_env.dart
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sample_app/integration_test/pages/message_list_page.dart`:
- Around line 99-116: Update sendingStatus’s hasStatus cases for sent and
pending to mirror StreamSendingIndicator.build()’s priority order: exclude
messages that are already read or delivered, and ensure pending also does not
match completed messages. Preserve the existing status checks while preventing
lower-priority statuses from matching higher-priority states.
In `@sample_app/lib/pages/channel_page.dart`:
- Around line 125-132: Update both StreamMessageComposer onError handlers in
sample_app/lib/pages/channel_page.dart lines 125-132 and
sample_app/lib/pages/thread_page.dart lines 103-107 to include the received
stackTrace in their debugPrint failure logs alongside the error, preserving the
existing logging behavior and retry handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a563ea45-a671-4ace-b0bc-ee2a80cf3da3
📒 Files selected for processing (8)
melos.yamlsample_app/integration_test/message_list_test.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/robots/user_robot_message_list_asserts.dartsample_app/integration_test/support/stream_test_env.dartsample_app/integration_test/support/widget_test_extensions.dartsample_app/lib/pages/channel_page.dartsample_app/lib/pages/thread_page.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sample_app/fastlane/Fastfile`:
- Line 28: Validate udid before the sh call, rejecting any value containing
characters outside 0-9, A-F, a-f, and hyphen, then apply Shellwords.escape to
the validated value before interpolating it into the xcrun simctl bootstatus
command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31566eea-a395-42e9-b355-c8cbea489942
📒 Files selected for processing (14)
melos.yamlsample_app/fastlane/Fastfilesample_app/integration_test/message_list_test.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/robots/participant_robot.dartsample_app/integration_test/robots/user_robot.dartsample_app/integration_test/robots/user_robot_message_list_asserts.dartsample_app/integration_test/support/fake_url_launcher.dartsample_app/integration_test/support/stream_test_env.dartsample_app/integration_test/support/widget_test_extensions.dartsample_app/ios/Runner.xcodeproj/project.pbxprojsample_app/lib/pages/channel_page.dartsample_app/lib/pages/thread_page.dartsample_app/pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (11)
- sample_app/pubspec.yaml
- sample_app/integration_test/support/fake_url_launcher.dart
- sample_app/lib/pages/channel_page.dart
- sample_app/ios/Runner.xcodeproj/project.pbxproj
- sample_app/integration_test/robots/participant_robot.dart
- sample_app/integration_test/support/stream_test_env.dart
- sample_app/lib/pages/thread_page.dart
- sample_app/integration_test/robots/user_robot.dart
- sample_app/integration_test/support/widget_test_extensions.dart
- sample_app/integration_test/robots/user_robot_message_list_asserts.dart
- sample_app/integration_test/message_list_test.dart
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sample_app/integration_test/support/stream_test_env.dart`:
- Around line 180-190: Update _drainPendingPersistenceWork to await the
persistence layer’s actual idle or pending-request completion signal instead of
polling for a fixed two-second duration. Ensure tearDown waits until all
recovery queries and persistence work have completed before debugReset() closes
persistence, while preserving the existing _usePersistence guard.
In `@sample_app/lib/auth/auth_controller.dart`:
- Around line 273-280: Serialize client teardown by storing or awaiting the
Future started in disconnect() when it calls _client?.disconnectUser(...), then
have debugReset() await that same shared disconnect operation before disposing
the client. Preserve the existing flushChatPersistence behavior and ensure both
logout and reset cannot mutate or dispose the client concurrently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e438e4d5-1a1f-482d-af38-6acbf99650db
📒 Files selected for processing (3)
sample_app/fastlane/Fastfilesample_app/integration_test/support/stream_test_env.dartsample_app/lib/auth/auth_controller.dart
VelikovPetar
left a comment
VelikovPetar
left a comment
There was a problem hiding this comment.
Some small (and some unrelated) discussion points, but looks good to me!
| /// is `'1'` (the newest is `'$messagesCount'`). | ||
| Future<UserRobot> assertMessageListPagination({required int messagesCount}) async { | ||
| await tester.waitUntilVisible(find.text('$messagesCount')); | ||
| await tester.scrollUpUntilVisible(find.text('1')); |
There was a problem hiding this comment.
Small suggestion - maybe this can be dynamic value instead of 1? Or is it safe to assume 1 will always be the first message?
There was a problem hiding this comment.
fair concern, i updated it so now it does by message id and not by hard-coded text
| // response builds a `ChannelClientState`, whose constructor reads the cached | ||
| // threads. Both used to throw an async StateError, which the test framework | ||
| // then blamed on an unrelated test that had already passed. | ||
| final persistence = _client?.chatPersistenceClient; |
There was a problem hiding this comment.
I think we should also spend some time investigating why this workaround is needed. It might be surfacing a potential race-condition in the SDK (HTTP call in progress when the client is disposed).
There was a problem hiding this comment.
i created a ticket for this https://linear.app/stream/issue/FLU-664/
| child: Container( | ||
| alignment: Alignment.centerLeft, | ||
| color: context.streamColorScheme.backgroundApp.withValues(alpha: .9), | ||
| child: StreamTypingIndicator( |
There was a problem hiding this comment.
Question (not directly related to this PR): I see that we are manually adding the StreamTypingIndicator here - should it maybe be packaged together in the StreamMessageListView (I believe that's how other SDKs ship it)? it will be a breaking change to add it now, but perhaps something to consider for the next major release.
cc @xsahil03x
There was a problem hiding this comment.
i think this ticket might be related to this https://linear.app/stream/issue/FLU-661
There was a problem hiding this comment.
Yes, we have a ticket for this one
3 participants
Linear: https://linear.app/stream/issue/FLU-603
CLA
Description of the pull request
Ports the native
MessageList_Tests.swift/MessageListTests.ktsuite to the sample app's integration tests, plus the harness plumbing it needed. Second suite after #2825 (reactions).New tests (
integration_test/message_list_test.dart) — 27 total: 23 active, 4 skippedEditedfootnote)@and disappears when it's removedHarness additions
pages/message_list_page.dart— action rows, list, composer overlays, back button, thread header. All selectors reuse identifiers the SDK already exposes; no test-only keys were added to the SDK.support/widget_test_extensions.dart—tapFinder(ensureVisible + tap for below-fold modal rows),scrollMessageList(delta),scrollUpUntilVisible, plus a boundedsettle()so a perpetual reconnect animation can't hangpumpAndSettle.robots/user_robot.dart— edit / delete / openThread / typeText / clearComposer / tapBackButton / scroll, with chain mirrors;user_robot_message_list_asserts.dartgained edited / deleted / hardDeleted / typing / mentions / linkPreview / threadReply asserts plus pagination and the twogetSize-based geometry asserts.robots/participant_robot.dart— chain mirrors for editMessage / deleteMessage / sendMessageInThread / startTyping / stopTyping.support/stream_test_env.dart—moveToBackground()/moveToForeground()lifecycle helpers.lib/pages/thread_page.dart— the sample app's thread page now renders aStreamTypingIndicator(parentId:), which is what the thread-typing test asserts on.CI: recover a simulator that dies mid-run
The first nightly run of this suite exposed an unrelated harness gap. On the iOS 17.5 leg (job 89985512388) the simulator itself died ~11 min in: 17 tests had passed, the 18th went silent mid-setup, the remaining tests were reported as instant passes with no output, and
flutter testexited non-zero. Becauseprepare_simulatorran once before the attempt loop, every retry after that failed in ~15s withNo supported devices found with name or id matching <udid>— andreactions_test.dartnever ran a single test.run_e2e_testnow callsboot_simulatorat the top of every attempt except the first: a no-op when the device is already booted,simctl bootstatus -bwhen it is shut down, and an early return when the udid is gone entirely (so a missing device can't block the run). One crashed simulator now costs one attempt instead of the rest of the job.Test
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Tests
Chores