Skip to content

Conversation

@Rohit3523
Copy link
Collaborator

@Rohit3523 Rohit3523 commented Dec 5, 2025

Proposed changes

  1. Updated the test workspace endpoint.
  2. On the Create Account screen, the password input was covered by iOS secure text. To address this, secure text was disabled for E2E tests.
Screenshot 2025-12-08 at 12 08 56 AM
  1. On the Delete Server screen, Maestro was unable to click the Delete button, so a tap point was added.
  2. Added retryTapIfNoChange to improve test stability.
  3. Reduced the in-app notification duration from 60 seconds to 5 seconds for e2e builds.
  4. Made the unread badge Android-only because it was flaky on iOS.
  5. Added a new password generation function to meet the password requirement conditions.
  6. Improved the bash script to run correctly on macOS.
  7. Increased the test run limit from 3 to 4 because tests sometimes fail due to API errors.
  8. Fixed a few typos by using down in uppercase instead of lowercase in the scroll direction.

Working Action Link: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/20311081572/job/58355918165

Issue(s)

https://rocketchat.atlassian.net/browse/COMM-104

How to test or reproduce

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

  • Tests
    • Migrated many test cases and test data to the QA environment and replaced regex matches with exact text checks.
    • Added platform-specific test flows, animation waits, retry taps, and increased test rerun limit for more reliable UI tests.
  • New Features
    • Added a password generator utility used by test/data scripts.
  • Chores
    • Updated seed/test URLs to point at the QA server.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

E2E test infra and test data updated: Maestro rerun/dedup logic changed, seed/test URLs switched to QA, new password generator added, multiple Maestro test flows made platform-specific or more deterministic, and small app E2E-related adjustments to notification timing and password masking.

Changes

Cohort / File(s) Summary
Maestro CI script
.github/scripts/run-maestro.sh
Increased MAX_RERUN_ROUNDS from 2 to 3 and replaced a Bash associative-array SEEN with a string ledger SEEN_PATHS using grep-based containment checks for deduplication.
Workflows
.github/workflows/maestro-android.yml, .github/workflows/maestro-ios.yml
Only formatting/trailing-newline-level edits observed (no behavior changes); retention-days: 7 remains.
Maestro seed & helpers
.maestro/README.md, .maestro/scripts/data.js, .maestro/scripts/random.js
Switched seed server URL to https://mobile.qa.rocket.chat; added generatePassword(length = 10) and updated randomUser to use it.
Maestro test YAMLs — selectors & platform branches
.maestro/tests/assorted/accessibility-and-appearance.yaml, .maestro/tests/assorted/changeserver.yaml, .maestro/tests/assorted/delete-server.yaml, .maestro/tests/assorted/user-preferences.yaml, .maestro/tests/onboarding/server-history.yaml, .maestro/tests/room/room.yaml, .maestro/tests/room/unread-badge.yaml, .maestro/tests/teams/utils/close-action-sheet.yaml
Replaced regex text matches with exact text, added platform-specific branches (iOS coordinate taps vs Android text taps) and retry behavior, migrated many IDs to deterministic id: selectors, changed swipe direction case to DOWN, added animation waits and retryTapIfNoChange, and swapped some ASCII emoji to emoji glyphs.
App tests — QA domain migration
app/lib/methods/getQuoteAttachment.test.ts, app/lib/methods/helpers/getAvatarUrl.test.ts, app/reducers/server.test.ts, app/views/NewServerView/utils/completeUrl.test.ts
Updated test fixtures and expected values to use mobile.qa.rocket.chat instead of mobile.rocket.chat.
App E2E adjustments
app/containers/InAppNotification/index.tsx, app/views/RegisterView/index.tsx
Changed E2E notification fallback duration from 60000ms to 5000ms when RUNNING_E2E_TESTS is truthy; made secureTextEntry false during E2E when RUNNING_E2E_TESTS === 'true'.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review focus:
    • .github/scripts/run-maestro.sh — verify grep-based deduplication correctness and side-effects of string ledger approach.
    • .maestro/scripts/random.js — ensure generatePassword() enforces required character classes and no triple repeats.
    • YAML platform-specific taps (delete-server.yaml, changeserver.yaml) — validate coordinate accuracy for iOS and retry semantics.
    • app/containers/InAppNotification/index.tsx and RegisterView — confirm env-gated behavior matches test expectations.

Possibly related PRs

Suggested reviewers

  • diegolmello

Poem

🐰 I hopped from prod to QA with glee,

