Skip to content

feat(e2e): Migrate RF tests to detox. Optimize CI to run faster. Run smoke test by default#9694

Open
yasserfaraazkhan wants to merge 235 commits intomainfrom
SEC-9886-2
Open

feat(e2e): Migrate RF tests to detox. Optimize CI to run faster. Run smoke test by default#9694
yasserfaraazkhan wants to merge 235 commits intomainfrom
SEC-9886-2

Conversation

@yasserfaraazkhan
Copy link
Copy Markdown
Contributor

Jira:

Summary

This PR has three main areas of work:

  1. E2E test migration — Rainforest → Detox & Maestro

Migrates a large set of Rainforest test cycles to Detox and Maestro, covering:

  • Channel bookmarks (create, edit, delete, permissions, search)
  • iPad (account view, channel navigation, post message, sidebar)
  • Messaging (file preview/upload, gallery, image attachment options, GIF, mark as unread, pin/unpin, emoji display, misc)
  • Search (recent mentions, search behaviors)
  • Threads smoke test (updated)
  • Share extension (share link, text, image to channel)
  • Maestro flows: channel bookmark file, multi-device messaging, timezone, help URL
  1. CI optimizations
  • Build cache: JS-only PRs skip native rebuilds (~15 min → ~18s); Android limited to single ABI for CI emulator
  • Smoke test on every PR: messaging smoke test runs on all PRs, not just scheduled runs
  • iPad Detox CI job: new scheduled job targeting iPad Pro 13-inch (M5) / iOS 26.3
  • Maestro jobs: integrated into both PR and scheduled workflows
  1. App-side bug fixes (found while writing/running E2E tests)

Crashes:

  • scrollTo -Infinity RN bridge crash from keyboard animation (useKeyboardScrollAdjustment)
  • progressViewOffset: NaN RCTRefreshControl crash + CALayer position contains NaN (3 propagation paths through post list and keyboard hook)
  • OpenGraph null crash when page has no <title> tag
  • Thread transformer crash when raw.post is undefined
  • processReceivedThreads crash on null post/participants fields

Logic bugs:

  • Saved posts race condition: unsaving a post caused the bookmark icon to snap back due to an immediate WebSocket PREFERENCE_CHANGED event; fixed with
    EphemeralStore tracking + stale event filter across post list, thread overview, post options, and saved messages
  • savePreferences was fire-and-forget (errors silently swallowed)
  • markThreadAsUnread showed stale unread count until WS event arrived
  • Thread reply_count not updated locally on new reply; missing auto-follow when ThreadAutoFollow is enabled
  • Archived posts always included in search due to Boolean('false') === true
  • Users kicked from archived channels when ExperimentalViewArchivedChannels was unset (undefined treated as not-'true' → kick fired)
  • Pinned messages shown in reversed order (spurious .reverse())
  • Archive channel modal not dismissed in some modal stack states (dismissModal → dismissAllModals)
  • Category collapse animation not cancellable on rapid taps (useDerivedValue → useSharedValue with cancelAnimation)

Also merged from main: GIF animation URL fix, SQLite concurrent access crash (iOS, 36K users), Android RNN race condition, keyboard stuck fix, pre-auth
header case-insensitive check, duplicate hashtag search, search results layout overlap.

NONE

yasserfaraazkhan and others added 30 commits March 12, 2026 12:26
- Add opened/synchronize/reopened triggers to e2e-detox-pr.yml so
  smoke tests fire on every PR without requiring a manual label
- Fix concurrency group to use label name || 'smoke' so default runs
  cancel each other without cancelling full-suite label runs
- Remove gradle clean from detox Android build commands (CI workspaces
  are fresh per-run; clean destroys incremental compilation)
- Suppress RCTImageView LogBox warning and Metro debugger toast in E2E

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Revert ios_device_os_name default from iOS 26.3 back to iOS 26.2
  (CI runners have iOS 26.2 runtime, not 26.3)
- Remove hardcoded iOS 26.3 fallback in DEVICE_OS_VERSION env var
- Redesign detox/CLAUDE.md as AI agent operating manual with safety
  rules, CI integration docs, debugging guide, and full coverage map

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use pixel_4_xl instead of pixel_8 — the pixel_8 device definition
is not available on GitHub Actions ubuntu runners. The hardware
profile is overridden by android_emulator/config.ini anyway, so
the -d flag only needs a valid device name that exists on the runner.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add actions/cache restore/save around build steps in all four E2E build
jobs (iOS smoke, Android smoke, iOS full, Android full). Cache key is
a hash of ios/**, app/**, libraries/**, package-lock.json, patches/**.

- Cache hit: skip all build steps (~22min iOS, ~24min Android saved)
- Cache miss: build normally then save for subsequent runs
- PRs that only change CI config, docs, or tests will always hit cache

Also update Android test runner artifact download path to match the
new narrower upload path (outputs/apk/ instead of build/**/*).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lback

- Restore __DEVICE_NAME__ / __DEVICE_OS_VERSION__ placeholders in
  .detoxrc.json (were hardcoded to iPhone 17 Pro / iOS 26.3 by mistake)
- Restore android avdName to original detox_pixel_4_xl_api_34
- Fix generate_detox_config_ci.js fallback from iOS 26.3 to iOS 26.2

Config-gen replaces the placeholders at runtime from DEVICE_NAME and
DEVICE_OS_VERSION env vars set by the CI workflow. Without the
placeholders, the replace() had no effect and iOS 26.3 was used.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add detox build-framework-cache step after npm install in the iOS E2E
template, with caching keyed by OS/arch, Detox version, and Xcode version
to avoid rebuilding on every run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GitHub Actions runners have no hardware GPU/Vulkan support. The emulator
was launched with -gpu host which overrode -gpu off in emulator_opts,
causing VK_ERROR_INCOMPATIBLE_DRIVER and emulator hang. Switch to
-gpu swiftshader_indirect (software renderer) for CI/Linux which works
headless without hardware GPU.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
.detoxrc.json had detox_pixel_4_xl_api_34 hardcoded while CI creates
detox_pixel_8_api_35. Replace with __AVD_NAME__ placeholder and extend
generate_detox_config_ci.js to substitute it from the AVD_NAME env var.
Call detox:config-gen in create_android_emulator.sh before running tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix dot-location ESLint violation in generate_detox_config_ci.js:
  move method chain dots to end of line (linter had moved them to start)
- Add 3-attempt retry loop for sdkmanager install to handle transient
  zip corruption errors when downloading system images on CI runners

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- messaging.e2e.ts: Add ThreadScreen.toBeVisible() + 1s wait before
  openPostOptionsFor() in MM-T4786_4 — the flat list scroll was failing
  because the thread screen animation hadn't settled after tap(), causing
  the RCTCustomScrollView to fail Detox's 100% visibility check for scroll.
  This also fixes the cascade failure of MM-T4786_5/6/7 whose beforeEach
  was failing because the app was left stuck in ThreadScreen.

- server_login.e2e.ts: Check apiAdminLogin(siteTwoUrl) return value and
  throw on error — previously a silent failure (e.g. account lockout)
  left no valid session, causing the subsequent apiInit to return a
  cryptic session_expired instead of the real admin login error.

- config.js: Increase testTimeout from 180s to 240s (low-bandwidth: 300s)
  so the global beforeAll in setup.ts (app launch + wait-for-ready +
  admin login) has enough headroom on the slow iOS 26 simulator, preventing
  the timeout that caused session_expired errors in account.e2e.ts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… UITransitionView

iOS UIKit leaves a UITransitionView backdrop in the view hierarchy after a
modal sheet is dismissed. The default scroll start (y=5%, ~37px from top)
falls within this blocked area, causing MM-T4786_4 to fail. Starting the
scroll from y=50% (center) avoids the lingering backdrop entirely.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace per-file delete:true reinstall with process.env flag so only the
  first test file per CI run does a full clean install (~85s); subsequent
  files use newInstance:true restart (~5s), fixing 'Exceeded timeout of
  240000ms' failures on account.e2e.ts and autocomplete.e2e.ts on iOS.

- Reduce MAX_RETRY_ATTEMPTS 3→2: worst case is now 2×85s+5s=175s < 240s
  even if the first-file install is slow on iOS 26.x simulator.

- Replace waitForAppReady() with ensureOnServerScreen() which handles all
  three inter-file app states before each test file's beforeAll runs:
  1. server.screen — clean state, proceed immediately
  2. channel_list.screen — HomeScreen.logout() failed silently in previous
     afterAll; forces a cleanup logout inline so server is removed
  3. server_list.screen — server_login.e2e.ts logs out Server 2 via the
     server list (keeps it as inactive entry); taps "Add a server" to open
     server.screen so threads.e2e.ts beforeAll can connect normally

- Change subsequent launch from newInstance:false (resumes mid-transition
  process) to newInstance:true (fresh process reads WatermelonDB from disk);
  after a successful logout the DB has no servers → app shows server.screen.

- server_login.e2e.ts: skip MM-T4675_2 gracefully when Site 2 admin login
  fails (account locked from accumulated CI failures) instead of throwing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… JS-only PRs

