ci(repo): Add E2E reactions tests#2825
Open
testableapple wants to merge 21 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAndroid E2E tests now capture failure diagnostics and per-test videos, selectively retry failed cases, and attach recordings to Allure results. Integration tests add reaction scenarios and deterministic offline/online controls, while Allure supports structured steps and binary or text attachments. ChangesE2E observability and reaction testing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant E2ETest
participant Allure
participant Fastlane
participant CI
E2ETest->>Allure: Emit test result and status markers
E2ETest->>Fastlane: Save recording manifest
CI->>Fastlane: Run attach_videos after failure
Fastlane->>Allure: Add clipped videos to failed results
Possibly related PRs
🚥 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 |
testableapple
force-pushed
the
ci/e2e-reactions-tests
branch
from
July 17, 2026 14:47
6186245 to
d212baf
Compare
testableapple
force-pushed
the
ci/e2e-reactions-tests
branch
from
July 17, 2026 14:49
d212baf to
08bb32a
Compare
testableapple
marked this pull request as ready for review
July 21, 2026 11:38
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2825 +/- ##
=======================================
Coverage 71.35% 71.35%
=======================================
Files 430 430
Lines 26945 26945
=======================================
Hits 19226 19226
Misses 7719 7719 ☔ 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: 2
🧹 Nitpick comments (1)
sample_app/integration_test/support/stream_test_case.dart (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSkip reason is silently discarded.
skipis accepted as a descriptiveString?but converted toskip: skip != nullfortestWidgets, which only acceptsbool?. Since the callback body (where the reason could otherwise be logged) never runs for skipped tests, the actual reason never surfaces anywhere — not in CI output, not in Allure. Consider folding the reason into the test description so it's visible in test run output.💡 Suggested fix
IntegrationTestWidgetsFlutterBinding.ensureInitialized(); - testWidgets(description, skip: skip != null, (tester) async { + final effectiveDescription = skip != null ? '$description (skipped: $skip)' : description; + testWidgets(effectiveDescription, skip: skip != null, (tester) async {"
@isTestvoid testWidgets( String description, WidgetTesterCallback callback, { bool? skip, test_package.Timeout? timeout" confirmsskipontestWidgetsis strictlybool?, so a reason string can't be passed through directly — please double check there isn't already a downstream consumer (e.g. Fastlane's retry parser) that expects the raw reason.Also applies to: 74-79
🤖 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/support/stream_test_case.dart` around lines 18 - 27, The streamTest helper discards the descriptive skip reason by passing only a boolean to testWidgets. Update streamTest so when skip is non-null, the reason is incorporated into the test description supplied to testWidgets, while preserving the original description when no skip reason exists and continuing to mark the test as skipped.
🤖 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 196-227: Update the retry loop around start_screen_recording, the
videos[test_name] assignment, and failed-result attachment so each attempt
preserves its own recording file and metadata instead of overwriting the fixed
test-name path. Ensure failed Allure results are matched to the per-attempt
recording whose start/end time window contains their timestamps, including when
later retries narrow the test set or pass.
- Around line 24-38: Update start_screen_recording to pass an explicit
--time-limit value to the adb shell screenrecord command, using a duration
sufficient for the longest supported test run. Preserve the existing
Android-only behavior, device serial handling, output path, and process
metadata.
---
Nitpick comments:
In `@sample_app/integration_test/support/stream_test_case.dart`:
- Around line 18-27: The streamTest helper discards the descriptive skip reason
by passing only a boolean to testWidgets. Update streamTest so when skip is
non-null, the reason is incorporated into the test description supplied to
testWidgets, while preserving the original description when no skip reason
exists and continuing to mark the test as skipped.
🪄 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
Run ID: 660b22e3-96f8-44af-a713-8dc8c7d8ca10
📒 Files selected for processing (19)
.github/workflows/e2e_test.yml.github/workflows/e2e_test_cron.ymlsample_app/fastlane/Allurefilesample_app/fastlane/Fastfilesample_app/integration_test/allure/allure.dartsample_app/integration_test/message_list_test.dartsample_app/integration_test/mock_server/data_types.dartsample_app/integration_test/pages/message_list_page.dartsample_app/integration_test/reactions_test.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/failure_artifacts.dartsample_app/integration_test/support/step.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/app.dartsample_app/lib/auth/auth_controller.dart
VelikovPetar
approved these changes
Jul 21, 2026
xsahil03x
reviewed
Jul 21, 2026
| /// Only the methods currently used by the ported suites are mirrored here; add | ||
| /// more from [ParticipantRobot] as new suites need to chain them. | ||
| extension ParticipantRobotChain on Future<ParticipantRobot> { | ||
| Future<ParticipantRobot> readMessage() async => (await this).readMessage(); |
Member
There was a problem hiding this comment.
we can also use .then instead of awaiting first. Not sure if the syntax is correct.
Suggested change
| Future<ParticipantRobot> readMessage() async => (await this).readMessage(); | |
| Future<ParticipantRobot> readMessage() async => .then((it)=>it.readMessage()); |
xsahil03x
reviewed
Jul 21, 2026
| /// native robots (`userRobot.login().openChannel()`) instead of a block of | ||
| /// sequential `await`s. Each method mirrors the instance method above. | ||
| extension UserRobotChain on Future<UserRobot> { | ||
| Future<UserRobot> login([ |
Member
There was a problem hiding this comment.
xsahil03x
reviewed
Jul 21, 2026
| /// Chainable counterparts to [UserRobotMessageListAsserts], so an assertion can | ||
| /// follow a fluent action chain (`userRobot.addReaction(x).assertReaction(...)`). | ||
| extension UserRobotMessageListAssertsChain on Future<UserRobot> { | ||
| Future<UserRobot> assertMessage(String text) async => (await this).assertMessage(text); |
Member
There was a problem hiding this comment.
xsahil03x
approved these changes
Jul 21, 2026
xsahil03x
left a comment
xsahil03x
left a comment
Member
There was a problem hiding this comment.
Added minor comments, Otherwise LGTM
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
3 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.
Resolve https://linear.app/stream/issue/FLU-604
CLA
Description of the pull request
Adds an end-to-end test suite for message reactions to the sample app's integration tests, plus the harness plumbing needed to run them reliably in CI.
New tests (
integration_test/reactions_test.dart)Covers the full reaction matrix against the mock server:
Deterministic offline/online simulation
Real network toggling isn't reproducible in CI, so the harness drives connectivity itself:
StreamTestEnv.goOffline()/goOnline()simulate a full outage and recovery, then wait for the WebSocket to reach the expectedConnectionStatus.debugConnectivityStreamis injected into theStreamChatwidget in place of the realconnectivity_plusmonitor (null in production).debugForceOfflineflag +chatApiInterceptorsreject every HTTP request while "offline", so the HTTP and WebSocket halves stay consistent.FOREIGN KEYerror that poisoned the test binding.Failure artifacts & CI
support/failure_artifacts.dart,allure/allure.dart, Fastfileattach_videoslane).e2e_test.yml/e2e_test_cron.ymlgained an "Attach videos to failed tests" step (installsffmpeg, runs the slicing lane on failure).skipparameter to thestreamTest/streamTestWithEnvhelpers.Known issue
The "user adds a reaction while offline" test is currently skipped pending FLU-506.
Screenshots / Videos
N/A — CI-only integration tests.
Summary by CodeRabbit
Summary by CodeRabbit