Stabilize the thread quote jump test and stop dropping focused-message jumps - #6594
Conversation
|
@CodeRabbit review |
✅ Action performedReview finished.
|
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
WalkthroughThe changes improve Android UI test interaction reliability, synchronize focused-message scrolling with list settling, update the message-list selector, and re-enable a quoted-reply end-to-end test using target-based scrolling. ChangesQuoted reply reliability
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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
`@stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobot.kt`:
- Around line 335-347: The message visibility logic in the surrounding scroll
method must validate both the target’s top and bottom against
MessageList.messageList.visibleBounds, not just listTop, and perform that
validation after every settled swipe, including the final scrollMessageListUp
call. Reorder the loop so each swipe settles before checking the target, while
preserving the existing success return and retry behavior.
In
`@stream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Wait.kt`:
- Around line 80-87: Update the retry loop in the timeout-aware wait method
around waitToAppear and StaleObjectException so it stops when the remaining time
is non-positive, rather than granting POLL_INTERVAL_MILLIS beyond endTime.
Preserve the last StaleObjectException and rethrow it when the requested timeout
expires, avoiding another waitToAppear call after the deadline.
🪄 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 Plus
Run ID: d50acb6c-d743-45a1-bac4-6200b7977acf
📒 Files selected for processing (5)
stream-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/tests/QuotedReplyTests.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/list/Messages.ktstream-chat-android-e2e-test/src/main/kotlin/io/getstream/chat/android/e2e/test/uiautomator/Wait.kt
SDK Size Comparison 📏
|
f2e0468 to
9f54290
Compare
…essage The focused-message scroll effect skipped the jump when the list was still scrolling, and the focused state is consumed either way, so a jump requested while the list settles was silently dropped and never retried. Wait for the scroll to end instead of skipping.
waitToAppearAndClick only covered single-object selectors; clicks on an object picked by index from findObjects still threw StaleObjectException when the containing list refreshed between the find and the click. Mirror the same re-find and retry contract for the indexed lookup.
Three failure modes made the test flaky. The fixed count of eight scroll-ups could leave the target message clipped at the viewport edge, where its text is not laid out and a long press does nothing: scroll until the message is fully inside the viewport instead. A tap on the quoted message that lands while the list is still moving is cancelled by the touch slop and never reaches the click handler: verify the jump moved the quote out of the viewport and tap again when it did not. The channel click at the start of the test could hit a stale node: open the channel through the stale-safe indexed click. Also fixes the MessageListPage list selector, which pointed at a resource id that does not exist.
Moves the body of the focused-message scroll effect into an internal LazyListState extension so the guard, the idle jump, and the wait-out-an-ongoing-scroll behavior can be exercised in unit tests.
States why scrollMessageListUpToMessage only checks the top edge of the list viewport, and documents the final grace poll of the indexed waitToAppearAndClick the same way its selector-based sibling does.
With the clock auto-advancing, runOnIdle finished the scroll animation before scrollToFocusedItem started, so the test passed even with the wait reverted to the old skip behavior. Pause the main clock, launch the jump in the middle of the animation, and assert it stays parked until the scroll ends.
9f54290 to
4826025
Compare
|
|
🚀 Available in v7.7.0 |


Goal
test_quotedReplyNotInList_whenUserAddsQuotedReply_InThread(AllureId 5892) was ignored in #6592 after failing the e2e batch on unrelated PRs. Local soak runs (30 to 40 repetitions per iteration) surfaced four distinct causes, three in the test flow and one in the SDK. This stabilizes the test and re-enables it.Resolves AND-1332
Implementation
scrollMessageListUpToMessagescrolls one page at a time until the message is fully inside the list viewport after the fling settles.focusMessagecall in the failing attempts).tapOnQuotedMessagenow verifies the jump moved the quote out of the viewport and taps again when it did not.openChannelclick picks an object by index fromfindObjects, a path the stale-safe helpers did not cover, and threwStaleObjectExceptionwhen the channel list refreshed between the find and the click. AddedwaitToAppearAndClick(withIndex)to the toolkit, mirroring the existing selector-based contract, and used it inopenChannel.MessageListPage.messageListselector (Stream_MessageList, which exists nowhere) now points at the realStream_Messagestag.Testing
Local soak evidence, same emulator and mock server for all iterations:
openChannelpath).For reviewers: run
QuotedReplyTests#test_quotedReplyNotInList_whenUserAddsQuotedReply_InThreadrepeatedly against the mock server; before this change it fails within a handful of attempts, after it it stays green.Summary by CodeRabbit
Bug Fixes
Tests