- Remove app/** from iOS/Android build cache keys so JS-only changes
  hit the binary cache instead of forcing a full rebuild (~90% of PRs)
- Add Xcode build intermediates cache (ios/Build/Products) with
  restore-keys fallback for incremental native-change builds
- Skip rm -rf ios/Build/Products in simulator lane (ci_optimized=true)
  and add COMPILER_INDEX_STORE_ENABLE=NO + DEBUG_INFORMATION_FORMAT=dwarf
  flags to save 3-8 min per Xcode build
- Enable org.gradle.caching=true and org.gradle.parallel=true for
  Gradle build output caching (FROM-CACHE on unchanged modules)
- Add ~/.gradle/build-cache caching in CI for Android build jobs
- Restrict E2E Android builds to arm64-v8a only (emulator is arm64-v8a)
  cutting C++/JNI compile time by ~50-60%
- Cache detox/node_modules to save 30-60s per Android build
- Add missing e2e:android-inject-settings in scheduled workflow
- Apply all changes to both e2e-detox-pr.yml and e2e-detox-scheduled.yml

Expected outcomes:
  JS-only PR: 25-45min → 1-2min (cache hit)
  Native-change PR warm cache: 25-45min → 5-10min (incremental)
  Android JS-only PR: 20-30min → 1-2min (cache hit)
  Android native-change warm cache: 20-30min → 5-8min (FROM-CACHE + 1 ABI)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit touches only app/** (TypeScript) and no native files.
Expected: iOS and Android build jobs should find cache-hit=true and
complete in under 2 minutes without running Xcode/Gradle.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sh drift

The prepare-android-build action appends signing credentials to
android/gradle.properties (lines 49-51 of the action), and
prepare-ios-build installs CocoaPods into ios/Pods. Both modify
paths included in the hashFiles() expression. Since hashFiles()
is evaluated at step-run time, the restore key (computed before
prepare) differs from the save key (computed after prepare),
causing the binary cache to never hit.

Fix: add a dedicated "Compute {platform} build cache key" step
immediately after checkout (before prepare), capturing the hash
into a step output. All restore and save steps then reference
${{ steps.{id}.outputs.hash }} so restore and save always use
the same pre-prepare hash.

Applied to all 4 build jobs in e2e-detox-pr.yml and 2 build
jobs in e2e-detox-scheduled.yml.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Only app/utils/url/index.ts changed. Cache keys are based on
ios/**, android/**, libraries/**, package-lock.json, patches/**
— none of which changed. Expected: both iOS and Android build
jobs should show cache-hit=true and complete in under 2 minutes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The detox_pixel_8 emulator on GitHub's ubuntu runners uses the
system-images;android-35;google_apis;x86_64 image (confirmed in
e2e-android-template.yml line 231). Building arm64-v8a only caused
libworklets.so (react-native-reanimated) to be missing from the APK,
crashing the app on every test launch.

Fix: restrict E2E builds to x86_64 instead of arm64-v8a.
Still saves ~50-60% compile time vs the previous 4-ABI build
(armeabi-v7a,arm64-v8a,x86,x86_64).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The APK cache key lacked the target ABI, so an arm64-v8a APK built in
an earlier run was being restored and used on x86_64 emulators, causing
libworklets.so crash on every test. Including x86_64 in the key ensures
ABI changes always produce a distinct cache entry.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
iOS UIKit leaves a UITransitionView backdrop after a modal is dismissed.
Scrolling the post list from y=50% before the longPress clears the blocked
area (same fix applied to thread.ts in 0c9476c). Also adds a 1s settle
wait on Android to prevent longPress race conditions on slow emulators.

Fixes MM-T4786_1-7 (messaging smoke) and MM-T4811_1-2 (threads smoke).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove invalid inputs.MOBILE_VERSION ref from scheduled Android checkout
  (scheduled workflow has no inputs context — only workflow_dispatch does)
- Fix set -e short-circuit in parallel lint+tsc wait loop in action.yaml;
  wrap with set +e/set -e and quote PIDs to collect both exit codes correctly
- Guard ChannelScreen.back() in at_mention afterAll with try/catch for iOS 26
- Quote \$GITHUB_OUTPUT in e2e-ios-template.yml (shellcheck SC2086)
- Fix create_android_emulator.sh: array assignment for TEST_FILES, pixel_4_xl
  device profile corrected to pixel_8, array expansion in run_detox_tests call
- Add comment explaining MM-T3392 and MM-T4882 test coexistence in channel_post_draft
- Parameterize iPhone 17 Pro / iOS 26. in disable_ios_autofill.js with env vars
- Add fenced code block language tag in detox/CLAUDE.md
- Consolidate duplicate wait(ONE_SEC) in channel.ts openPostOptionsFor by hoisting
- Remove trailing slash from android APK path in scheduled.yml
- Add comment on shared artifact name between smoke/full iOS build jobs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add eslint-disable no-process-env to match the pattern used by all
other detox/utils files that read from process.env.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Rename detox node_modules cache key detox-npm- → detox-deps- in both
  pr.yml and scheduled.yml to match the key used by e2e-ios-template.yml
  and e2e-android-template.yml (cache was saved but never hit by test jobs)
- Quote \$GITHUB_OUTPUT in all four compute-cache-key steps in e2e-detox-pr.yml
  and both in e2e-detox-scheduled.yml (shellcheck SC2086)
- Fix disable_ios_autofill.js error messages to only append "x" when the OS
  prefix is a major-version wildcard (ends with "."), so a full version like
  "iOS 26.2" is displayed as-is rather than "iOS 26.2x"
- Add DEVICE_NAME / DEVICE_OS_VERSION as intermediate fallbacks for
  IOS_SIMULATOR_DEVICE / IOS_SIMULATOR_OS_PREFIX, aligning with existing
  Detox CI conventions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pixel_8 is not available in the older cmdline-tools version on GitHub
Actions runners. The -d flag is overridden immediately anyway since the
script replaces config.ini wholesale via cp + sed, making the profile
choice cosmetic. pixel_4_xl is the known-working value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove trailing slashes from all android/app/build/outputs/apk paths
  in pr.yml (6 occurrences) and scheduled.yml — aligns with the path
  the Android template uses when downloading artifacts, preventing
  double-nested directory unpacking
- Add && success() to Save Android APK build cache and Save Gradle build
  cache steps in both workflows — prevents caching partial/corrupt
  outputs when the Detox build step fails midway

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
macos-15 runners are Apple Silicon — Homebrew installs to /opt/homebrew,
not /usr/local. The old path caused cache misses on every iOS test run.
Also add runner.arch to the cache key to prevent cross-architecture
cache pollution if Intel runners are used in future.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Apply the same pattern used in ThreadScreen.openPostOptionsFor:
- Scroll on both platforms (not just iOS) to dismiss the keyboard and
  settle the UI before longPress — the keyboard left open after posting
  a message was intercepting the gesture on Android
- Pass TWO_SEC duration to longPress() to match ThreadScreen behavior

Fixes MM-T4811_1 failing with PostOptionsScreen.toBeVisible() timeout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After posting a message the keyboard dismiss animation temporarily blocks
React Native's gesture responder system. A plain longPress fires without
effect during this window even after fixed waits, causing PostOptionsScreen
to never open.

Add longPressWithScrollRetry() helper to support/utils/index.ts:
- Scrolls the post list (dismisses keyboard + settles layout)
- longPresses the target element
- If PostOptionsScreen does not appear within 3s, re-scrolls and retries
- Up to 3 attempts before throwing

Replace the manual scroll+wait+longPress sequences in both
ChannelScreen.openPostOptionsFor and ThreadScreen.openPostOptionsFor with
the shared helper — fixing the issue in all post-after-keyboard flows.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Dismiss keyboard explicitly after every message send in doSubmitMessage
  so the keyboard is gone before the user can long-press a post
- Change keyboardShouldPersistTaps from 'handled' to 'always' in
  post_list FlatList so touch events always reach post items even when
  keyboard dismiss animation is in flight
- Force-relink applesimutils after brew cache restore in iOS CI template:
  cache restores Cellar files but not /opt/homebrew/bin/ symlinks, so
  brew install was a no-op and the binary was missing from PATH
- Guard getAllTests in report.js against empty JUnit XML (all suites
  failed to launch) to prevent generate-report crash

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yasserfaraazkhan and others added 18 commits April 5, 2026 00:37
…Id fix

After the product fix (727d680) that awaits switchToChannelById in
browse channels, the Browse Channels modal is now dismissed as part of
the navigation flow (via dismissAllModalsAndPopToScreen). The test's
closeBrowseChannelsChannel() helper on iOS was unconditionally tapping
the close button, which no longer exists. Check if Browse Channels is
still on screen before attempting to close it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…and channel removal alerts

- archive_channel: add waitForArchivedChannelScreen() that disables
  Detox sync on iOS during modal transition to bypass UITransitionView
  overlay blocking, then re-enables sync after channel loads
- archive_channel/browse_channels/find_channels: dismiss "Removed from
  channel" and "Archived channel" alerts in beforeEach — these appear
  asynchronously via WebSocket events and block all Detox interactions
- alert.ts: add dismissChannelRemoveOrArchiveAlert() helper for both
  "Removed from channel" and "Archived channel" dialog variants
- login.ts: handle both server upgrade dialog variants — "Dismiss"
  (unsupportedServerAdminAlert) and "OK" (GlobalEventHandler), with
  Android uppercase button text ("DISMISS", "OK")
- add_members.ts: dismiss keyboard on iOS before tapping "Add Members"
  button which is behind the keyboard after user search/selection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… bypass

- message_edit: scroll post list after edit screen closes to dismiss
  keyboard and bring the edited post into the fully visible area before
  asserting visibility — fixes 75% threshold failures on iOS where
  keyboard + channel intro header push the post below visible bounds
- archive_channel: apply waitForArchivedChannelScreen() and polling
  helpers to the permalink→channel transition path (MM-T1679_1,
  MM-T1722_1) which also produces UITransitionView overlays on iOS

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ule picker

The schedule picker bottom sheet never appeared because:
- iOS: main run loop never idles, blocking longPress() and waitFor()
- Android: JS bridge stays busy after text input, same blocking effect

Fixes:
- longPressSendButton: disable Detox sync before longPress, poll for
  the schedule picker bottom sheet with waitForElementToExist, then
  re-enable sync in a finally block
- scheduleMessageForMonday/Tomorrow/NextMonday: use polling helpers
  instead of waitFor().toExist() which depends on idle-sync
- Add scheduleMessageForAnyAvailableDay() that picks the first
  available option regardless of day-of-week, handling the case where
  "Monday" isn't shown on Mondays (only "Next Monday" appears)

Affects: MM-T5762, T5767, T5731, T5730, T5720 (both platforms)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes for the "Couldn't load" cascade where a 240s Jest timeout
corrupts app state and all subsequent test files fail:

1. Add 1-second delay after kill -9 in clearIOSAppData() — the
   simulator needs time to release file handles on SQLite WAL/SHM
   files. Without this, the data wipe races with file locks, leaving
   partially recovered database files that cause stale server entries.

2. Add iOS recovery in launchAndVerify() — detect when the app shows
   channel_list.screen instead of server.screen (stale state from
   incomplete wipe), re-run clearIOSAppData(), and relaunch. Mirrors
   the existing Android recovery for pm-clear failures.

Affects: ~12 iOS cascade failures (message_local_drafts, recent_mentions,
channel_mention) that appear after a prior test file times out at 240s.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirror the channel.ts fix for ThreadScreen.longPressSendButton() —
disable Detox synchronization before the long press and poll for the
schedule picker bottom sheet. MM-T5767 schedules a message from a
reply thread, hitting the same iOS 26 main-run-loop idle blocker.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ures

- at_mention MM-T171: type "@" + first 3 chars of username together
  instead of bare "@" alone — avoids noResultsTerm race condition in
  at_mention.tsx where an empty-matchTerm search returns 0 results and
  sets noResultsTerm="" which suppresses all future searches
- pin_and_unpin MM-T142: scroll post list up before asserting newer
  post visibility — after pinning, a system post pushes the post down
  where the input bar clips it below 75% visibility on iOS 26.x
- server_login MM-T4675_2: increase wait after "Add a server" tap to
  TWO_SEC, use toExist() with HALF_MIN timeout instead of
  toBeVisible() with TEN_SEC — bottom sheet animation on iOS needs
  more time to complete on CI runners

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…chSettings

The EnableCrossTeamSearch setting lives under ServiceSettings, not
SearchSettings. The wrong config section meant the API call succeeded
but the setting was ignored — the "All teams" option never appeared
in the team picker dropdown.

Reference: mattermost/mattermost#30518 moved this from a feature flag
to ServiceSettings.EnableCrossTeamSearch in server v10.7.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…failure

- mute_and_unmute: use waitFor().toBeVisible() before asserting toast
  text — the toast renders via Animated.View which on iOS 26 triggers
  the sync blocker, so an immediate expect() finds no element
- channel_settings: wrap initial scroll(100, 'down') in try/catch —
  on newly created channels the settings content is short enough to
  fit without scrolling, causing "Unable to scroll" on iOS

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… FlatList wait

- archive_channel MM-T1719_1: call closeTutorial() BEFORE toBeVisible()
  on Android — the tutorial Dialog creates a separate native window that
  blocks Espresso from finding the Members screen behind it
- alert.ts: increase "Removed from channel" dismiss timeout from 2s to
  4s — WebSocket events can arrive after the check window, causing the
  alert to block subsequent tests (MM-T1679_1 cascade)
- favorite_channel MM-T4929_3: add waitFor().toBeVisible() before
  tapping user item in Create DM — Android FlatList can lag behind
  search input, causing testID lookup to fail

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Maestro testing infrastructure for system-level E2E scenarios:
- CI workflow template (.github/workflows/e2e-maestro-template.yml)
- Test flows for share extension, calls, multi-device sync, timezone
- Utility flows for login, navigation, setup
- Fixtures for seeding test data and polling messages
- Scripts for two-device testing and timezone manipulation
- Report generation utilities for unified Detox/Maestro reporting

Maestro handles scenarios Detox cannot:
- Share extension (cross-app flows)
- System permission dialogs
- Multi-device message sync verification
- Autocorrect/IME behavior testing

See maestro/README.md for setup and usage instructions.
Log server URL, version, build number, build date, build hash, and
enterprise flag after the health check so CI logs make it clear which
exact server build was tested against.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 16, 2026

Coverage Comparison Report

Generated on April 16, 2026 at 12:15:37 UTC

+-----------------+------------+------------+-----------+
| Metric          | Main       | This PR    | Diff      |
+-----------------+------------+------------+-----------+
| Lines           |     85.17% |     85.15% |    -0.02% |
| Statements      |     85.03% |     85.03% |     0.00% |
| Branches        |     72.22% |     72.18% |    -0.04% |
| Functions       |     84.03% |     84.12% |     0.09% |
+-----------------+------------+------------+-----------+
| Total           |     81.61% |     81.62% |     0.01% |
+-----------------+------------+------------+-----------+

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 16, 2026

📝 Walkthrough

Walkthrough

Skip traversal of ipad/ spec files during spec discovery; add iPad/Maestro E2E pipelines and many Detox device/config updates; introduce ephemeral “recently unsaved” saved-post tracking integrated into observables; add numerous test helpers, server API wrappers, CI/workflow changes, testIDs, and assorted app defensive guards.

Changes

Cohort / File(s) Summary
Spec discovery & CI actions
.github/actions/generate-specs/split-tests.js, .github/actions/prepare-node-deps/action.yaml, .github/actions/prepare-ios-build/action.yaml, .github/actions/test/action.yaml
Ignore ipad/ in Specs.findFiles(); simplify node/npm and pods caching; run lint + tsc concurrently and handle exit codes explicitly.
Workflows & templates
.github/workflows/... (compatibility-matrix-testing.yml, e2e-*.yml, e2e-maestro-template.yml, e2e-android-template.yml, e2e-ios-template.yml), .github/workflows/e2e-cancel-on-label-removal.yml
Add iPad/maestro/android jobs, new inputs (search_path, parallelism, detox_config), change defaults (API 35, AVD names), propagate report metrics, add cancel-on-label-removal workflow.
Detox config, emulator & scripts
detox/.detoxrc.json, detox/detox/.detoxrc.json, detox/android_emulator/config.ini, detox/create_android_emulator.sh, detox/e2e/config.js, detox/e2e/global_setup.js
Add iPad simulator entries, bump Android API to 35/Pixel8, tune emulator RAM/skins, add SDK retry logic, add Jest globalSetup to clean zombie simulators and perform server bootstrap (health/auth/config/plugins).
Detox support libs & helpers
detox/e2e/support/utils/index.ts, detox/e2e/support/utils/detoxhelpers.ts, detox/CLAUDE.md, detox/README.md
Add isIpad/isIpad helper, extended timeouts, polling and long-press-retry helpers, contributor guidance and README updates.
Server API helpers for E2E
detox/e2e/support/server_api/* (client.ts, post.ts, plugin.ts, channel_bookmark.ts, command.ts, custom_profile_attributes.ts, system.ts, webhook.ts, index.ts)
New and expanded test-side API wrappers (bookmarks, commands, plugins, files, trial license, webhook checks); HTTP client: IPv4 forcing, 401 re-login and 5xx retry logic, clearCookies export.
Detox screens & tests
detox/e2e/support/ui/screen/*, detox/e2e/test/... (many new/modified specs, including .../ipad/*)
Add ChannelBookmark screen; update 30+ screen objects with polling, timeouts, retry/sync helpers; add many new and updated E2E suites (iPad-specific, bookmarks, file/gallery, messaging, search, threads, etc.).
Ephemeral saved-post tracking & observables
app/store/ephemeral_store.ts, app/queries/servers/post.ts, app/queries/servers/post.test.ts, app/actions/websocket/preferences.ts, app/actions/remote/preference.ts, app/actions/remote/preference.test.ts
Introduce ephemeral “recently unsaved saved post” with 2‑minute expiry, observable subjects, APIs to add/clear/observe; integrate filtering into saved-post observables and preference save/delete flows (awaits and EphemeralStore updates).
Thread, unread & preference flows
app/actions/local/thread.ts, app/actions/local/thread.test.ts, app/actions/remote/thread.ts, app/actions/remote/thread.test.ts, app/actions/remote/search.ts
createThreadFromNewPost now backfills thread from root post when needed and marks following; markThreadAsUnread updates unread counts from REST response; use getConfigBooleanValue for boolean config reads.
UI components — testIDs & accessibility
app/components/**, app/screens/**, app/products/calls/** (30+ files)
Added many testID props and small accessibility tweaks (bottom sheet modal flag, accessible flags) for robust E2E targeting.
Keyboard/animation guards & UI HOCs
app/components/post_list/*, app/components/floating_input/*, app/hooks/useKeyboardAwarePostDraft.ts, app/hooks/useKeyboardScrollAdjustment.ts, app/components/post_with_channel_info/*, app/components/thread_overview/*, app/components/post_list/index.ts
Add finite-number guards to animated values, defensive contentInset handling, Android keyboardDismissMode adjustments, thread/post saved-state HOC changes to accept optional serverUrl and wrap with withServerUrl.
Misc & repo config
android/gradle.properties, .gitignore
Enable Gradle parallel & caching; update .gitignore for mobile artifacts, Maestro files, and Detox outputs.

Sequence Diagram(s)

sequenceDiagram
    participant Client as UI (user)
    participant App as Mobile app logic
    participant Server as REST API
    participant Ephemeral as EphemeralStore
    participant DB as Local observers / Database

    Client->>App: User taps "Unsave post"
    App->>Server: await DELETE /api/v4/preferences
    alt server 200
        Server-->>App: success
        App->>Ephemeral: addRecentlyUnsavedSavedPost(serverUrl, postId)
        Ephemeral-->>DB: emits updated Set / post boolean
        DB-->>App: saved-post observable emits false
        App-->>Client: UI updates (post shown unsaved)
    else server error
        Server-->>App: error
        App->>Client: surface error, do not mark ephemeral saved state
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested Labels

2: Dev Review, Build Apps for PR, 3: QA Review

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch SEC-9886-2

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (9)
detox/e2e/support/ui/screen/notification_settings.ts (1)

45-52: ⚠️ Potential issue | 🟠 Major

Do not swallow back-navigation failures in screen helpers.

Catching and only warning here can let tests pass even when navigation is broken. Please rethrow after logging (or remove the catch) so failures remain actionable.

Suggested fix
     back = async () => {
-        try {
-            await waitFor(this.backButton).toExist().withTimeout(timeouts.TEN_SEC);
-            await this.backButton.tap();
-            await expect(this.notificationSettingsScreen).not.toBeVisible();
-        } catch (error) {
-            // Back button may not exist if screen failed to load or already navigated away
-            console.warn('[NotificationSettingsScreen.back] Navigation failed:', error); // eslint-disable-line no-console
-        }
+        await waitFor(this.backButton).toExist().withTimeout(timeouts.TEN_SEC);
+        await this.backButton.tap();
+        await expect(this.notificationSettingsScreen).not.toBeVisible();
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/notification_settings.ts` around lines 45 - 52,
The current catch block in the back navigation helper for
NotificationSettingsScreen swallows errors (around
waitFor(this.backButton).toExist(), this.backButton.tap(), and
expect(this.notificationSettingsScreen).not.toBeVisible()), which lets
navigation failures go unnoticed; update the catch to log the error (keep the
existing console.warn) and then rethrow the caught error (or simply remove the
try/catch) so the failure surfaces to the test runner instead of being silently
ignored.
detox/e2e/support/ui/screen/auto_responder_notification_settings.ts (1)

40-48: ⚠️ Potential issue | 🟠 Major

Don't swallow failed back navigation here.

If the back button never appears, the tap fails, or the screen stays mounted, this helper still resolves successfully and the test keeps running from the wrong state. That makes the real failure much harder to diagnose.

Suggested fix
     back = async () => {
-        try {
-            await waitFor(this.backButton).toExist().withTimeout(timeouts.TEN_SEC);
-            await this.backButton.tap();
-            await expect(this.autoResponderNotificationSettingsScreen).not.toBeVisible();
-        } catch (error) {
-            // Back button may not exist if screen failed to load or already navigated away
-            console.warn('[AutoResponderNotificationSettingsScreen.back] Navigation failed:', error); // eslint-disable-line no-console
-        }
+        await waitFor(this.backButton).toExist().withTimeout(timeouts.TEN_SEC);
+        await this.backButton.tap();
+        await expect(this.autoResponderNotificationSettingsScreen).not.toBeVisible();
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/auto_responder_notification_settings.ts` around
lines 40 - 48, The back() helper currently swallows navigation failures in its
try/catch, so change it to surface errors: in the back() function, after
awaiting waitFor(this.backButton).toExist(), await this.backButton.tap(), then
await expect(this.autoResponderNotificationSettingsScreen).not.toBeVisible(); if
any of those steps fail, do not just console.warn — rethrow the caught error (or
throw a new Error with context) so the test fails; alternatively, validate the
final state and throw a descriptive error when the screen is still visible.
Ensure the change references the same symbols: back(), this.backButton, and
this.autoResponderNotificationSettingsScreen.
.github/workflows/compatibility-matrix-testing.yml (1)

219-237: ⚠️ Potential issue | 🟠 Major

The final commit status still only reflects the iOS Detox job.

Adding the new jobs to needs is not enough here: lines 235-236 still publish STATUS and TARGET_URL from detox-e2e only. If Android, iPad, or Maestro fails while iOS passes, this workflow will still report green. Please aggregate all new job results before updating the commit status.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/compatibility-matrix-testing.yml around lines 219 - 237,
The step calling mattermost/actions/delivery/update-commit-status@main currently
uses only needs.detox-e2e.outputs.STATUS and TARGET_URL so the final commit
status reflects iOS only; update that step to aggregate outputs from all
dependent jobs (detox-e2e, detox-android-e2e, detox-ipad-e2e, maestro-ios-e2e,
maestro-android-e2e) by computing an overall STATUS (e.g., fail if any job
STATUS == "failure", otherwise success) and choose a TARGET_URL that points to
the first failing job or a combined summary link, then pass those aggregated
values into the update-commit-status inputs instead of the single detox-e2e
outputs.
detox/e2e/test/products/channels/interactive_dialog/interactive_dialog_plugin.e2e.ts (1)

102-107: ⚠️ Potential issue | 🟡 Minor

Missing .tap() call on iOS alert dismissal.

The iOS branch finds the element but never taps it. This will silently fail to dismiss the alert.

🐛 Proposed fix
 async function dismissErrorAlert() {
     try {
-        isAndroid() ? await element(by.text('OK')).tap() : await element(by.label('OK')).atIndex(1);
+        isAndroid() ? await element(by.text('OK')).tap() : await element(by.label('OK')).atIndex(1).tap();
         await wait(300);
     } catch {}
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@detox/e2e/test/products/channels/interactive_dialog/interactive_dialog_plugin.e2e.ts`
around lines 102 - 107, In dismissErrorAlert(), the iOS branch locates the OK
button via element(by.label('OK')).atIndex(1) but never performs the tap; update
the iOS branch to await a .tap() on that element (mirror the Android branch's
await element(...).tap()) so the alert is actually dismissed (references:
dismissErrorAlert, isAndroid, element(by.label('OK')).atIndex(1)).
detox/e2e/support/ui/screen/saved_messages.ts (1)

52-57: ⚠️ Potential issue | 🟠 Major

Ensure savedMessagesTab is visible before tapping in HomeScreen.

The "View is not visible around point" error occurs because HomeScreen.savedMessagesTab.tap() is called without verifying the element is ready. The HomeScreen.toBeVisible() method only checks channelListTab availability, leaving savedMessagesTab unchecked. Add visibility polling for savedMessagesTab in HomeScreen.toBeVisible() to prevent this flake:

toBeVisible = async () => {
    const timeout = isAndroid() ? timeouts.TWENTY_SEC : timeouts.TEN_SEC;
    await waitFor(this.channelListTab).toExist().withTimeout(timeout);
    await waitFor(this.savedMessagesTab).toExist().withTimeout(timeout);
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/saved_messages.ts` around lines 52 - 57, The
HomeScreen.open calls HomeScreen.savedMessagesTab.tap() without ensuring the
savedMessagesTab element is present; update HomeScreen.toBeVisible() to also
wait for savedMessagesTab (in addition to channelListTab) to exist before
returning—use the same timeout logic used for channelListTab (e.g., isAndroid()
? timeouts.TWENTY_SEC : timeouts.TEN_SEC) and call
waitFor(this.savedMessagesTab).toExist().withTimeout(timeout) so
savedMessagesTab is visible before open() taps it.
detox/e2e/support/ui/screen/search_messages.ts (1)

82-87: ⚠️ Potential issue | 🟡 Minor

Add visibility check before tapping searchTab in the open() method.

The searchTab.tap() call needs a visibility wait before it, following the pattern already established in HomeScreen.logout() (lines 40-51 in home.ts). Add waitFor(HomeScreen.searchTab).toExist().withTimeout(timeout) using an appropriate timeout constant before the tap to prevent "View is not hittable" errors across platforms.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/search_messages.ts` around lines 82 - 87, In the
open() method of search_messages.ts, add a visibility wait before calling
HomeScreen.searchTab.tap(): call
waitFor(HomeScreen.searchTab).toExist().withTimeout(timeout) (using the same
timeout constant/pattern used in HomeScreen.logout) immediately before
HomeScreen.searchTab.tap() so the tap only occurs after the element is present
and avoid "View is not hittable" errors.
detox/e2e/test/products/channels/channels/convert_to_private_channel.e2e.ts (1)

82-85: ⚠️ Potential issue | 🟠 Major

Dismiss the scheduled-post tooltip in the cancel flow too.

The first test clears this overlay before navigating away, but this path still goes straight into channel info. If the tooltip renders here, ChannelScreen.back() at the end can leave the suite on the channel screen, which matches the logout timeout showing up in CI.

🛠️ Suggested fix
         await CreateOrEditChannelScreen.openCreateChannel();
         await CreateOrEditChannelScreen.displayNameInput.replaceText(channelDisplayName);
         await CreateOrEditChannelScreen.createButton.tap();
+        await ChannelScreen.dismissScheduledPostTooltip();
         await ChannelInfoScreen.open();
         await ChannelInfoScreen.openChannelSettings();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/channels/convert_to_private_channel.e2e.ts`
around lines 82 - 85, The cancel flow can leave the scheduled-post tooltip
active after creating a channel and opening ChannelInfoScreen; update the test
flow that calls CreateOrEditChannelScreen.openCreateChannel(),
displayNameInput.replaceText(...), createButton.tap(), and
ChannelInfoScreen.open() to explicitly dismiss the tooltip before navigating
away by invoking the scheduled-post tooltip helper (e.g.
ScheduledPostTooltip.dismissIfVisible() or ScheduledPostsTooltip.dismiss())
after ChannelInfoScreen.open() and prior to ChannelScreen.back(), ensuring the
overlay cannot block the back/navigation step.
detox/e2e/test/products/channels/threads/follow_and_unfollow_thread.e2e.ts (1)

180-210: ⚠️ Potential issue | 🟠 Major

Return to the channel list before ending this test.

This case stops on GlobalThreadsScreen, so afterAll logs out from the wrong surface and the suite leaks state into teardown. Please navigate back to the channel list after the final assertion.

As per coding guidelines, "Each test should navigate back to channel list at the end to ensure clean state for the next test".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/threads/follow_and_unfollow_thread.e2e.ts`
around lines 180 - 210, The test ends on GlobalThreadsScreen and never returns
to the channel list; after the final assertion add navigation back to the
channel list by calling await GlobalThreadsScreen.back(); (if that back helper
isn't available in this screen, instead call await
ChannelScreen.open(channelsCategory, testChannel.name)) so the test finishes on
the channel list and does not leak state for subsequent tests.
detox/e2e/test/products/channels/smoke_test/threads.e2e.ts (1)

270-272: ⚠️ Potential issue | 🟡 Minor

Potential navigation state mismatch.

After ThreadScreen.back() on line 258 and ThreadOptionsScreen.openInChannelOption flow, you navigate through PermalinkScreen and end on ChannelScreen. The GlobalThreadsScreen.back() call on line 272 appears unreachable since you're not on the GlobalThreadsScreen at this point - you're on ChannelScreen (confirmed by line 266-268).

🔧 Suggested fix
         // # Go back to channel list screen
         await ChannelScreen.back();
-        await GlobalThreadsScreen.back();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/smoke_test/threads.e2e.ts` around lines 270
- 272, The GlobalThreadsScreen.back() call is unreachable after the
ThreadScreen.back() -> ThreadOptionsScreen.openInChannelOption ->
PermalinkScreen -> ChannelScreen flow; remove the dangling
GlobalThreadsScreen.back() invocation (or if the intent was to return to
GlobalThreadsScreen, instead call GlobalThreadsScreen.back() before navigating
into the channel/permalink flow). Update the sequence around
ThreadScreen.back(), ThreadOptionsScreen.openInChannelOption, PermalinkScreen,
and ChannelScreen to either delete GlobalThreadsScreen.back() or reposition it
so it is executed while the app is actually on GlobalThreadsScreen.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33a52e39-af83-49b1-b5ae-a2037cb022c0

📥 Commits

Reviewing files that changed from the base of the PR and between 1e169c7 and 1938bb1.

⛔ Files ignored due to path filters (5)
  • detox/e2e/support/fixtures/audio.mp3 is excluded by !**/*.mp3
  • detox/e2e/support/fixtures/image.png is excluded by !**/*.png
  • detox/e2e/support/fixtures/sample.pdf is excluded by !**/*.pdf
  • detox/package-lock.json is excluded by !**/package-lock.json
  • maestro/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (282)
  • .github/actions/generate-specs/split-tests.js
  • .github/actions/prepare-ios-build/action.yaml
  • .github/actions/prepare-node-deps/action.yaml
  • .github/actions/test/action.yaml
  • .github/workflows/compatibility-matrix-testing.yml
  • .github/workflows/e2e-android-template.yml
  • .github/workflows/e2e-cancel-on-label-removal.yml
  • .github/workflows/e2e-detox-pr.yml
  • .github/workflows/e2e-detox-release.yml
  • .github/workflows/e2e-detox-scheduled.yml
  • .github/workflows/e2e-ios-template.yml
  • .github/workflows/e2e-maestro-template.yml
  • .gitignore
  • android/gradle.properties
  • app/actions/local/thread.test.ts
  • app/actions/local/thread.ts
  • app/actions/remote/preference.test.ts
  • app/actions/remote/preference.ts
  • app/actions/remote/search.ts
  • app/actions/remote/thread.test.ts
  • app/actions/remote/thread.ts
  • app/actions/websocket/channel.ts
  • app/actions/websocket/preferences.test.ts
  • app/actions/websocket/preferences.ts
  • app/components/channel_bookmarks/add_bookmark.tsx
  • app/components/channel_bookmarks/bookmark_type.tsx
  • app/components/channel_bookmarks/channel_bookmark/bookmark_details.tsx
  • app/components/channel_bookmarks/channel_bookmark/channel_bookmark.tsx
  • app/components/channel_bookmarks/channel_bookmarks.tsx
  • app/components/common_post_options/save_option.tsx
  • app/components/floating_input/floating_input_container.tsx
  • app/components/floating_input/floating_text_input_label.tsx
  • app/components/keyboard_aware_post_draft_container.tsx
  • app/components/post_list/index.ts
  • app/components/post_list/post_list.tsx
  • app/components/post_list/thread_overview/index.ts
  • app/components/post_with_channel_info/index.ts
  • app/components/pressable_opacity/index.tsx
  • app/hooks/useKeyboardAwarePostDraft.ts
  • app/hooks/useKeyboardScrollAdjustment.ts
  • app/products/calls/components/current_call_bar/current_call_bar.tsx
  • app/products/calls/components/join_call_banner/join_call_banner.tsx
  • app/queries/servers/post.test.ts
  • app/queries/servers/post.ts
  • app/queries/servers/system.test.ts
  • app/queries/servers/system.ts
  • app/queries/servers/thread.test.ts
  • app/queries/servers/thread.ts
  • app/screens/bottom_sheet/index.tsx
  • app/screens/browse_channels/browse_channels.tsx
  • app/screens/browse_channels/index.ts
  • app/screens/channel_bookmark/components/bookmark_detail.tsx
  • app/screens/channel_bookmark/components/bookmark_file/bookmark_file.tsx
  • app/screens/channel_bookmark/components/bookmark_link.tsx
  • app/screens/channel_bookmark/index.tsx
  • app/screens/channel_settings/archive/index.ts
  • app/screens/gallery/footer/actions/action.tsx
  • app/screens/gallery/footer/actions/index.tsx
  • app/screens/gallery/header/index.tsx
  • app/screens/home/channel_list/categories_list/categories/categories.tsx
  • app/screens/home/channel_list/categories_list/categories/header/header.tsx
  • app/screens/home/channel_list/categories_list/categories/header/index.ts
  • app/screens/home/channel_list/categories_list/categories/helpers/observe_flattened_categories.ts
  • app/screens/home/channel_list/categories_list/header/header.tsx
  • app/screens/home/saved_messages/index.ts
  • app/screens/pinned_messages/pinned_messages.tsx
  • app/screens/post_options/index.ts
  • app/screens/post_priority_picker/post_priority_picker.test.tsx
  • app/screens/thread_options/index.ts
  • app/screens/user_profile/custom_attributes.tsx
  • app/store/ephemeral_store.ts
  • app/utils/opengraph.ts
  • detox/.detoxrc.json
  • detox/CLAUDE.md
  • detox/README.md
  • detox/android_emulator/config.ini
  • detox/create_android_emulator.sh
  • detox/detox/.detoxrc.json
  • detox/e2e/config.js
  • detox/e2e/global_setup.js
  • detox/e2e/support/fixtures/sample.txt
  • detox/e2e/support/server_api/channel_bookmark.ts
  • detox/e2e/support/server_api/client.ts
  • detox/e2e/support/server_api/command.ts
  • detox/e2e/support/server_api/common.ts
  • detox/e2e/support/server_api/custom_profile_attributes.ts
  • detox/e2e/support/server_api/index.ts
  • detox/e2e/support/server_api/plugin.ts
  • detox/e2e/support/server_api/post.ts
  • detox/e2e/support/server_api/system.ts
  • detox/e2e/support/server_api/webhook.ts
  • detox/e2e/support/test_config.ts
  • detox/e2e/support/ui/component/alert.ts
  • detox/e2e/support/ui/component/autocomplete.ts
  • detox/e2e/support/ui/screen/account.ts
  • detox/e2e/support/ui/screen/add_members.ts
  • detox/e2e/support/ui/screen/auto_responder_notification_settings.ts
  • detox/e2e/support/ui/screen/browse_channels.ts
  • detox/e2e/support/ui/screen/channel.ts
  • detox/e2e/support/ui/screen/channel_bookmark.ts
  • detox/e2e/support/ui/screen/channel_dropdown_menu.ts
  • detox/e2e/support/ui/screen/channel_info.ts
  • detox/e2e/support/ui/screen/channel_list.ts
  • detox/e2e/support/ui/screen/channel_settings.ts
  • detox/e2e/support/ui/screen/create_direct_message.ts
  • detox/e2e/support/ui/screen/create_or_edit_channel.ts
  • detox/e2e/support/ui/screen/display_settings.ts
  • detox/e2e/support/ui/screen/edit_post.ts
  • detox/e2e/support/ui/screen/edit_profile.ts
  • detox/e2e/support/ui/screen/edit_server.ts
  • detox/e2e/support/ui/screen/email_notification_settings.ts
  • detox/e2e/support/ui/screen/emoji_picker.ts
  • detox/e2e/support/ui/screen/global_threads.ts
  • detox/e2e/support/ui/screen/home.ts
  • detox/e2e/support/ui/screen/index.ts
  • detox/e2e/support/ui/screen/login.ts
  • detox/e2e/support/ui/screen/manage_channel_members.ts
  • detox/e2e/support/ui/screen/mention_notification_settings.ts
  • detox/e2e/support/ui/screen/notification_settings.ts
  • detox/e2e/support/ui/screen/permalink.ts
  • detox/e2e/support/ui/screen/pinned_messages.ts
  • detox/e2e/support/ui/screen/post_options.ts
  • detox/e2e/support/ui/screen/push_notification_settings.ts
  • detox/e2e/support/ui/screen/recent_mentions.ts
  • detox/e2e/support/ui/screen/saved_messages.ts
  • detox/e2e/support/ui/screen/search_messages.ts
  • detox/e2e/support/ui/screen/server.ts
  • detox/e2e/support/ui/screen/server_list.ts
  • detox/e2e/support/ui/screen/settings.ts
  • detox/e2e/support/ui/screen/table.ts
  • detox/e2e/support/ui/screen/team_dropdown_menu.ts
  • detox/e2e/support/ui/screen/thread.ts
  • detox/e2e/support/utils/detoxhelpers.ts
  • detox/e2e/support/utils/index.ts
  • detox/e2e/test/products/agents/channel_summary.e2e.ts
  • detox/e2e/test/products/agents/tool_calls_in_channels.e2e.ts
  • detox/e2e/test/products/channels/account/account_menu.e2e.ts
  • detox/e2e/test/products/channels/account/account_profile_picture.e2e.ts
  • detox/e2e/test/products/channels/account/advanced_settings.e2e.ts
  • detox/e2e/test/products/channels/account/auto_responder_notification_settings.e2e.ts
  • detox/e2e/test/products/channels/account/clock_display_settings.e2e.ts
  • detox/e2e/test/products/channels/account/custom_status.e2e.ts
  • detox/e2e/test/products/channels/account/edit_profile.e2e.ts
  • detox/e2e/test/products/channels/account/notification_settings.e2e.ts
  • detox/e2e/test/products/channels/account/theme_color_picker.e2e.ts
  • detox/e2e/test/products/channels/account/user_attributes.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/at_mention_name_matching.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/at_mention_user_suggestions.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/channel_mention.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/channel_post_draft.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/edit_channel.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/edit_post.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/search.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_copy_tests.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_create_edit.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_join_leave.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_members.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_navigation.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_notifications.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_settings_smoke.e2e.ts
  • detox/e2e/test/products/channels/channel_settings/channel_system_messages.e2e.ts
  • detox/e2e/test/products/channels/channels/archive_channel.e2e.ts
  • detox/e2e/test/products/channels/channels/browse_channels.e2e.ts
  • detox/e2e/test/products/channels/channels/channel_bookmarks.e2e.ts
  • detox/e2e/test/products/channels/channels/channel_bookmarks_permissions.e2e.ts
  • detox/e2e/test/products/channels/channels/channel_bookmarks_search.e2e.ts
  • detox/e2e/test/products/channels/channels/channel_info.e2e.ts
  • detox/e2e/test/products/channels/channels/channel_list.e2e.ts
  • detox/e2e/test/products/channels/channels/convert_to_private_channel.e2e.ts
  • detox/e2e/test/products/channels/channels/create_channel_and_edit_channel_header.e2e.ts
  • detox/e2e/test/products/channels/channels/create_direct_message.e2e.ts
  • detox/e2e/test/products/channels/channels/edit_channel.e2e.ts
  • detox/e2e/test/products/channels/channels/favorite_and_unfavorite_channel.e2e.ts
  • detox/e2e/test/products/channels/channels/find_channels.e2e.ts
  • detox/e2e/test/products/channels/channels/leave_channel.e2e.ts
  • detox/e2e/test/products/channels/channels/manage_own_channel_membership.e2e.ts
  • detox/e2e/test/products/channels/channels/mute_and_unmute_channel.e2e.ts
  • detox/e2e/test/products/channels/channels/unarchive_channel.e2e.ts
  • detox/e2e/test/products/channels/interactive_dialog/interactive_dialog_plugin.e2e.ts
  • detox/e2e/test/products/channels/ipad/ipad_account_view.e2e.ts
  • detox/e2e/test/products/channels/ipad/ipad_channel_navigation.e2e.ts
  • detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts
  • detox/e2e/test/products/channels/ipad/ipad_sidebar_always_visible.e2e.ts
  • detox/e2e/test/products/channels/localization/language.e2e.ts
  • detox/e2e/test/products/channels/messaging/at_mention.e2e.ts
  • detox/e2e/test/products/channels/messaging/channel_link.e2e.ts
  • detox/e2e/test/products/channels/messaging/channel_mention.e2e.ts
  • detox/e2e/test/products/channels/messaging/emoji_display.e2e.ts
  • detox/e2e/test/products/channels/messaging/emojis_and_reactions.e2e.ts
  • detox/e2e/test/products/channels/messaging/file_preview_gallery.e2e.ts
  • detox/e2e/test/products/channels/messaging/file_type_preview.e2e.ts
  • detox/e2e/test/products/channels/messaging/file_upload.e2e.ts
  • detox/e2e/test/products/channels/messaging/image_attachment_post_options.e2e.ts
  • detox/e2e/test/products/channels/messaging/large_gif_not_rendered.e2e.ts
  • detox/e2e/test/products/channels/messaging/mark_as_unread.e2e.ts
  • detox/e2e/test/products/channels/messaging/markdown_code.e2e.ts
  • detox/e2e/test/products/channels/messaging/markdown_table.e2e.ts
  • detox/e2e/test/products/channels/messaging/message_delete.e2e.ts
  • detox/e2e/test/products/channels/messaging/message_edit.e2e.ts
  • detox/e2e/test/products/channels/messaging/message_permalink.e2e.ts
  • detox/e2e/test/products/channels/messaging/message_post.e2e.ts
  • detox/e2e/test/products/channels/messaging/messaging_misc.e2e.ts
  • detox/e2e/test/products/channels/messaging/pin_and_unpin_message.e2e.ts
  • detox/e2e/test/products/channels/scheduled_messages/create_schedule_message.e2e.ts
  • detox/e2e/test/products/channels/search/cross_team_search.e2e.ts
  • detox/e2e/test/products/channels/search/hashtag_search.e2e.ts
  • detox/e2e/test/products/channels/search/pinned_messages.e2e.ts
  • detox/e2e/test/products/channels/search/recent_mentions.e2e.ts
  • detox/e2e/test/products/channels/search/search_behaviors.e2e.ts
  • detox/e2e/test/products/channels/search/search_cycle.e2e.ts
  • detox/e2e/test/products/channels/search/search_messages.e2e.ts
  • detox/e2e/test/products/channels/server_login/connect_to_server.e2e.ts
  • detox/e2e/test/products/channels/server_login/server_list.e2e.ts
  • detox/e2e/test/products/channels/smoke_test/channels.e2e.ts
  • detox/e2e/test/products/channels/smoke_test/messaging.e2e.ts
  • detox/e2e/test/products/channels/smoke_test/server_login.e2e.ts
  • detox/e2e/test/products/channels/smoke_test/threads.e2e.ts
  • detox/e2e/test/products/channels/teams/invite_people.e2e.ts
  • detox/e2e/test/products/channels/threads/follow_and_unfollow_thread.e2e.ts
  • detox/e2e/test/products/channels/threads/open_thread_in_channel.e2e.ts
  • detox/e2e/test/products/channels/threads/reply_to_thread.e2e.ts
  • detox/e2e/test/products/playbooks/playbooks-e2e.ts
  • detox/e2e/test/products/playbooks/playbooks.ts
  • detox/e2e/test/setup.ts
  • detox/generate_unified_report.js
  • detox/inject-detox-settings.js
  • detox/package.json
  • detox/provision_server.js
  • detox/scripts/postinstall.js
  • detox/scripts/run_android_gradle_build.sh
  • detox/scripts/run_detox.sh
  • detox/utils/disable_ios_autofill.js
  • detox/utils/generate_detox_config_ci.js
  • detox/utils/maestro_report.js
  • detox/utils/report.js
  • fastlane/Fastfile
  • index.ts
  • ios/MattermostShare/Views/ContentViews/ChannelListView/ChannelItemView.swift
  • ios/MattermostShare/Views/ContentViews/ContentView.swift
  • ios/MattermostShare/Views/NavigationButtons/PostButton.swift
  • maestro/AGENTS.md
  • maestro/README.md
  • maestro/fixtures/calls_seed.ts
  • maestro/fixtures/poll_for_message.ts
  • maestro/fixtures/seed.ts
  • maestro/fixtures/seed_file_preview.ts
  • maestro/flows/account/help_url.yml
  • maestro/flows/calls/call_ui_permission.yml
  • maestro/flows/calls/device_a_start_call.yml
  • maestro/flows/calls/device_b_join_call.yml
  • maestro/flows/calls/leave_call.yml
  • maestro/flows/calls/mute_unmute.yml
  • maestro/flows/calls/start_call.yml
  • maestro/flows/channels/channel_bookmark_file.yml
  • maestro/flows/channels/channel_bookmark_file_android_picker.yml
  • maestro/flows/channels/channel_bookmark_file_ios_picker.yml
  • maestro/flows/channels/file_type_preview.yml
  • maestro/flows/multi_device/user_a_sends_message.yml
  • maestro/flows/multi_device/user_b_receives_message.yml
  • maestro/flows/share_extension/share_image_to_channel.yml
  • maestro/flows/share_extension/share_link_to_channel.yml
  • maestro/flows/share_extension/share_text_to_channel.yml
  • maestro/flows/timezone/clock_display.yml
  • maestro/package.json
  • maestro/scripts/reset_timezone.js
  • maestro/scripts/run_calls_two_device.sh
  • maestro/scripts/run_timezone_test.sh
  • maestro/scripts/run_two_device.sh
  • maestro/scripts/set_timezone.js
  • maestro/tsconfig.json
  • maestro/utils/connect_server.yml
  • maestro/utils/login.yml
  • maestro/utils/logout.yml
  • maestro/utils/navigate_to_channel.yml
  • maestro/utils/setup.yml
  • patches/@gorhom+bottom-sheet+5.1.2.patch
  • patches/react-native+0.77.3.patch
  • patches/react-native-reanimated+3.17.3.patch
  • share_extension/components/channel_item/channel_item.tsx
  • share_extension/components/content_view/options/option.tsx
  • share_extension/components/content_view/options/options.tsx
  • share_extension/components/header/post_button.tsx
