chore: separate keyboard e2e testing flow on CI#7300
Conversation
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
WalkthroughThis PR refactors Android E2E testing to conditionally manage keyboard navigation through workflow parameters and environment flags, updating CI orchestration, emulator configuration, test flows, and application-level keyboard suppression logic. It also updates AVD display settings for consistent rendering. ChangesKeyboard Navigation E2E Testing
AVD Display Configuration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/views/RoomView/index.tsx (1)
266-276:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInverted env-var check disables keyboard navigation in the wrong build.
The env var
DISABLE_KEYBOARD_NAVIGATION_ENABLEDis set frominputs.DISABLE_KEYBOARD_NAVIGATION(see.github/workflows/e2e-build-android.ymlline 96). Perbuild-pr.yml, the default APK is built withDISABLE_KEYBOARD_NAVIGATION: true(env='true') and the keyboard-nav APK withDISABLE_KEYBOARD_NAVIGATION: false(env='false').So intent should be:
- env=
'true'(keyboard nav disabled) → skip autofocus.- env=
'false'(keyboard nav enabled) → run theisExternalKeyboardConnected()path.Current code does the opposite: it returns when env is
'false', meaning the keyboard‑nav build can never focus the composer, while the default build keeps trying to focus on a detected external keyboard.🐛 Proposed fix
- if (__DEV__ || process.env.DISABLE_KEYBOARD_NAVIGATION_ENABLED === 'false') { + if (__DEV__ || process.env.DISABLE_KEYBOARD_NAVIGATION_ENABLED === 'true') { return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/RoomView/index.tsx` around lines 266 - 276, The environment-var check is inverted so keyboard-nav builds are skipping autofocus; update the conditional in the autofocus block to skip when in dev OR when process.env.DISABLE_KEYBOARD_NAVIGATION_ENABLED === 'true' (i.e. keyboard navigation disabled), and only run isExternalKeyboardConnected() and call this.messageComposerRef.current?.focus() when the env var is not 'true'; modify the conditional that currently checks process.env.DISABLE_KEYBOARD_NAVIGATION_ENABLED === 'false' accordingly so the logic is reversed..github/workflows/build-pr.yml (1)
76-114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winType mismatch: passing booleans to a
string‑typed reusable workflow input (lines 77 and 100).
./.github/workflows/e2e-build-android.ymldeclaresDISABLE_KEYBOARD_NAVIGATIONastype: string(line 6-8), buttrue/falsehere are YAML booleans. actionlint flags both. The same value is passed tomaestro-android.yml, which declares it astype: boolean— so the two reusable workflows already disagree on the type.Recommend standardizing on
booleanend‑to‑end: change the input type ine2e-build-android.ymltoboolean, switch itsif: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'true' / 'false' }}checks to direct boolean tests, and likewise updatemaestro-android.yml's comparisons. The values here can then stay as baretrue/false.If you'd rather keep the strings, quote the values:
🛡️ Quick alternative (string everywhere)
e2e-build-android: ... with: - DISABLE_KEYBOARD_NAVIGATION: true + DISABLE_KEYBOARD_NAVIGATION: 'true' @@ e2e-build-android-keyboard-navigation: ... with: - DISABLE_KEYBOARD_NAVIGATION: false + DISABLE_KEYBOARD_NAVIGATION: 'false'…and change
maestro-android.yml’s input back totype: stringfor consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-pr.yml around lines 76 - 114, The DISABLE_KEYBOARD_NAVIGATION input is passed as YAML booleans but e2e-build-android.yml declares it as type: string and maestro-android.yml mismatches; standardize on boolean: change the input declaration DISABLE_KEYBOARD_NAVIGATION in e2e-build-android.yml to type: boolean, update any conditionals in e2e-build-android.yml that compare inputs to 'true'/'false' to use direct boolean tests (e.g., if: inputs.DISABLE_KEYBOARD_NAVIGATION), and make matching boolean comparisons in maestro-android.yml so both reusable workflows agree; then keep the callers' with: DISABLE_KEYBOARD_NAVIGATION: true/false as bare booleans..github/workflows/maestro-android.yml (1)
81-171:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInverted
if:conditions — default tests will run on the keyboard‑enabled AVD and keyboard‑nav tests on the keyboardless AVD.Per
build-pr.ymlande2e-build-android.yml:
DISABLE_KEYBOARD_NAVIGATION='true'→ default APK (Android Experimental APK), should run on the keyboardless AVD (profile: pixel_7_pro+enable-hw-keyboard: false).DISABLE_KEYBOARD_NAVIGATION='false'→Android Experimental APK with Keyboard Navigation, should run on thePixel_API_34_KeyboardAVD (hw.keyboard=yes).The download steps at lines 46 and 52 follow this. But the AVD creation at line 82, the keyboard-AVD run at line 137, and the default-emulator run at line 155 are flipped. Combined with the inverted env check in
app/views/RoomView/index.tsxline 268, default tests end up booting a hardware-keyboard AVD with autofocus enabled (composer keeps grabbing focus → unrelated test failures) while shard 14’s actual keyboard-nav assertions run on a no-keyboard AVD with autofocus suppressed (so nothing is being exercised).🐛 Proposed fix
- name: Create Keyboard-enabled AVD - if: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'true' }} + if: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'false' }} run: | @@ - name: Start Android Emulator and Run Maestro Tests (keyboard AVD) - if: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'true' }} + if: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'false' }} uses: reactivecircus/android-emulator-runner@e89f39f1abbbd05b1113a29cf4db69e7540cae5a # v2.37.0 @@ - name: Start Android Emulator and Run Maestro Tests (default) - if: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'false' }} + if: ${{ inputs.DISABLE_KEYBOARD_NAVIGATION == 'true' }} uses: reactivecircus/android-emulator-runner@e89f39f1abbbd05b1113a29cf4db69e7540cae5a # v2.37.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/maestro-android.yml around lines 81 - 171, The AVD creation and emulator-run steps have their DISABLE_KEYBOARD_NAVIGATION checks inverted: the "Create Keyboard-enabled AVD" and "Start Android Emulator and Run Maestro Tests (keyboard AVD)" blocks currently run when inputs.DISABLE_KEYBOARD_NAVIGATION == 'true' but should run when it's 'false', and the "Start Android Emulator and Run Maestro Tests (default)" block should run when it's 'true'; update those three if: conditions (or swap the boolean values) so Pixel_API_34_Keyboard (AVD_NAME) and the keyboard AVD run when DISABLE_KEYBOARD_NAVIGATION == 'false', while the default pixel_7_pro profile with enable-hw-keyboard: false runs when DISABLE_KEYBOARD_NAVIGATION == 'true'..github/workflows/e2e-build-android.yml (1)
44-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBuild cache key doesn't include
DISABLE_KEYBOARD_NAVIGATION— parallel builds can collide and share wrong APK variants.Both
e2e-build-android(DISABLE_KEYBOARD_NAVIGATION=true) ande2e-build-android-keyboard-navigation(DISABLE_KEYBOARD_NAVIGATION=false) run in parallel and share the same cache key. Since the env var is inlined into the JS bundle at build time via the Babel pluginmodule:react-native-dotenv, both builds produce different artifacts. Whichever finishes second can pull the first's cachedandroid/app/buildand ship the wrong embedded value.🛡️ Proposed fix
- name: Cache Android build uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: path: | android/.gradle android/app/build android/app/.cxx - key: android-build-${{ runner.os }}-${{ hashFiles('package.json', '**/*.gradle*', 'android/gradle.properties', 'android/settings.gradle') }} + key: android-build-${{ runner.os }}-kbdnav-${{ inputs.DISABLE_KEYBOARD_NAVIGATION }}-${{ hashFiles('package.json', '**/*.gradle*', 'android/gradle.properties', 'android/settings.gradle') }} restore-keys: | - android-build-${{ runner.os }}- + android-build-${{ runner.os }}-kbdnav-${{ inputs.DISABLE_KEYBOARD_NAVIGATION }}-🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e-build-android.yml around lines 44 - 53, The cache key for the "Cache Android build" step currently omits the DISABLE_KEYBOARD_NAVIGATION environment variable, causing parallel runs with different values to collide; update the key (and restore-keys) used in the actions/cache step (the line starting with key: android-build-${{ runner.os }}-${{ hashFiles(... ) }}) to incorporate the DISABLE_KEYBOARD_NAVIGATION value (e.g. append -${{ env.DISABLE_KEYBOARD_NAVIGATION }} or similar) so that builds with DISABLE_KEYBOARD_NAVIGATION=true and =false produce distinct cache keys and cannot pull each other's android/app/build artifacts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/maestro-android.yml:
- Around line 9-11: DISABLE_KEYBOARD_NAVIGATION is declared as boolean but code
compares inputs.DISABLE_KEYBOARD_NAVIGATION to string literals, causing type
mismatch and fragile checks; change the input declaration to boolean in both
workflow files and replace all string comparisons of
inputs.DISABLE_KEYBOARD_NAVIGATION (e.g., inputs.DISABLE_KEYBOARD_NAVIGATION ==
'true' / 'false') with direct boolean checks (use
inputs.DISABLE_KEYBOARD_NAVIGATION and !inputs.DISABLE_KEYBOARD_NAVIGATION) in
the workflows that reference it.
---
Outside diff comments:
In @.github/workflows/build-pr.yml:
- Around line 76-114: The DISABLE_KEYBOARD_NAVIGATION input is passed as YAML
booleans but e2e-build-android.yml declares it as type: string and
maestro-android.yml mismatches; standardize on boolean: change the input
declaration DISABLE_KEYBOARD_NAVIGATION in e2e-build-android.yml to type:
boolean, update any conditionals in e2e-build-android.yml that compare inputs to
'true'/'false' to use direct boolean tests (e.g., if:
inputs.DISABLE_KEYBOARD_NAVIGATION), and make matching boolean comparisons in
maestro-android.yml so both reusable workflows agree; then keep the callers'
with: DISABLE_KEYBOARD_NAVIGATION: true/false as bare booleans.
In @.github/workflows/e2e-build-android.yml:
- Around line 44-53: The cache key for the "Cache Android build" step currently
omits the DISABLE_KEYBOARD_NAVIGATION environment variable, causing parallel
runs with different values to collide; update the key (and restore-keys) used in
the actions/cache step (the line starting with key: android-build-${{ runner.os
}}-${{ hashFiles(... ) }}) to incorporate the DISABLE_KEYBOARD_NAVIGATION value
(e.g. append -${{ env.DISABLE_KEYBOARD_NAVIGATION }} or similar) so that builds
with DISABLE_KEYBOARD_NAVIGATION=true and =false produce distinct cache keys and
cannot pull each other's android/app/build artifacts.
In @.github/workflows/maestro-android.yml:
- Around line 81-171: The AVD creation and emulator-run steps have their
DISABLE_KEYBOARD_NAVIGATION checks inverted: the "Create Keyboard-enabled AVD"
and "Start Android Emulator and Run Maestro Tests (keyboard AVD)" blocks
currently run when inputs.DISABLE_KEYBOARD_NAVIGATION == 'true' but should run
when it's 'false', and the "Start Android Emulator and Run Maestro Tests
(default)" block should run when it's 'true'; update those three if: conditions
(or swap the boolean values) so Pixel_API_34_Keyboard (AVD_NAME) and the
keyboard AVD run when DISABLE_KEYBOARD_NAVIGATION == 'false', while the default
pixel_7_pro profile with enable-hw-keyboard: false runs when
DISABLE_KEYBOARD_NAVIGATION == 'true'.
In `@app/views/RoomView/index.tsx`:
- Around line 266-276: The environment-var check is inverted so keyboard-nav
builds are skipping autofocus; update the conditional in the autofocus block to
skip when in dev OR when process.env.DISABLE_KEYBOARD_NAVIGATION_ENABLED ===
'true' (i.e. keyboard navigation disabled), and only run
isExternalKeyboardConnected() and call this.messageComposerRef.current?.focus()
when the env var is not 'true'; modify the conditional that currently checks
process.env.DISABLE_KEYBOARD_NAVIGATION_ENABLED === 'false' accordingly so the
logic is reversed.
🪄 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: fa8ac9a5-4670-4c82-8382-a2db34dcdb7b
📒 Files selected for processing (8)
.github/workflows/build-pr.yml.github/workflows/e2e-build-android.yml.github/workflows/maestro-android.yml.maestro/tests/room/room-actions.yaml.maestro/tests/room/room.yaml.maestro/tests/room/threads.yamlapp/views/RoomView/index.tsxscripts/create-avd.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: E2E Build iOS / ios-build
- GitHub Check: E2E Build Android Keyboard Navigation / android-build
- GitHub Check: E2E Build Android / android-build
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/RoomView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/views/RoomView/index.tsx
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character width, no trailing commas, avoid arrow parens, and bracket same line
Files:
app/views/RoomView/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/views/RoomView/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/RoomView/index.tsx
🧠 Learnings (3)
📚 Learning: 2026-03-05T14:28:10.004Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6997
File: .maestro/tests/room/message-markdown-click.yaml:28-39
Timestamp: 2026-03-05T14:28:10.004Z
Learning: In Maestro YAML selector fields (text, id) within the Rocket.Chat React Native repository, use the contains pattern '.*keyword.*' (leading and trailing '.*') for matching text. The pattern '.*keyword*.' is incorrect and will fail to match cases where the keyword appears at the end of the element's text. This guideline applies to all Maestro YAML selector fields across the codebase.
Applied to files:
.maestro/tests/room/room.yaml.maestro/tests/room/room-actions.yaml.maestro/tests/room/threads.yaml
📚 Learning: 2026-03-17T19:15:26.536Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:26.536Z
Learning: In YAML test files under .maestro/tests/room, use tapping the empty area (e.g., tapOn: point: 5%,10%) to dismiss both the bottom sheet and keyboard when needed. Do not rely on action-sheet-handle alone if the keyboard also needs to be dismissed in the same step. This pattern is acceptable for tests where a single tap should close both UI elements.
Applied to files:
.maestro/tests/room/room.yaml.maestro/tests/room/room-actions.yaml.maestro/tests/room/threads.yaml
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/RoomView/index.tsx
🪛 actionlint (1.7.12)
.github/workflows/build-pr.yml
[error] 77-77: input "DISABLE_KEYBOARD_NAVIGATION" is typed as string by reusable workflow "./.github/workflows/e2e-build-android.yml". bool value cannot be assigned
(expression)
[error] 100-100: input "DISABLE_KEYBOARD_NAVIGATION" is typed as string by reusable workflow "./.github/workflows/e2e-build-android.yml". bool value cannot be assigned
(expression)
🪛 Shellcheck (0.11.0)
scripts/create-avd.sh
[style] 74-74: Consider using { cmd1; cmd2; } >> file instead of individual redirects.
(SC2129)
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
official_android_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
Rohit3523
changed the title
1 participant
Proposed changes
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-2185
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Chores