Three reruns now to set tests free,
Passwords crafted with hops and flair,
Taps that retry — precise and fair,
Maestro paths now tracked in a string-y tree.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'chore: new test workspace' is overly vague and does not clearly summarize the main changes. The PR contains 11 distinct functional updates to test infrastructure and app behavior, not just a workspace change. Revise the title to reflect the primary change or use a more descriptive phrase like 'chore: update e2e test infrastructure and app behavior' to better convey the scope of changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch new-mobile-workspace

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing December 5, 2025 12:50 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to official_android_build December 5, 2025 12:54 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build December 5, 2025 12:54 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build December 5, 2025 12:54 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing December 6, 2025 21:04 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build December 6, 2025 21:08 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build December 6, 2025 21:08 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build December 6, 2025 21:08 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing December 7, 2025 16:31 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build December 7, 2025 16:34 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build December 17, 2025 19:34 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build December 17, 2025 19:34 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build December 17, 2025 19:34 — with GitHub Actions Error
@Rohit3523 Rohit3523 marked this pull request as ready for review December 18, 2025 14:52
Copy link
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

🧹 Nitpick comments (1)
.maestro/README.md (1)

42-42: Documentation updated correctly, with a minor style suggestion.

The documentation now accurately reflects the QA server being used for testing.

Optional: Address markdown linting warning

The markdown linter flags this as a bare URL. Consider wrapping it in angle brackets or code formatting for consistency:

-- Currently we point to https://mobile.qa.rocket.chat as main server
+- Currently we point to <https://mobile.qa.rocket.chat> as main server

or

-- Currently we point to https://mobile.qa.rocket.chat as main server
+- Currently we point to `https://mobile.qa.rocket.chat` as main server
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 59e873a and 9339a40.

📒 Files selected for processing (20)
  • .github/scripts/run-maestro.sh (2 hunks)
  • .github/workflows/maestro-android.yml (2 hunks)
  • .github/workflows/maestro-ios.yml (2 hunks)
  • .maestro/README.md (1 hunks)
  • .maestro/scripts/data.js (2 hunks)
  • .maestro/scripts/random.js (1 hunks)
  • .maestro/tests/assorted/accessibility-and-appearance.yaml (4 hunks)
  • .maestro/tests/assorted/changeserver.yaml (1 hunks)
  • .maestro/tests/assorted/delete-server.yaml (1 hunks)
  • .maestro/tests/assorted/user-preferences.yaml (1 hunks)
  • .maestro/tests/onboarding/server-history.yaml (2 hunks)
  • .maestro/tests/room/room.yaml (3 hunks)
  • .maestro/tests/room/unread-badge.yaml (9 hunks)
  • .maestro/tests/teams/utils/close-action-sheet.yaml (1 hunks)
  • app/containers/InAppNotification/index.tsx (1 hunks)
  • app/lib/methods/getQuoteAttachment.test.ts (4 hunks)
  • app/lib/methods/helpers/getAvatarUrl.test.ts (1 hunks)
  • app/reducers/server.test.ts (1 hunks)
  • app/views/NewServerView/utils/completeUrl.test.ts (2 hunks)
  • app/views/RegisterView/index.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-16T12:13:16.929Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6598
File: .maestro/tests/room/room.yaml:416-419
Timestamp: 2025-09-16T12:13:16.929Z
Learning: Maestro swipe command accepts uppercase direction values: UP, DOWN, LEFT, RIGHT (not lowercase as incorrectly suggested). The official documentation shows examples using uppercase direction values.

Applied to files:

  • .maestro/tests/teams/utils/close-action-sheet.yaml
🧬 Code graph analysis (1)
.maestro/scripts/data.js (1)
.maestro/scripts/data-setup.js (1)
  • output (7-7)
🪛 markdownlint-cli2 (0.18.1)
.maestro/README.md

42-42: Bare URL used

(MD034, no-bare-urls)

🪛 Shellcheck (0.11.0)
.github/scripts/run-maestro.sh

[warning] 56-56: name appears unused. Verify use (or export if used externally).

(SC2034)

⏰ 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). (2)
  • GitHub Check: ESLint and Test / run-eslint-and-test
  • GitHub Check: format
🔇 Additional comments (19)
app/containers/InAppNotification/index.tsx (1)

62-62: Excellent improvement to E2E test performance!

Reducing the notification duration from 60 seconds to 5 seconds for E2E builds is a significant improvement that will speed up test execution while still providing sufficient visibility. The fallback to 3 seconds for normal operation is also appropriate, and the logic correctly prioritizes notification.customTime when provided.

