test(repo): Add channel list and quoted reply e2e suites - #2869
Open
testableapple wants to merge 28 commits into
Open
test(repo): Add channel list and quoted reply e2e suites#2869testableapple wants to merge 28 commits into
testableapple wants to merge 28 commits into
Conversation
Contributor
|
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 PR adds Allure launch orchestration and batched Android and iOS E2E runs. It fixes duplicate thread roots, exports ChangesE2E launch orchestration
SDK and widget behavior fixes
Flutter integration coverage
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2869 +/- ##
=======================================
Coverage 73.18% 73.18%
=======================================
Files 429 429
Lines 27724 27724
=======================================
Hits 20289 20289
Misses 7435 7435 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
sample_app/integration_test/channel_list_test.dart (2)
345-356: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider exercising real network pagination.
The comment states the mock server pages the list only when it holds more than the requested limit, and that the app requests 30 channels at a time. With
channelsCount = 30, the test covers scrolling only.Raising the count above the page limit would exercise the paging request path at low cost. If strict parity with the native scenario is the goal, keep 30 and add a separate test for the paging path.
♻️ Proposed change to cover the paging request path
- const channelsCount = 30; + // Exceeds the app's 30-channel page size, so the mock server pages the list. + const channelsCount = 45;🤖 Prompt for 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. In `@sample_app/integration_test/channel_list_test.dart` around lines 345 - 356, Update the channel-list pagination test around assertChannelListPagination to use a channel count greater than the app’s 30-item page limit, so it exercises real network paging in addition to scrolling. If preserving native parity requires keeping channelsCount at 30, add a separate test with an above-limit count targeting the same pagination flow.
122-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTests assert SDK translation literals instead of the translation source. Four assertions hardcode strings that the SDK owns and localizes. A translation change breaks these tests with an unclear failure message. This PR already localizes another string in
packages/stream_chat_flutter/lib/src/message_widget/stream_message_item.dart, so the risk is active. Reference the translation values instead of the literals.
sample_app/integration_test/channel_list_test.dart#L122-L124: replace the'No messages yet'literal with the SDK'semptyMessagesTexttranslation value.sample_app/integration_test/channel_list_test.dart#L153-L153: replace the'Message deleted'literal with the SDK's deleted-message translation value.sample_app/integration_test/channel_list_test.dart#L186-L186: replace the'Message deleted'literal with the same deleted-message translation value.sample_app/integration_test/channel_list_test.dart#L329-L329: replace the'No messages yet'literal with the sameemptyMessagesTexttranslation value.🤖 Prompt for 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. In `@sample_app/integration_test/channel_list_test.dart` around lines 122 - 124, Update the assertions in sample_app/integration_test/channel_list_test.dart at 122-124, 153, 186, and 329 to compare against the SDK-owned translation source instead of hardcoded literals; use the same emptyMessagesText value for the “No messages yet” checks and the SDK’s deleted-message translation value for both “Message deleted” checks. Keep the existing assertion flow in each test, only swapping the expected string source so the tests follow localization changes.sample_app/integration_test/quoted_reply_test.dart (3)
107-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
firstMessageat Line 125 for consistency.Line 107 declares
firstMessage. Line 118 uses it, but Line 125 repeats the literal'1'. Use the constant at both sites.♻️ Proposed change
- await env.userRobot.assertMessageOnScreen('1').assertScrollToBottomButton(isDisplayed: true); + await env.userRobot.assertMessageOnScreen(firstMessage).assertScrollToBottomButton(isDisplayed: true);🤖 Prompt for 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. In `@sample_app/integration_test/quoted_reply_test.dart` around lines 107 - 125, Update the final assertMessageOnScreen call in the quoted reply test to use the existing firstMessage constant instead of repeating the literal message value, while preserving the current assertion behavior.
204-207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deriving the deleted-message label from the SDK translations.
Lines 207 and 247 assert the literal
'Message deleted'. The comment states the SDK rendersmessageDeletedLabel. If that translation value changes, or if the app runs under a non-default locale, both assertions fail for a reason unrelated to quoted replies. Read the label from the SDK translations, or define one shared constant in the test support layer.🤖 Prompt for 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. In `@sample_app/integration_test/quoted_reply_test.dart` around lines 204 - 207, Update the deleted-message assertions in the quoted-reply test, including the cases around the existing `assertQuotedMessage` calls, to derive the expected label from the SDK translations instead of hardcoding 'Message deleted'. Reuse the SDK’s messageDeletedLabel value for both assertions so locale changes remain supported.
701-705: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftFixed scroll counts make these thread-root tests screen-size dependent.
Lines 702, 729, 755, and 776 scroll a fixed number of times to reach the top of the thread. The comment at Line 727 states each drag is 300px. On a taller device, or if the message item height changes, 8 drags may not reach the root, and the failure surfaces as
assertMessages(count: 1)returning 0. Line 776 also uses 5 without a stated reason.Consider adding a scroll-until-visible helper in the test harness that drags until the target text is found or a timeout expires. That removes the tuning constant from four tests.
🤖 Prompt for 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. In `@sample_app/integration_test/quoted_reply_test.dart` around lines 701 - 705, Replace the fixed scroll counts in the quoted-reply thread-root tests with a test-harness helper that repeatedly scrolls until the target message text is visible or a timeout expires. Update the flows around the assertions using assertMessages and the existing scrollMessageListUp calls at all four affected locations, preserving the exact-once assertion while removing screen-size-dependent tuning constants.
🤖 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`:
- Around line 27-32: Update the batching logic in Fastfile around the slice
calculation so an empty targets list is handled before calling
targets.each_slice. Add an early return or empty-batch branch in the same flow
that uses batch_count, batch, and slice, so targets.empty? skips the each_slice
call entirely and preserves the existing “nothing to run” behavior without
raising ArgumentError.
In `@sample_app/integration_test/channel_list_test.dart`:
- Around line 97-107: Update the invalid-command scenario around
userRobot.sendMessage so it calls assertInvalidCommandMessage('/test') before
tapBackButton, verifying the mock server’s error response is rendered before
navigation; keep the existing channel preview assertions afterward.
In `@sample_app/integration_test/quoted_reply_test.dart`:
- Around line 263-265: Update both invalid-command test assertions in
sample_app/integration_test/quoted_reply_test.dart at lines 263-265 and 464-466
to verify the submitted "/$invalidCommand" is not displayed, or that no quote
bubble is rendered, instead of checking the unsent quoteReply text. Apply the
same correction to both the regular and thread variants.
In `@sample_app/integration_test/robots/user_robot_message_list_asserts.dart`:
- Around line 171-180: Update assertMessageCount in UserRobot so its rowCount
helper includes StreamModeratedMessage rows in the total, not just
MessageListPage.messageItem and MessageListPage.list.systemMessage. Keep the
existing polling and expectation flow unchanged, and use the same message-row
counting pattern as the other UserRobot assertion helpers so invalid-command
feedback is counted correctly.
In `@sample_app/integration_test/robots/user_robot.dart`:
- Line 249: Update the fluent openThread method in UserRobot to accept an
optional parentText parameter and forward it, along with messageIndex, to the
underlying it.openThread call so callers can select thread parents by text.
---
Nitpick comments:
In `@sample_app/integration_test/channel_list_test.dart`:
- Around line 345-356: Update the channel-list pagination test around
assertChannelListPagination to use a channel count greater than the app’s
30-item page limit, so it exercises real network paging in addition to
scrolling. If preserving native parity requires keeping channelsCount at 30, add
a separate test with an above-limit count targeting the same pagination flow.
- Around line 122-124: Update the assertions in
sample_app/integration_test/channel_list_test.dart at 122-124, 153, 186, and 329
to compare against the SDK-owned translation source instead of hardcoded
literals; use the same emptyMessagesText value for the “No messages yet” checks
and the SDK’s deleted-message translation value for both “Message deleted”
checks. Keep the existing assertion flow in each test, only swapping the
expected string source so the tests follow localization changes.
In `@sample_app/integration_test/quoted_reply_test.dart`:
- Around line 107-125: Update the final assertMessageOnScreen call in the quoted
reply test to use the existing firstMessage constant instead of repeating the
literal message value, while preserving the current assertion behavior.
- Around line 204-207: Update the deleted-message assertions in the quoted-reply
test, including the cases around the existing `assertQuotedMessage` calls, to
derive the expected label from the SDK translations instead of hardcoding
'Message deleted'. Reuse the SDK’s messageDeletedLabel value for both assertions
so locale changes remain supported.
- Around line 701-705: Replace the fixed scroll counts in the quoted-reply
thread-root tests with a test-harness helper that repeatedly scrolls until the
target message text is visible or a timeout expires. Update the flows around the
assertions using assertMessages and the existing scrollMessageListUp calls at
all four affected locations, preserving the exact-once assertion while removing
screen-size-dependent tuning constants.
🪄 Autofix
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: e6091c28-d588-4e78-b9b4-dabd3bae06d2
📒 Files selected for processing (19)
.github/actions/allure-launch/action.yml.github/workflows/e2e_test.ymlpackages/stream_chat/CHANGELOG.mdpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/test/src/client/channel_test.dartpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/message_widget/stream_message_item.dartsample_app/fastlane/Fastfilesample_app/integration_test/channel_list_test.dartsample_app/integration_test/pages/channel_list_page.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/quoted_reply_test.dartsample_app/integration_test/robots/user_robot.dartsample_app/integration_test/robots/user_robot_channel_list_asserts.dartsample_app/integration_test/robots/user_robot_message_list_asserts.dartsample_app/integration_test/support/stream_test_case.dartsample_app/integration_test/support/stream_test_env.dartsample_app/integration_test/support/widget_test_extensions.dartsample_app/lib/pages/thread_page.dart
💤 Files with no reviewable changes (1)
- sample_app/lib/pages/thread_page.dart
Contributor
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 `@packages/stream_chat_flutter/lib/stream_chat_flutter.dart`:
- Line 100: Update the message list view consumer so `StreamEphemeralMessage` is
imported from its local source file instead of relying on the public barrel
export. In `message_list_view.dart`, adjust the imports around
`StreamEphemeralMessage` to reference
`src/message_widget/stream_ephemeral_message.dart` directly, or otherwise ensure
the internal caller intentionally uses the barrel only if that is the desired
contract.
🪄 Autofix
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: 2b861601-c6d5-43c0-b31e-9a95ef439734
📒 Files selected for processing (12)
.github/workflows/e2e_test.yml.github/workflows/e2e_test_cron.ymlpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dartpackages/stream_chat_flutter/lib/stream_chat_flutter.dartsample_app/fastlane/Fastfilesample_app/integration_test/channel_list_test.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/quoted_reply_test.dartsample_app/integration_test/robots/user_robot.dartsample_app/integration_test/robots/user_robot_message_list_asserts.dartsample_app/integration_test/support/widget_test_extensions.dart
💤 Files with no reviewable changes (1)
- packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart
🚧 Files skipped from review as they are similar to previous changes (8)
- sample_app/fastlane/Fastfile
- sample_app/integration_test/pages/message_list_page.dart
- sample_app/integration_test/channel_list_test.dart
- .github/workflows/e2e_test.yml
- sample_app/integration_test/quoted_reply_test.dart
- sample_app/integration_test/robots/user_robot.dart
- sample_app/integration_test/robots/user_robot_message_list_asserts.dart
- sample_app/integration_test/support/widget_test_extensions.dart
testableapple
commented
Aug 5, 2026
VelikovPetar
reviewed
Aug 7, 2026
|
|
||
| step('AND the message timestamp is still shown'); | ||
| // iOS expects it hidden here. On Flutter `lastMessageAt` lives on the | ||
| // channel model and truncation only empties `state.messages`, so nothing |
Contributor
There was a problem hiding this comment.
Hmm could this be a bug? - I think we should investigate this, or at least check how Android/iOS/RN handle this case.
Member
Author
There was a problem hiding this comment.
nice catch, you are right! i filed a ticket (https://linear.app/stream/issue/FLU-685) and fixed it as part of this PR and added unit tests as well, please check it out
Contributor
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
`@packages/stream_chat_flutter/lib/src/scroll_view/channel_scroll_view/stream_channel_list_item.dart`:
- Around line 780-800: Update _LastMessageResolver.resolveLastMessage to track
the ChannelClientState associated with _currentLastMessage and clear the cached
message whenever a different state is received before resolving. Preserve the
existing fallback behavior for the same non-up-to-date state, and add a
regression test covering an empty replacement channel with isUpToDate false.
In
`@packages/stream_chat_flutter/test/src/message_widget/stream_message_item_test.dart`:
- Around line 49-70: Update the tests around buildScene and the single- and
multiple-reply testWidgets cases to inject a translation implementation whose
threadReplyCountText values differ from the default English strings. Assert the
injected singular and plural values instead of hard-coded English labels,
ensuring both counts verify that StreamMessageItem uses the provided
translation.
🪄 Autofix
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: a499aaa7-9d1d-42fb-ac19-9cd81254da70
📒 Files selected for processing (9)
packages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/scroll_view/channel_scroll_view/stream_channel_list_item.dartpackages/stream_chat_flutter/test/src/message_widget/stream_message_item_test.dartpackages/stream_chat_flutter/test/src/scroll_view/channel_scroll_view/stream_channel_list_item_test.dartsample_app/integration_test/channel_list_test.dartsample_app/integration_test/pages/channel_list_page.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/quoted_reply_test.dartsample_app/integration_test/robots/user_robot.dart
🚧 Files skipped from review as they are similar to previous changes (6)
- sample_app/integration_test/pages/channel_list_page.dart
- sample_app/integration_test/quoted_reply_test.dart
- sample_app/integration_test/channel_list_test.dart
- sample_app/integration_test/robots/user_robot.dart
- sample_app/integration_test/pages/message_list_page.dart
- packages/stream_chat_flutter/CHANGELOG.md
The channel.getReplies, thread-replies label and channel-preview fixes (plus their unit tests and changelog entries) move to a stacked PR so this one carries only e2e suites and CI. The e2e tests covering them fail here until that PR lands. The StreamEphemeralMessage export stays: the e2e page object references it, so this branch would not compile without it.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submit a pull request
Linear: FLU-605
Linear: FLU-606
CLA
Description of the pull request
Ports two more e2e suites to the Flutter sample app — ChannelList (13 tests) and QuotedReply (32 tests) — taking the suite to 82 tests, 2 skipped. Both are ports of the native Android/iOS suites and reuse identifiers the SDK already exposes, so no test-only keys were added to the SDK.
e2e suites
channel_list_test.dart— ported from AndroidChannelListTests.kt+ iOSChannelList_Tests.swift(SwiftUI and UIKit, which is where the two truncation cases come from).quoted_reply_test.dart— ported from AndroidQuotedReplyTests.ktplus the 4 cases that only exist in iOS SwiftUI'sQuotedReply_Tests.swift.channel_list_page.dart,message_list_page.dart) and assert mixins for the channel list and message list;widget_test_extensions.dartgained the sharedrenderedText/ visibility helpers both suites need.CI
batchmatrix per platform, mirroring android'se2e-test.ymland swift'ssmoke-checks.yml. Slicing is per test file, since a file is oneflutter testinvocation = one native build.allure_launchexportsLAUNCH_IDper job, batching required hoisting it into its own job — one per platform (allure_launch_android/allure_launch_ios), via the new.github/actions/allure-launchcomposite action. They must stay separate: the reporter'shistoryIdcarries no platform, so a shared launch would read Android and iOS results for the same test as retries of one test and mask a failure on one platform.SDK issues found by the suites
Filed & Fixed — moved to #2874
The SDK fixes below, with their unit tests and changelog entries, live in #2874, which is based on
masterand reviewable on its own. The e2e tests covering them stay here and fail until #2874 lands andmasteris merged into this branch — expected, not a regression.🔼 FLU-668 —
Channel.getRepliesadded the parent message to the thread, so the root rendered twice.🔼 FLU-669 — the thread-replies footer hardcoded
'$replyCount replies', reading "1 replies" and never localized. Asserted byquoted_reply_test.dart.🔼 FLU-685 — a truncated channel kept its timestamp next to "No messages yet". iOS hides it (UIKit scenario 284); the ported e2e test had been written to Flutter's behaviour instead and now asserts
isDisplayed: false.🔼 FLU-686 — a channel-list row could preview another channel's last message after a reorder. Surfaced by review, not by a test.
Still fixed here, since it is sample-app code the suites drive rather than an SDK change:
🟢
sample_app'sthread_page.dartdroppedshowScrollToBottom: false, so threads get the same scroll-to-bottom button as channels (matchingchannel_page.dartand the native suites).⚪
StreamEphemeralMessageis now exported (with a changelog entry) — the onlypackages/change left in this PR, becausemessage_list_page.dartreferences it and the suites would not compile otherwise.Filed
🔴 FLU-670 — the message-list loading skeleton overflows in a short viewport (Android only, so skipped only there).
🔴 FLU-671 — jumping to a quote inside a thread doesn't page in a target that isn't loaded.
Test harness
streamTest/streamTestWithEnvgainedskipPlatforms, so askip:can be narrowed to the platform where the issue reproduces. Used by one test whose failure is Android-only.streamTestnow logs the fullFlutterErrorDetails—takeException()hands back only the exception, so a layout error used to report "203 pixels" without ever naming the widget.find.byType(X).at(n)) throw when the list hasn't rendered yet instead of resolving empty, which killed two Android tests mid-poll;evaluateSafely()now absorbs that so the wait loops actually wait.How this was tested
Whole
integration_test/dir on the iPhone 17 (iOS 26.5) simulator, plus per-suite runs on device, taken while the SDK fixes were still on this branch. With them moved to #2874 the suites covering FLU-668/669/685 fail here by design.dart analyze --fatal-infosis clean onintegration_test/and onstream_chat_flutter.Screenshots / Videos
N/A — test and CI changes only; the SDK fixes and their user-visible effects moved to #2874.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
StreamEphemeralMessageto the public Flutter package exports.Bug Fixes
Tests