💤 Files with no reviewable changes (1)
  • .github/actions/prepare-node-deps/action.yaml

Comment thread detox/e2e/test/products/channels/autocomplete/search.e2e.ts Outdated
Comment thread detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts Outdated
Comment on lines +167 to +188
it('MM-T171 - should be able to autocomplete at-mention for out-of-channel member', async () => {
// # Create a user who is on the team but not in the channel
const {user: outOfChannelUser} = await User.apiCreateUser(siteOneUrl);
await Team.apiAddUserToTeam(siteOneUrl, outOfChannelUser.id, testTeam.id);

// # Open a channel screen and type "@" + first chars to activate at-mention autocomplete
// Type "@" together with the first 3 chars of the username in a single typeText
// call. Typing bare "@" alone can trigger the noResultsTerm race condition in
// at_mention.tsx: if the initial empty-matchTerm search returns 0 results before
// any users load, noResultsTerm is set to "" which suppresses all future searches
// (every string starts with ""). Typing a few chars alongside "@" ensures the
// first search has a non-empty matchTerm that returns results.
await ChannelScreen.open(channelsCategory, testChannel.name);
await ChannelScreen.postInput.tap();
await wait(timeouts.ONE_SEC);
const usernamePrefix = outOfChannelUser.username.substring(0, 3);
await ChannelScreen.postInput.typeText(`@${usernamePrefix}`);
await wait(timeouts.TWO_SEC);

// * Verify at-mention list is displayed
await waitFor(Autocomplete.sectionAtMentionList).toExist().withTimeout(timeouts.HALF_MIN);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

The new out-of-channel autocomplete flow is still flaky.

Line 187 waits for the generic list after searching with a prefix that only matches a freshly-created user. In CI that user is not always indexed locally yet, so Autocomplete.sectionAtMentionList never appears and the test times out. Seed autocomplete with a known in-channel match first, or poll for the actual out-of-channel suggestion instead of sectionAtMentionList.

🧰 Tools
🪛 GitHub Actions: E2E

[error] 187-187: Detox test failed. Test Failed: Timed out while waiting for expectation: TOEXIST WITH MATCHER(id == “autocomplete.at_mention.section_list”) TIMEOUT(30s). (at_mention list not found)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/messaging/at_mention.e2e.ts` around lines
167 - 188, The test is flaky because it waits for the generic
Autocomplete.sectionAtMentionList which may never appear if the freshly-created
outOfChannelUser isn’t indexed yet; update the check to poll for the specific
suggestion for outOfChannelUser instead of the generic list (e.g., waitFor an
item like Autocomplete.atMentionListItem(outOfChannelUser.username) or
Autocomplete.suggestionFor(outOfChannelUser.username) after typing into
ChannelScreen.postInput using usernamePrefix), and as a fallback seed the
autocomplete first with a known in-channel username (type a
knownInChannelUser.username briefly before typing `@${usernamePrefix}`) so the
component has results to display while the out-of-channel user is being indexed.

Comment thread detox/e2e/test/products/channels/messaging/large_gif_not_rendered.e2e.ts Outdated
Comment on lines +125 to +128
// # While in thread view, long press the parent post (top post), select Delete and confirm
await ThreadScreen.openPostOptionsFor(parentPost.id, message);
await PostOptionsScreen.deletePost({confirm: true});
await wait(timeouts.TWO_SEC);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

This new thread-delete flow is already red in CI.

Line 126 hits the same visibility-threshold failure as MM-T4784_3, so this adds another blocker instead of covering the regression. Please make ThreadScreen.openPostOptionsFor(...) reliably scroll to a fully hittable target in thread view before reusing it here.

🧰 Tools
🪛 GitHub Actions: E2E

[error] 126-126: Detox test failed (MM-T112): View is not hittable/visible around point. Error: View is not visible: View does not pass visibility percent threshold (100). Root call: longPressWithScrollRetry (support/utils/index.ts:196).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/messaging/message_delete.e2e.ts` around
lines 125 - 128, ThreadScreen.openPostOptionsFor currently can target a post
that's not fully hittable in thread view; update openPostOptionsFor to reliably
scroll the thread list until the target post element is completely
visible/hittable before performing the long-press (so the call in
message_delete.e2e.ts no longer flakes). Specifically, inside
ThreadScreen.openPostOptionsFor locate the target post element (the same
selector used there), perform scrolling (loop if necessary) to bring it to the
center/fully visible area and verify hittability with a visibility/hittable
check before invoking the long-press that triggers
PostOptionsScreen.deletePost({confirm:true}); ensure the method returns only
after the element is confirmed hittable.

@yasserfaraazkhan yasserfaraazkhan added the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Apr 16, 2026
@mm-cloud-bot
Copy link
Copy Markdown

❌ E2E Test Setup Failed

Failed to create E2E test instances: failed to create installation: failed with status code 409

yasserfaraazkhan and others added 2 commits April 16, 2026 15:44
E2E test fixes:
- Add missing `waitFor` import to search.e2e.ts, large_gif_not_rendered.e2e.ts,
  and ipad_post_message.e2e.ts (all used waitFor without importing it)
- Guard bot creation in agent_mention.e2e.ts with an explicit error check to
  prevent TypeError when apiCreateBot returns {error} instead of {bot}
- Add `waitFor(sendButton).toBeVisible()` guard before disabling Detox sync in
  longPressSendButton() for both channel.ts and thread.ts screens, matching the
  existing pattern in tapSendButton()
- Fix archive_channel.e2e.ts iOS tap-before-sync race: wrap Browse Channels tap
  inside sync-disabled block via new tapChannelAndWaitForArchivedChannelScreen()
  helper so the modal dismiss + channel push don't permanently busy the bridge

Calls fix:
- Extend WebSocketClientInterface locally with optional sendBinary to resolve
  pre-existing TS2339 error introduced when sendBinary was added to the native
  client after the published type definition was last updated

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…failures

Sidebar channel visibility (11 iOS failures — message_draft, channel_mention,
reply_to_thread, save_and_unsave_message, cross_team_search):
- Replace split-budget waitFor().toExist().withTimeout() in
  waitForSidebarPublicChannelDisplayNameVisible with a rolling polling probe
  (waitForElementToExist, 3 s per category). iOS 26.x never idles the bridge so
  the old approach's timer stalled; polling uses real wall-clock time and finds
  the channel in whichever category it lands (channels/unreads/favorites).
- Apply same polling change to tapSidebarPublicChannelDisplayName.

Scheduled-message long-press (5 iOS + 5 Android create_schedule_message):
- Swap waitFor(sendButton).toBeVisible().withTimeout() → waitForElementToBeVisible
  (polling) so Android API 35 bridge-busy state doesn't block the visibility guard.
- Add Android keyboard-dismiss swipe in longPressSendButton (channel.ts + thread.ts)
  BEFORE disabling sync: the soft keyboard intercepts the long-press when open.

"View is not hittable" iOS long-press (9 failures across message_delete,
message_edit, permalink, channel_link, follow_and_unfollow_thread):
- Add scroll(100, 'up') on the post list before long-pressing in openPostOptionsFor
  (channel.ts + thread.ts): the newest post sits at the bottom edge of the list,
  partially occluded by the post input bar, making its centre point unhittable.

Android "Element did not appear after 5 longPress attempts" (8 failures across
global_threads, open_thread_in_channel, save_and_unsave_thread, message_edit,
mark_as_unread, save_and_unsave_message, channel_link, pin_and_unpin_message):
- Increase longPressWithScrollRetry Android wait 2 s → 3 s and press duration
  3 s → 4 s in utils/index.ts to give API 35 emulators more time for the gesture
  responder to become available after keyboard-dismiss animations.

cross_team_search "All teams" Android assertion (1 Android failure):
- Replace waitFor(by.text()).toBeVisible() with waitForElementToExist polling on
  Android: bridge-idle sync blocks the check during sheet-close animation on API 35.

channel_info.ts iOS timeout (unarchive_channel iOS failures):
- Increase iOS toBeVisible timeout from TEN_SEC → HALF_MIN and switch to
  waitForElementToExist polling: post-unarchive navigation stack settles slowly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (7)
detox/e2e/support/utils/index.ts (1)

308-331: Redundant require('detox') calls in final checks.

Both waitForElementToNotExist and waitForElementToExist import detoxExpect at the start of the function, then import it again as detoxExpectFinal for the final check outside the loop. Since the same detoxExpect reference would work, the second import is unnecessary.

♻️ Proposed simplification
 export async function waitForElementToNotExist(
     detoxElement: Detox.NativeElement,
     timeout: number = timeouts.HALF_MIN,
     pollInterval: number = timeouts.HALF_SEC,
 ): Promise<void> {
     const {expect: detoxExpect} = require('detox');
     const startTime = Date.now();
     /* eslint-disable no-await-in-loop */
     while (Date.now() - startTime < timeout) {
         try {
             await detoxExpect(detoxElement).not.toExist();
             return; // Element no longer exists
         } catch (error) {
             if ((Date.now() - startTime) + pollInterval >= timeout) {
                 throw error;
             }
             await wait(pollInterval);
         }
     }
     /* eslint-enable no-await-in-loop */
     // Final check - will throw if still exists
-    const {expect: detoxExpectFinal} = require('detox');
-    await detoxExpectFinal(detoxElement).not.toExist();
+    await detoxExpect(detoxElement).not.toExist();
 }

Apply the same pattern to waitForElementToExist.

Also applies to: 353-376

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/utils/index.ts` around lines 308 - 331, The final explicit
require('detox') call is redundant: in waitForElementToNotExist (and similarly
in waitForElementToExist) you already destructured detoxExpect at the top;
remove the second require and reuse the initial detoxExpect instead of
detoxExpectFinal for the final check and any other post-loop assertions to avoid
duplicate requires and keep behavior identical.
detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts (2)

136-140: Redundant assertion after waitFor.

At line 139, await expect(postListPostItem).toExist() immediately follows await waitFor(postListPostItem).toExist().withTimeout(...) at line 138. If the waitFor succeeds, the expect is guaranteed to pass and adds no value.

♻️ Remove redundant assertion
         const {post} = await Post.apiGetLastPostInChannel(siteOneUrl, testChannel.id);
         const {postListPostItem} = ChannelScreen.getPostListPostItem(post.id, message);
         await waitFor(postListPostItem).toExist().withTimeout(timeouts.TEN_SEC);
-        await expect(postListPostItem).toExist();
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts` around lines
136 - 140, The second explicit assertion is redundant: after awaiting
waitFor(postListPostItem).toExist().withTimeout(...), the element existence is
already verified, so remove the duplicate await
expect(postListPostItem).toExist(); Delete the expect line that references
postListPostItem in the test using ChannelScreen.getPostListPostItem and rely on
the waitFor(...) call to assert existence.

61-82: Test doesn't navigate back to channel list at end.

Per coding guidelines, each test should navigate back to channel list at the end for clean state for next test. While beforeEach relaunches the app on iPad which provides isolation, for consistency and to align with guidelines, consider adding navigation back to channel list.

This applies to all five tests in this file (MM-TIPAD_15 through MM-TIPAD_19).

♻️ Proposed fix for MM-TIPAD_15
         // * Verify the sidebar is still visible after posting (tablet split view).
         // On iPad the software keyboard (UITextEffectsWindow) may cover the lower portion of
         // the channel_list.screen SafeAreaView, dropping its visible percentage below the 75%
         // threshold required by toBeVisible().  Use toExist() to confirm the view is in the
         // hierarchy regardless of transient keyboard obstruction.
         await waitFor(ChannelListScreen.channelListScreen).toExist().withTimeout(timeouts.TEN_SEC);
+
+        // # Navigate back to channel list for clean state
+        await ChannelScreen.back();
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts` around lines
61 - 82, The test MM-TIPAD_15 does not navigate back to the channel list at the
end; update the test to return the app to the channel list after the final
assertion by invoking the navigation helper (e.g., call
ChannelScreen.backToChannelList() — or the existing helper that performs the
same action) immediately after the
waitFor(ChannelListScreen.channelListScreen)...withTimeout(...) line so the test
leaves the app in the channel-list state for subsequent tests; apply the same
change to MM-TIPAD_16 through MM-TIPAD_19.
detox/e2e/test/products/channels/channels/archive_channel.e2e.ts (2)

105-130: Android fallback in closeBrowseChannelsChannel may cause unexpected navigation.

In the Android branch (lines 112-121), if the close button isn't found, the code falls back to device.pressBack(). However, if Browse Channels was already dismissed, pressBack() might navigate further back than intended (e.g., to a previous screen in the stack).

Consider whether the catch block should simply do nothing (like the iOS branch) rather than calling pressBack().

♻️ Proposed fix for safer fallback
     if (isAndroid()) {
         // On Android the modal stack can collapse differently — use pressBack
         // which reliably dismisses the current screen regardless of stack depth.
         try {
             await waitFor(BrowseChannelsScreen.closeButton).toExist().withTimeout(timeouts.FOUR_SEC);
             await BrowseChannelsScreen.closeButton.tap();
         } catch {
             // Browse Channels was already dismissed when Channel.back() popped the stack
-            await device.pressBack();
+            // No action needed — navigation already complete
         }
     } else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/channels/archive_channel.e2e.ts` around
lines 105 - 130, The Android fallback in closeBrowseChannelsChannel currently
calls device.pressBack() inside the catch when BrowseChannelsScreen.closeButton
isn't found, which can over-navigate; change the catch to mirror the iOS branch
by doing nothing (no pressBack) or alternatively perform a safe existence check
before calling device.pressBack(); update the catch block that references
BrowseChannelsScreen.closeButton and device.pressBack so that if the close
button is missing after ChannelScreen.back()/switchToChannelById, the function
simply returns without invoking device.pressBack().

666-733: Search test properly handles synchronization around tapReturnKey.

The test at MM-T1679_1 correctly disables synchronization before tapReturnKey() and re-enables it after waiting for results. However, if waitForElementToBeVisible throws, synchronization would be left disabled.

♻️ Proposed fix for synchronization safety
         // * Verify the search result contains the message from the archived channel
         // Disable synchronization before tapReturnKey: the search keeps the dispatch
         // queue busy while processing network/DB work, which would otherwise cause
         // tapReturnKey to block indefinitely waiting for Detox idle.
         await device.disableSynchronization();
-        await SearchMessagesScreen.searchInput.tapReturnKey();
-
-        // Wait for the search result text to appear (text element, not the composed matcher
-        // which uses withDescendant and can be unreliable when text is highlighted/split).
-        const searchResultText = element(
-            by.
-                text(uniqueMessage).
-                withAncestor(by.id(SearchMessagesScreen.postList.testID.flatList)),
-        );
-        await waitForElementToBeVisible(searchResultText, timeouts.ONE_MIN);
-        await device.enableSynchronization();
+        try {
+            await SearchMessagesScreen.searchInput.tapReturnKey();
+
+            // Wait for the search result text to appear (text element, not the composed matcher
+            // which uses withDescendant and can be unreliable when text is highlighted/split).
+            const searchResultText = element(
+                by.
+                    text(uniqueMessage).
+                    withAncestor(by.id(SearchMessagesScreen.postList.testID.flatList)),
+            );
+            await waitForElementToBeVisible(searchResultText, timeouts.ONE_MIN);
+        } finally {
+            await device.enableSynchronization();
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/channels/archive_channel.e2e.ts` around
lines 666 - 733, The test disables Detox synchronization before calling
SearchMessagesScreen.searchInput.tapReturnKey() but re-enables it only after
waitForElementToBeVisible, so if that wait throws the test leaves
synchronization disabled; wrap the disable/enable sequence in a try/finally
(call device.disableSynchronization() then try { tapReturnKey(); await
waitForElementToBeVisible(searchResultText, ...) } finally {
device.enableSynchronization() }) to guarantee device.enableSynchronization()
always runs; update the MM-T1679_1 test around
SearchMessagesScreen.searchInput.tapReturnKey / waitForElementToBeVisible to use
this pattern.
detox/e2e/support/ui/screen/thread.ts (1)

91-107: Tooltip dismissal could verify dismissal completed.

The dismissScheduledPostTooltip method taps the close button but doesn't verify the tooltip actually disappeared. The relevant code snippet from account.ts (lines 115-125) shows a more robust pattern that waits for the tooltip to not exist after tapping.

This is a minor concern since the wait(HALF_SEC) provides some buffer, but for reliability on slower CI environments, consider adding the verification.

♻️ Proposed improvement for robustness
     dismissScheduledPostTooltip = async () => {
         // Try to close scheduled post tooltip if it exists (try both regular and admin account versions)
         try {
             await waitFor(this.scheduledPostTooltipCloseButton).toBeVisible().withTimeout(timeouts.FOUR_SEC);
             await this.scheduledPostTooltipCloseButton.tap();
-            await wait(timeouts.HALF_SEC);
+            await waitFor(this.scheduledPostTooltipCloseButton).not.toExist().withTimeout(timeouts.FIVE_SEC);
         } catch {
             // Try admin account version
             try {
                 await waitFor(this.scheduledPostTooltipCloseButtonAdminAccount).toBeVisible().withTimeout(timeouts.FOUR_SEC);
                 await this.scheduledPostTooltipCloseButtonAdminAccount.tap();
-                await wait(timeouts.HALF_SEC);
+                await waitFor(this.scheduledPostTooltipCloseButtonAdminAccount).not.toExist().withTimeout(timeouts.FIVE_SEC);
             } catch {
                 // Tooltip not visible, continue
             }
         }
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/thread.ts` around lines 91 - 107, The
dismissScheduledPostTooltip function currently taps the close buttons
(scheduledPostTooltipCloseButton and
scheduledPostTooltipCloseButtonAdminAccount) but only waits a fixed HALF_SEC;
update both tap paths in dismissScheduledPostTooltip to explicitly verify the
tooltip is gone after tapping by waiting for the corresponding tooltip element
to be not visible/not exist (use the same waitFor(...).toBeNotVisible() or
.toNotExist() pattern used in account.ts) with an appropriate timeout instead of
relying on wait(timeouts.HALF_SEC), so each branch confirms dismissal completed.
detox/e2e/support/ui/screen/channel.ts (1)

420-446: Comment about locale-dependent behavior is incorrect.

The comment at line 442 mentions moment.weekday() being locale-dependent, but the code uses new Date().getDay() which returns 0-6 (Sunday-Saturday) regardless of locale. The comment should be updated to reflect the actual implementation.

Additionally, the logic for Sunday (day 0) falls through to scheduleMessageForMonday() per the comment at lines 441-444, but the docstring at line 424 states Sunday should use "Tomorrow". Consider clarifying which behavior is intended.

📝 Proposed comment fix
     scheduleMessageForAvailableOption = async () => {
         const day = new Date().getDay(); // 0 = Sunday … 6 = Saturday
         if (day === 5 || day === 6) {
             // Friday or Saturday: only "Monday" is available
             await this.scheduleMessageForMonday();
         } else if (day === 1) {
             // Monday: "Tomorrow" and "Next Monday" are available; pick "Next Monday"
             await this.scheduleMessageForNextMonday();
         } else {
-            // Sunday, Tuesday–Thursday: "Tomorrow" is available but may not match on
-            // all CI server locales (moment.weekday() is locale-dependent). Use "Monday"
-            // which is always present as a stable fallback on weekdays.
+            // Sunday, Tuesday–Thursday: "Tomorrow" is available. Use "Monday"
+            // as a stable fallback since it's reliably present on these days.
             await this.scheduleMessageForMonday();
         }
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/channel.ts` around lines 420 - 446, The docstring
and inline comment for scheduleMessageForAvailableOption are inconsistent and
incorrect about locale dependence: replace the mention of moment.weekday() with
an accurate note that new Date().getDay() returns 0–6 (Sunday–Saturday)
regardless of locale, and then reconcile the Sunday behavior—either change the
code to call scheduleMessageForTomorrow when day === 0 (if you intend Sunday to
pick "Tomorrow") or update the docstring's schedule table to state that Sunday
falls through to scheduleMessageForMonday; adjust the comment block and any
mention near scheduleMessageForMonday and scheduleMessageForNextMonday
accordingly so comments match the actual logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@detox/e2e/support/ui/screen/channel_list.ts`:
- Around line 138-190: The recovery relaunch in ChannelListScreen.toBeVisible
uses the wrong launch arg key/value (detoxDisableSynchronization: 'YES') so
synchronization isn't actually disabled; update the device.launchApp call in
toBeVisible to pass launchArgs with the correct key detoxEnableSynchronization
and set it to 0 (or "NO") to disable Detox synchronization during the recovery
relaunch.

---

Nitpick comments:
In `@detox/e2e/support/ui/screen/channel.ts`:
- Around line 420-446: The docstring and inline comment for
scheduleMessageForAvailableOption are inconsistent and incorrect about locale
dependence: replace the mention of moment.weekday() with an accurate note that
new Date().getDay() returns 0–6 (Sunday–Saturday) regardless of locale, and then
reconcile the Sunday behavior—either change the code to call
scheduleMessageForTomorrow when day === 0 (if you intend Sunday to pick
"Tomorrow") or update the docstring's schedule table to state that Sunday falls
through to scheduleMessageForMonday; adjust the comment block and any mention
near scheduleMessageForMonday and scheduleMessageForNextMonday accordingly so
comments match the actual logic.

In `@detox/e2e/support/ui/screen/thread.ts`:
- Around line 91-107: The dismissScheduledPostTooltip function currently taps
the close buttons (scheduledPostTooltipCloseButton and
scheduledPostTooltipCloseButtonAdminAccount) but only waits a fixed HALF_SEC;
update both tap paths in dismissScheduledPostTooltip to explicitly verify the
tooltip is gone after tapping by waiting for the corresponding tooltip element
to be not visible/not exist (use the same waitFor(...).toBeNotVisible() or
.toNotExist() pattern used in account.ts) with an appropriate timeout instead of
relying on wait(timeouts.HALF_SEC), so each branch confirms dismissal completed.

In `@detox/e2e/support/utils/index.ts`:
- Around line 308-331: The final explicit require('detox') call is redundant: in
waitForElementToNotExist (and similarly in waitForElementToExist) you already
destructured detoxExpect at the top; remove the second require and reuse the
initial detoxExpect instead of detoxExpectFinal for the final check and any
other post-loop assertions to avoid duplicate requires and keep behavior
identical.

In `@detox/e2e/test/products/channels/channels/archive_channel.e2e.ts`:
- Around line 105-130: The Android fallback in closeBrowseChannelsChannel
currently calls device.pressBack() inside the catch when
BrowseChannelsScreen.closeButton isn't found, which can over-navigate; change
the catch to mirror the iOS branch by doing nothing (no pressBack) or
alternatively perform a safe existence check before calling device.pressBack();
update the catch block that references BrowseChannelsScreen.closeButton and
device.pressBack so that if the close button is missing after
ChannelScreen.back()/switchToChannelById, the function simply returns without
invoking device.pressBack().
- Around line 666-733: The test disables Detox synchronization before calling
SearchMessagesScreen.searchInput.tapReturnKey() but re-enables it only after
waitForElementToBeVisible, so if that wait throws the test leaves
synchronization disabled; wrap the disable/enable sequence in a try/finally
(call device.disableSynchronization() then try { tapReturnKey(); await
waitForElementToBeVisible(searchResultText, ...) } finally {
device.enableSynchronization() }) to guarantee device.enableSynchronization()
always runs; update the MM-T1679_1 test around
SearchMessagesScreen.searchInput.tapReturnKey / waitForElementToBeVisible to use
this pattern.

In `@detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts`:
- Around line 136-140: The second explicit assertion is redundant: after
awaiting waitFor(postListPostItem).toExist().withTimeout(...), the element
existence is already verified, so remove the duplicate await
expect(postListPostItem).toExist(); Delete the expect line that references
postListPostItem in the test using ChannelScreen.getPostListPostItem and rely on
the waitFor(...) call to assert existence.
- Around line 61-82: The test MM-TIPAD_15 does not navigate back to the channel
list at the end; update the test to return the app to the channel list after the
final assertion by invoking the navigation helper (e.g., call
ChannelScreen.backToChannelList() — or the existing helper that performs the
same action) immediately after the
waitFor(ChannelListScreen.channelListScreen)...withTimeout(...) line so the test
leaves the app in the channel-list state for subsequent tests; apply the same
change to MM-TIPAD_16 through MM-TIPAD_19.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7989c8d8-1776-4c81-879a-3ff4dbc0a565

📥 Commits

Reviewing files that changed from the base of the PR and between 1938bb1 and 491abc1.

📒 Files selected for processing (12)
  • app/products/calls/connection/websocket_client.ts
  • detox/e2e/support/ui/screen/channel.ts
  • detox/e2e/support/ui/screen/channel_info.ts
  • detox/e2e/support/ui/screen/channel_list.ts
  • detox/e2e/support/ui/screen/thread.ts
  • detox/e2e/support/utils/index.ts
  • detox/e2e/test/products/agents/agent_mention.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/search.e2e.ts
  • detox/e2e/test/products/channels/channels/archive_channel.e2e.ts
  • detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts
  • detox/e2e/test/products/channels/messaging/large_gif_not_rendered.e2e.ts
  • detox/e2e/test/products/channels/search/cross_team_search.e2e.ts
✅ Files skipped from review due to trivial changes (3)
  • app/products/calls/connection/websocket_client.ts
  • detox/e2e/test/products/channels/messaging/large_gif_not_rendered.e2e.ts
  • detox/e2e/test/products/channels/autocomplete/search.e2e.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • detox/e2e/support/ui/screen/channel_info.ts
  • detox/e2e/test/products/channels/search/cross_team_search.e2e.ts

Comment thread detox/e2e/support/ui/screen/channel_list.ts
- Fix incorrect Detox launch arg: detoxDisableSynchronization:'YES' →
  detoxEnableSynchronization:0 (critical — sync was never actually disabled)
- Remove redundant detoxExpectFinal require() in waitForElementToNotExist
  and waitForElementToExist; reuse the top-level detoxExpect reference
- Wrap disableSynchronization/enableSynchronization in try/finally in
  archive_channel MM-T1679_1 so sync is always re-enabled on failure
- Remove unsafe device.pressBack() fallback in closeBrowseChannelsChannel
  Android catch block to avoid over-navigating the stack
- Replace fixed wait(HALF_SEC) with waitFor().not.toExist() in
  ThreadScreen.dismissScheduledPostTooltip to confirm tooltip dismissal
- Fix stale comment referencing moment.weekday() in
  scheduleMessageForAvailableOption (code uses Date.getDay())
- Remove redundant expect().toExist() assertion following waitFor in
  ipad_post_message MM-TIPAD_18
- Add ChannelScreen.back() at the end of all five iPad tests for clean
  channel-list state per coding guidelines

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
detox/e2e/test/products/channels/channels/archive_channel.e2e.ts (1)

99-129: Duplicated logic between iOS and Android branches.

The closeBrowseChannelsChannel helper has nearly identical code in both the if (isAndroid()) and else branches. Consider consolidating:

💡 Suggested simplification
 async function closeBrowseChannelsChannel() {
     await ChannelScreen.back();
     await wait(timeouts.ONE_SEC);

-    // After the app awaits switchToChannelById, the Browse Channels modal may already
-    // be dismissed (dismissAllModalsAndPopToScreen closes it during navigation). Check
-    // whether Browse Channels is still on screen before trying to close it.
-    if (isAndroid()) {
-        // On Android the modal stack can collapse differently — tap close button if present.
-        try {
-            await waitFor(BrowseChannelsScreen.closeButton).toExist().withTimeout(timeouts.FOUR_SEC);
-            await BrowseChannelsScreen.closeButton.tap();
-        } catch {
-            // Browse Channels was already dismissed when Channel.back() popped the stack
-            // No action needed — navigation already complete
-        }
-    } else {
-        try {
-            await waitFor(BrowseChannelsScreen.closeButton).toExist().withTimeout(timeouts.FOUR_SEC);
-            await BrowseChannelsScreen.closeButton.tap();
-        } catch {
-            // Browse Channels was already dismissed by switchToChannelById navigation
-        }
-    }
+    // After the app awaits switchToChannelById, the Browse Channels modal may already
+    // be dismissed (dismissAllModalsAndPopToScreen closes it during navigation). Check
+    // whether Browse Channels is still on screen before trying to close it.
+    try {
+        await waitFor(BrowseChannelsScreen.closeButton).toExist().withTimeout(timeouts.FOUR_SEC);
+        await BrowseChannelsScreen.closeButton.tap();
+    } catch {
+        // Browse Channels was already dismissed — navigation already complete
+    }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/test/products/channels/channels/archive_channel.e2e.ts` around
lines 99 - 129, The helper closeBrowseChannelsChannel contains duplicated
try/catch logic in both the if (isAndroid()) and else branches; consolidate into
a single code path that calls ChannelScreen.back(), waits, then runs one try {
await
waitFor(BrowseChannelsScreen.closeButton).toExist().withTimeout(timeouts.FOUR_SEC);
await BrowseChannelsScreen.closeButton.tap(); } catch { /* swallow: Browse
Channels already dismissed */ } so you remove the duplicated branches but keep
the pre-check wait(timeouts.ONE_SEC) and any Android-specific comment if you
want to document modal-stack differences for future readers.
detox/e2e/support/utils/index.ts (1)

308-330: Consider adding diagnostic information to the timeout error.

When waitForElementToNotExist times out, the thrown error comes from the underlying detoxExpect which may not include context about which element was being waited for. This makes debugging flaky tests harder.

💡 Suggested improvement
 export async function waitForElementToNotExist(
     detoxElement: Detox.NativeElement,
     timeout: number = timeouts.HALF_MIN,
     pollInterval: number = timeouts.HALF_SEC,
 ): Promise<void> {
     const {expect: detoxExpect} = require('detox');
     const startTime = Date.now();
     /* eslint-disable no-await-in-loop */
     while (Date.now() - startTime < timeout) {
         try {
             await detoxExpect(detoxElement).not.toExist();
             return; // Element no longer exists
         } catch (error) {
             if ((Date.now() - startTime) + pollInterval >= timeout) {
-                throw error;
+                throw new Error(`Element still exists after ${timeout}ms timeout`);
             }
             await wait(pollInterval);
         }
     }
     /* eslint-enable no-await-in-loop */
     // Final check - will throw if still exists
     await detoxExpect(detoxElement).not.toExist();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/utils/index.ts` around lines 308 - 330, The timeout error
from waitForElementToNotExist lacks contextual diagnostics; update the function
(referencing waitForElementToNotExist, detoxElement, detoxExpect, timeout and
pollInterval) to catch the underlying errors both inside the loop and on the
final check and rethrow a new Error that includes identifying context (e.g.,
stringified detoxElement or a brief selector/description and the timeout value)
plus the original error message/stack (attach as cause or include text) so test
logs show which element and timeout caused the failure.
detox/e2e/support/ui/screen/channel.ts (1)

420-446: Day-of-week logic comment is slightly inconsistent with implementation.

The comment states:

  • "Sunday (0): Tomorrow"
  • "Tue–Thu (2–4): Tomorrow, Monday"

But the implementation (lines 441-444) uses scheduleMessageForMonday() for Sunday and Tuesday–Thursday, not "Tomorrow". While this works (Monday is available on those days), the comment implies "Tomorrow" would be selected.

Consider aligning the comment with the actual implementation or using scheduleMessageForTomorrow() where the comment says "Tomorrow" is the option.

💡 Suggested comment fix
     /**
      * Picks the first available schedule option regardless of the current day.
      *
      * The picker shows different options depending on the day of the week:
      *   Sunday  (0): Tomorrow
      *   Monday  (1): Tomorrow, Next Monday
      *   Tue–Thu (2–4): Tomorrow, Monday
      *   Friday  (5): Monday
      *   Saturday(6): Monday
      *
-     * This method always selects a valid option so tests pass on any day.
+     * This method selects "Monday" (or "Next Monday" on Monday) for stability,
+     * since "Monday" is available on all days except Sunday where we also use it.
      */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/channel.ts` around lines 420 - 446, The comment
describing day-of-week behavior for scheduleMessageForAvailableOption is out of
sync with the implementation: the code calls scheduleMessageForMonday() for
Sunday and Tue–Thu but the comment says "Tomorrow" is selected for those days;
either update the comment to state that Monday is chosen as a stable fallback on
Sunday and Tue–Thu, or change the logic to call scheduleMessageForTomorrow() for
the days where the comment expects "Tomorrow" (keep
scheduleMessageForNextMonday() for Monday and scheduleMessageForMonday() for
Fri/Sat). Ensure references to scheduleMessageForAvailableOption,
scheduleMessageForMonday, scheduleMessageForNextMonday, and
scheduleMessageForTomorrow are used so reviewers can locate the changes.
detox/e2e/support/ui/screen/channel_list.ts (1)

105-124: Category order may cause incorrect tap when channel appears in multiple categories.

If a channel appears in both 'unreads' and 'channels' (e.g., after receiving a new message), this method will always tap the 'channels' entry first since that's the order of iteration. This is likely acceptable for test scenarios but worth documenting.

The error message is clear about the failure mode but doesn't indicate which categories were searched, which could help debug flaky tests.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@detox/e2e/support/ui/screen/channel_list.ts` around lines 105 - 124, The
tapSidebarPublicChannelDisplayName function may tap the wrong category due to
fixed iteration order and the thrown Error lacks context; update the error
thrown at the end of tapSidebarPublicChannelDisplayName to include the
categories that were searched (use the local categories constant and
channelName) so failures show which lists were probed (reference
tapSidebarPublicChannelDisplayName and getChannelItemDisplayName and the
categories array), e.g. build a message like "Sidebar channel item not found for
channel: X; searched categories: [..]" and throw that.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@detox/e2e/support/ui/screen/channel_list.ts`:
- Around line 105-124: The tapSidebarPublicChannelDisplayName function may tap
the wrong category due to fixed iteration order and the thrown Error lacks
context; update the error thrown at the end of
tapSidebarPublicChannelDisplayName to include the categories that were searched
(use the local categories constant and channelName) so failures show which lists
were probed (reference tapSidebarPublicChannelDisplayName and
getChannelItemDisplayName and the categories array), e.g. build a message like
"Sidebar channel item not found for channel: X; searched categories: [..]" and
throw that.

In `@detox/e2e/support/ui/screen/channel.ts`:
- Around line 420-446: The comment describing day-of-week behavior for
scheduleMessageForAvailableOption is out of sync with the implementation: the
code calls scheduleMessageForMonday() for Sunday and Tue–Thu but the comment
says "Tomorrow" is selected for those days; either update the comment to state
that Monday is chosen as a stable fallback on Sunday and Tue–Thu, or change the
logic to call scheduleMessageForTomorrow() for the days where the comment
expects "Tomorrow" (keep scheduleMessageForNextMonday() for Monday and
scheduleMessageForMonday() for Fri/Sat). Ensure references to
scheduleMessageForAvailableOption, scheduleMessageForMonday,
scheduleMessageForNextMonday, and scheduleMessageForTomorrow are used so
reviewers can locate the changes.

In `@detox/e2e/support/utils/index.ts`:
- Around line 308-330: The timeout error from waitForElementToNotExist lacks
contextual diagnostics; update the function (referencing
waitForElementToNotExist, detoxElement, detoxExpect, timeout and pollInterval)
to catch the underlying errors both inside the loop and on the final check and
rethrow a new Error that includes identifying context (e.g., stringified
detoxElement or a brief selector/description and the timeout value) plus the
original error message/stack (attach as cause or include text) so test logs show
which element and timeout caused the failure.

In `@detox/e2e/test/products/channels/channels/archive_channel.e2e.ts`:
- Around line 99-129: The helper closeBrowseChannelsChannel contains duplicated
try/catch logic in both the if (isAndroid()) and else branches; consolidate into
a single code path that calls ChannelScreen.back(), waits, then runs one try {
await
waitFor(BrowseChannelsScreen.closeButton).toExist().withTimeout(timeouts.FOUR_SEC);
await BrowseChannelsScreen.closeButton.tap(); } catch { /* swallow: Browse
Channels already dismissed */ } so you remove the duplicated branches but keep
the pre-check wait(timeouts.ONE_SEC) and any Android-specific comment if you
want to document modal-stack differences for future readers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2cfeaf68-5a65-40c8-b76c-e70098263b14

📥 Commits

Reviewing files that changed from the base of the PR and between 491abc1 and 8d1573d.

📒 Files selected for processing (6)
  • detox/e2e/support/ui/screen/channel.ts
  • detox/e2e/support/ui/screen/channel_list.ts
  • detox/e2e/support/ui/screen/thread.ts
  • detox/e2e/support/utils/index.ts
  • detox/e2e/test/products/channels/channels/archive_channel.e2e.ts
  • detox/e2e/test/products/channels/ipad/ipad_post_message.e2e.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • detox/e2e/support/ui/screen/thread.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

E2E/Run Triggers E2E tests on both iOS and Android via Matterwick release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants