e2e: Harden the test harness: retry visibility, video on retries only, cleanup#6568
Conversation
- waitForCount now polls at the existing interval instead of spinning the CPU until timeout - MockServer.execute throws on failure instead of calling fail() and returning an unreachable empty Response - Remove unused helpers (stopApp, nameOf, stringOf, testAssets, bottomPoint, leftPoint, toSeconds, UiObject2.width, waitForTextToChange) - Drop the redundant ParticipantRobot.sleep and the no-op parentId parameter on readMessage - Fix pinMesage/unpinMesage typos
…E split - Rename io.getstream.chat.android.compose.uiautomator to io.getstream.chat.android.e2e.test.uiautomator so the package matches its location and the module's other packages (mockserver, robots, rules) - Move VisualOrder into the e2e-test module's uiautomator toolkit (now public), since it is app-agnostic infrastructure - Document the module-vs-sample E2E convention in AGENTS.md
- Each failed attempt that is retried is written as its own Allure result sharing the real test's historyId, with that attempt's steps, error and artifacts, so TestOps groups attempts as retries and can flag flaky tests - Screen recording now runs only on retry attempts; previously every test, including the ~99.9% that pass on the first attempt, recorded and discarded an 8 Mbit/s video - Remove the unused Retry annotation and simplify DatabaseOperations (private, no self-instantiation) - Name Allure launches 'Cron checks' only for real scheduled events, so manual dispatches of the nightly workflow no longer pollute TestOps analytics
… split - assertSystemMessage asserted nothing: the positive path discarded the check and the negative path never throws; both now assert via a shared assertVisibility helper - Collapse the duplicated reaction selector mappings into a single By.res template - batch_tests now splits round-robin, always yielding exactly batch_count groups; the previous rounded-up slicing could produce fewer groups and crash on nil for the last batch - Remove a redundant 5s sleep before an assertion that already polls for disappearance
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
SDK Size Comparison 📏
|
WalkthroughThe PR moves shared UIAutomator utilities into the E2E module, updates Compose sample consumers, refactors retry and batching behavior, improves Allure failure reporting, and documents E2E test structure and execution. ChangesE2E UIAutomator contracts
E2E runtime and sample integration
Execution and documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RetryRule
participant Test
participant DatabaseOperations
participant Allure
RetryRule->>Test: execute attempt
Test-->>RetryRule: failure
RetryRule->>DatabaseOperations: clear databases
RetryRule->>Allure: write failed attempt artifacts
RetryRule->>Test: retry or throw final exception
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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 `@fastlane/Fastfile`:
- Around line 154-159: Validate batch_count immediately after parsing in the
batch-processing logic, raising UI.user_error! with a clear message when it is
zero or negative before the modulo operation. Then validate that batch is within
0...batch_count and report an out-of-range batch index explicitly; retain the
existing empty-batch error only for valid indices that receive no tests. Use the
existing batch, batch_count, and current_batch logic to make these checks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aa6f9969-8f7b-453c-8a25-e0cb3da66024
📒 Files selected for processing (25)
AGENTS.mdfastlane/Allurefilefastlane/Fastfilestream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/pages/MessageListPage.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobot.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobotChannelListAsserts.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobotMessageListAsserts.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AuthTests.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/ChannelListTests.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/DraftMessagesTests.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/MessageListTests.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/QuotedReplyTests.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/ReactionsTests.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/StreamTestCase.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/mockserver/MockServer.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/robots/ParticipantRobot.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/rules/RetryRule.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Actions.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Base.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Element.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/FindBy.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Math.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Permissions.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/VisualOrder.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Wait.kt
…llure TestOps picks the same-historyId result with the latest start as the launch's current one. The real result starts a few ms before any retry attempt, so the attempt results were shown as current and a test that passed on retry displayed as broken. Each attempt's start is now shifted just below the real result's start, keeping attempt order and leaving the final result current. Verified by uploading pre-fix and post-fix result sets to scratch TestOps launches.
|
|
🚀 Available in v7.6.0 |
… on retries only (#1746) - RetryRule: each failed attempt that is retried is written as its own Allure result sharing the real test's historyId, with that attempt's steps, error and artifacts, so TestOps groups attempts as retries and can flag flaky tests - RetryRule: screen recording runs only on retry attempts; previously every test recorded an 8 Mbit/s video that was discarded on pass - RetryRule: the recording file uses methodName instead of displayName (the display name's parentheses break when the recording commands go through the device shell), and a recording stop failure no longer replaces the test result - RetryRule: move to io.getstream.video.android.rules (the file declared a chat package), remove the unused Retry annotation, simplify DatabaseOperations - Wait: waitForText and waitForCount poll at 50ms instead of spinning the CPU until timeout; remove the unused waitForTextToChange - Allurefile: name launches 'Cron checks' only for real scheduled events, so manual dispatches stay distinguishable in TestOps - Fastfile: batch_tests splits round-robin, always yielding exactly batch_count groups; the previous rounded-up slicing could produce fewer groups and crash on nil for the last batch Ported from GetStream/stream-chat-android#6568.



Goal
Make the E2E pipeline cheaper and its flakiness measurable: report retry attempts to Allure TestOps, record video only on retries, and clean up the harness.
Resolves AND-1302
Implementation
RetryRule: each failed attempt that is retried becomes its own Allure result (same historyId), so TestOps shows retries and can flag flaky tests. Before, a pass-on-retry looked identical to a stable pass.RetryRule: screen recording runs only on retry attempts. Before, every test recorded an 8 Mbit/s video and discarded it on pass.assertSystemMessageasserted nothing; fixed, and six copies of the appear/disappear assert pattern merged into one helper.waitForCountpolls at 50ms instead of spinning the CPU until timeout.batch_testssplits round-robin (the old slicing could crash on the last shard), and manual runs of the nightly workflow are no longer named "Cron checks" in TestOps.pinMesagetypos fixed, the uiautomator package now matches its location, and the module-vs-sample convention is documented in AGENTS.md.Out of scope: the remaining fixed sleeps are wall-clock by design (token expiry, cooldown) or load-bearing; they belong to the flaky-test work in AND-1303.
Metrics
E2E CI duration. Wall clock is what the Actions page shows (the 3 batches run in parallel); total is the compute across the 3 batch jobs. Baselines are the four most recent full runs on other branches:
Shared runners, so the nightly trend after merge is the authoritative signal.
Screen recordings: 99.9% of nightly test executions pass (27,704 of the last 27,734) and every one recorded a video that was thrown away. Now only retries record.
Retry visibility:
It already fired for real:
test_threadIsNotLocked_afterParentMessageDeletedByParticipantflaked during this PR's CI run and its failed attempt is visible in the TestOps launch.Local A/B (full ReactionsTests class, same emulator and mock server): 124.2s on develop vs 113.6s on this branch (-8.5%), 10/10 passed on both.
Testing
With the mock server and an emulator running:
ReactionsTests#test_addsReactionviaconnectedE2eDebugAndroidTest, then checkadb shell ls /sdcard/googletest/test_outputfiles/allure-results(one-result.json, no.mp4).historyId, video attached only to retry attempts.No UI changes.