.maestro/tests/assorted/accessibility-and-appearance.yaml (1)

90-90: LGTM! Improved test reliability with exact text matches.

Replacing regex patterns with exact text matches makes these assertions more deterministic and performant while reducing false positives. The overly permissive .*text.* patterns could have matched unintended substrings.

Also applies to: 94-94, 129-129, 133-133, 168-168, 203-203

.maestro/tests/assorted/user-preferences.yaml (1)

84-90: LGTM - Test logic improved.

The removal of the redundant notVisible check for ':(' improves the test correctness. The current flow properly validates:

  • When ASCII-to-emoji is OFF: ':(' is visible (line 84-85) and '😞' is NOT visible (lines 87-90) ✓
  • When ASCII-to-emoji is ON: '😞' is visible (line 142) ✓

Having both visible and notVisible checks for the same text without intermediate actions would have been contradictory.

.maestro/tests/teams/utils/close-action-sheet.yaml (1)

9-9: LGTM! Direction value corrected to uppercase.

The swipe direction is now uppercase (DOWN), which aligns with Maestro's official convention for direction values.

Based on learnings, Maestro accepts uppercase direction values (UP, DOWN, LEFT, RIGHT) as documented in the official examples.

app/reducers/server.test.ts (1)

52-52: LGTM! Test data updated to QA domain.

The test data URL has been correctly updated to align with the broader QA domain migration across the test suite.

app/views/NewServerView/utils/completeUrl.test.ts (1)

17-17: LGTM! Test expectations updated to QA domain.

The test expectations have been correctly updated to use the QA domain, maintaining the same test logic while aligning with the broader domain migration across the test suite.

Also applies to: 21-21, 34-34

.maestro/scripts/data.js (1)

2-2: LGTM! Seed data updated to QA domain.

The server URL has been correctly updated to the QA domain, and the file now has a proper trailing newline, aligning with the broader test environment migration.

Also applies to: 18-18

.github/workflows/maestro-ios.yml (1)

39-39: LGTM! Maestro installation URL updated for reliability.

The installation URL change is consistent with the Android workflow and addresses the reliability issue when the primary endpoint is unavailable. The verification requested in the Android workflow review applies here as well.

.maestro/tests/onboarding/server-history.yaml (1)

26-26: LGTM! Element IDs updated to QA domain.

All server history element identifiers have been correctly updated to reference the QA domain. The test flow logic remains unchanged, and the updates are consistent with the server URL change in the seed data.

Also applies to: 30-30, 46-46, 52-52, 56-56

.maestro/tests/room/unread-badge.yaml (2)

7-7: Good practice to isolate flaky tests.

Restricting this test to Android-only addresses the iOS flakiness mentioned in the PR objectives. This improves overall test reliability.


25-26: Animation synchronization improves test stability.

The addition of waitForAnimationToEnd blocks throughout the test helps ensure UI state is settled before assertions, reducing race conditions.

Also applies to: 38-39, 53-54, 66-67, 82-83, 95-96, 111-112, 124-125

.maestro/scripts/random.js (1)

10-32: Password generation meets requirements.

The function correctly ensures passwords contain at least one lowercase, uppercase, and numeral, and prevents three consecutive identical characters. The initial three characters are guaranteed diverse since they come from separate character sets.

app/lib/methods/helpers/getAvatarUrl.test.ts (1)

7-7: Test data migration to QA domain.

This change aligns with the broader migration to QA endpoints for test stability, as indicated in the PR objectives.

Also applies to: 10-10

.github/scripts/run-maestro.sh (2)

8-8: Increased retry limit improves resilience.

Increasing MAX_RERUN_ROUNDS from 2 to 3 addresses intermittent API failures as mentioned in the PR objectives, improving overall test stability.


54-60: String-based deduplication improves macOS compatibility.

Replacing the associative array with string-based tracking using grep ensures compatibility with macOS's default bash 3.x, which doesn't support associative arrays. While less efficient, this approach is more portable.

Note on static analysis warning: The name variable at line 56 is intentionally read but unused. It's part of the tab-separated parsing where only path is needed for deduplication. The shellcheck warning is a false positive.

app/lib/methods/getQuoteAttachment.test.ts (1)

22-22: Test data migration to QA domain.

Comprehensive update of test fixtures to use QA endpoints, consistent with the broader migration strategy across the PR.

Also applies to: 30-30, 50-50, 70-70, 91-91

.maestro/tests/room/room.yaml (3)

259-259: LGTM! Direction case now consistent.

The uppercase DOWN aligns with the existing UP direction at line 420, establishing consistency across swipe actions.


268-272: LGTM! Improved selector determinism and tap reliability.

The ID-based selector and retryTapIfNoChange flag enhance test stability by targeting elements deterministically and handling scenarios where the tap action may not produce immediate visible changes.


293-297: LGTM! Consistent application of improved selector pattern.

The same ID-based selector approach and retry mechanism are correctly applied to the other user's profile interaction, maintaining consistency with the pattern established earlier in the file.

Comment on lines +183 to +196
- runFlow:
when:
platform: iOS
commands:
- tapOn:
point: 65%,55%
retryTapIfNoChange: true
- runFlow:
when:
platform: Android
commands:
- tapOn:
text: 'Delete'
retryTapIfNoChange: true
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use text or accessibility identifier-based selectors instead of coordinates for cross-device compatibility.

The iOS flow's hardcoded coordinates (65%, 55%) are device-dependent and may fail on different screen sizes or resolutions. The Android flow already uses the superior text: 'Delete' selector. Consider whether the Delete button has an accessibility identifier and update both flows consistently:

- runFlow:
    when:
      platform: iOS
    commands:
      - tapOn:
          text: 'Delete'
          retryTapIfNoChange: true
- runFlow:
    when:
      platform: Android
    commands:
      - tapOn:
          text: 'Delete'
          retryTapIfNoChange: true

Note: retryTapIfNoChange addresses timing issues with screen hierarchy changes, not coordinate fragility.

🤖 Prompt for AI Agents
.maestro/tests/assorted/changeserver.yaml around lines 183 to 196: the iOS
runFlow uses hardcoded screen coordinates (65%,55%) which are device-dependent;
replace the coordinate-based tap with a selector-based tap (use the button's
text or its accessibility identifier) and make the iOS flow consistent with the
Android flow by using the same tapOn selector (e.g., text: 'Delete' or
accessibilityId: '<identifier>') while keeping retryTapIfNoChange: true to
preserve retry behavior.

Comment on lines +96 to +109
- runFlow:
when:
platform: iOS
commands:
- tapOn:
point: 65%,55%
retryTapIfNoChange: true
- runFlow:
when:
platform: Android
commands:
- tapOn:
text: 'Delete'
retryTapIfNoChange: true
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Coordinate-based taps are fragile across device configurations.

Same concern as in changeserver.yaml: the iOS flow uses hardcoded coordinates (65%, 55%) which may fail on devices with different screen configurations. Consider using ID-based selectors if available.

🤖 Prompt for AI Agents
In .maestro/tests/assorted/delete-server.yaml around lines 96 to 109, the iOS
step uses a hardcoded coordinate tap (65%,55%) which is fragile; replace the
coordinate-based tap with an element-based selector (accessibility id,
resource-id, label or visible text) that targets the Delete button, preserve
retryTapIfNoChange behavior, and add a safe fallback to the coordinate tap only
if the selector lookup fails so tests remain robust across screen sizes and
configurations.

required
label={I18n.t('Password')}
secureTextEntry
secureTextEntry={process.env.RUNNING_E2E_TESTS !== 'true'}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Verify test data hygiene to prevent password exposure.

Disabling secureTextEntry during E2E tests makes passwords visible in plain text, which could appear in test artifacts (screenshots, recordings, logs). Ensure test passwords are synthetic and never reused.

Also applies to: 258-258

🤖 Prompt for AI Agents
In app/views/RegisterView/index.tsx around lines 243 and 258, tests currently
disable secureTextEntry for E2E which can expose passwords in artifacts; either
keep secureTextEntry enabled at all times or, if E2E requires visible input,
ensure the code pulls only synthetic, non-reused test passwords (generated by
the test harness or from a dedicated TEST_PASSWORD env var) and never real
credentials, and add a comment and/or assert to make it explicit these values
are ephemeral and not stored or reused.

@diegolmello diegolmello merged commit 0d13c14 into develop Dec 24, 2025
6 of 7 checks passed
@diegolmello diegolmello deleted the new-mobile-workspace branch December 24, 2025 14:06
@diegolmello diegolmello requested a deployment to experimental_ios_build December 24, 2025 14:08 — with GitHub Actions Waiting
@diegolmello diegolmello requested a deployment to experimental_android_build December 24, 2025 14:08 — with GitHub Actions Waiting
@diegolmello diegolmello requested a deployment to official_android_build December 24, 2025 14:08 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants