Skip to content

Conversation

@Rohit3523
Copy link
Contributor

@Rohit3523 Rohit3523 commented Oct 1, 2025

Proposed changes

This PR introduced deep link authentication. This approach allows quick login and eliminates the need to go through the standard authentication flow, significantly reducing test time.

Test 1: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/18312635832
Test 2: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/18313860204
Test 3: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/18323003939

Issue(s)

https://rocketchat.atlassian.net/browse/NATIVE-967

How to test or reproduce

N/A

Screenshots

N/A

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

  • New Features

    • Added a deeplink-based login helper; deeplink opening now scoped to iOS.
  • Tests

    • Many E2E tests switched to the deeplink login, some adding in-flow user creation and CLEAR_STATE flags; a few launch-app steps removed and some searches now submit with Enter.
  • Chores

    • Removed an unnecessary debug log in test setup scripts.
  • Style

    • Minor formatting cleanups across test and workflow files.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Adds a new Maestro helper that logs in via a deeplink, updates many Maestro test YAMLs to use that helper (some tests now create users first or remove explicit app-launch steps), narrows open-deeplink to iOS, removes a debug log, and tweaks CI artifact upload metadata.

Changes

Cohort / File(s) Summary of Changes
CI: Maestro artifact upload
.github/workflows/maestro-android.yml
Removed and re-added a retention-days line on the "Upload Screen Recording" step (no behavior change) and kept Maestro artifact upload step (artifact: maestro-${{ inputs.shard }}, path: ~/.maestro/tests/).
New helper: deeplink login
.maestro/helpers/login-with-deeplink.yaml
Added helper flow that performs auth via evalScript, constructs a deeplink, opens it, handles permission prompt approval, and waits for rooms-list-view.
Open-deeplink platform guard
.maestro/helpers/open-deeplink.yaml
Added when.platform: iOS guard to limit the runFlow open-deeplink action to iOS.
Bulk tests — switch to deeplink login
.maestro/tests/assorted/*, .maestro/tests/room/*, .maestro/tests/teams/*, .maestro/tests/onboarding/*, .maestro/tests/accessibilityAndAppearance/*
Replaced many references to login.yaml with login-with-deeplink.yaml; removed some launch-app.yaml steps and made minor formatting edits (blank-line removals).
Tests: create user then deeplink login
.maestro/tests/accessibilityAndAppearance/ToastsAndDialogs.yml, .maestro/tests/onboarding/roomslist.yaml, .maestro/tests/room/create-room.yaml, .maestro/tests/teams/create-team.yaml, .maestro/tests/teams/convert-team.yaml
Inserted evalScript steps to create users (and a random team name where applicable) and then run login-with-deeplink.yaml with created credentials via env.
Tests: small interaction tweaks
.maestro/tests/assorted/join-from-directory.yaml
Removed debounce workaround, added pressKey: Enter after search inputs to trigger results, and switched login to deeplink helper.
Tests: server-history onboarding enhancements
.maestro/tests/onboarding/server-history.yaml
Create user then deeplink login; updated username references and added waits, navigation, delete actions, and visibility checks for server history traversal.
Tests: env var key change
.maestro/tests/assorted/user-preferences.yaml
Switched to login-with-deeplink.yaml and changed env keys from USERNAME/PASSWORD to lowercase username/password.
Script: logging cleanup
.maestro/scripts/data-setup.js
Removed a console.log in createDM; no functional change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant TestFlow as Test Flow
  participant Helper as login-with-deeplink
  participant Eval as EvalScript (auth)
  participant DeeplinkFlow as open-deeplink (iOS)
  participant App as Mobile App

  TestFlow->>Helper: runFlow (env: USERNAME/PASSWORD or created user)
  Helper->>Eval: evalScript -> perform login (server,userId,authToken)
  Eval-->>Helper: login result
  Helper->>DeeplinkFlow: runFlow (open constructed deeplink)
  DeeplinkFlow->>App: open URL (iOS)
  App-->>Helper: permission prompt (if present)
  Helper->>App: tap approve -> extendedWaitUntil rooms-list-view
  App-->>Helper: rooms-list-view visible
  Helper-->>TestFlow: flow complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • diegolmello
  • OtavioStasiak

Poem

hop-hop, a deeplink bright and quick,
I leap through flows both neat and slick,
fewer logs, a guarded open gate,
artifacts tucked, new users create,
carrot cheers — the rabbit's click 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes modifications to the CI workflow retention-days setting in .github/workflows/maestro-android.yml and removes a console.log line in scripts/data-setup.js, neither of which relates to implementing deep link authentication in Maestro tests as described in NATIVE-967. These incidental changes fall outside the scope of the issue’s goals and testing improvements. Please remove or isolate the CI workflow retention setting change and the data-setup console.log removal into a separate commit or pull request, so that this PR exclusively addresses deep link authentication enhancements.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “test: use deeplink authentication” succinctly conveys the primary change of switching to deep link authentication in tests, clearly reflecting the main objective of the changeset without extraneous details. It identifies the scope as test-related and specifies the feature being introduced. This phrasing is concise, specific, and immediately signals to reviewers the core intent of the pull request.
Linked Issues Check ✅ Passed The pull request fully implements deep link authentication by adding a new login-with-deeplink helper, updating all Maestro test flows to use the new deep link login, and adjusting related helpers such as open-deeplink, which aligns directly with the intent of NATIVE-967 to accelerate test login. The changes consistently replace the old login flow across the suite and introduce state‐clearing flags where appropriate, matching the linked issue’s requirement for bypassing the standard authentication flow.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch deeplink-login-maestro

📜 Recent review details

Configuration used: CodeRabbit 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 b50c9dd and f5d81d9.

📒 Files selected for processing (1)
  • .github/workflows/maestro-android.yml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/maestro-android.yml
⏰ 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). (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test

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 October 1, 2025 19:04 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build October 1, 2025 19:07 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build October 1, 2025 19:07 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build October 1, 2025 19:07 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing October 1, 2025 19:54 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build October 1, 2025 19:57 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build October 1, 2025 19:57 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build October 1, 2025 19:57 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing October 1, 2025 20:51 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to official_android_build October 1, 2025 20:54 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build October 1, 2025 20:54 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build October 1, 2025 20:54 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing October 3, 2025 13:40 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build October 3, 2025 13:42 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build October 3, 2025 13:42 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing October 7, 2025 18:06 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build October 7, 2025 18:09 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build October 7, 2025 18:09 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build October 7, 2025 18:09 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing October 7, 2025 18:52 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to official_android_build October 7, 2025 18:55 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build October 7, 2025 18:55 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build October 7, 2025 18:55 — with GitHub Actions Error
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: 1

♻️ Duplicate comments (1)
.maestro/helpers/login-with-deeplink.yaml (1)

7-13: Revisit CLEAR_STATE/stopApp gating; when: true: CLEAR_STATE looks suspect.

  • Maestro typically uses when: { platform: android } and a boolean condition (or similar) for env‑based gating. Please verify true: CLEAR_STATE works as intended.
  • Prior feedback suggested removing this to let the deeplink open the app. Consider dropping the stop/clear or making it opt‑in.

Example options:

  • Keep Android‑only, no env gate:
-  when:
-    true: CLEAR_STATE
-    platform: android
+  when:
+    platform: android
  • Or explicitly gate on env (if supported in your setup):
   when:
-    true: CLEAR_STATE
     platform: android
+    condition: ${Boolean(CLEAR_STATE)}
🧹 Nitpick comments (1)
.maestro/helpers/login-with-deeplink.yaml (1)

23-38: Tighten Android prompt handling; “Allow” regex is broad and flaky.

  • Prefer targeting the permission button by resource id (e.g., com.android.permissioncontroller:id/permission_allow_button) and handle variants (“Allow”, “While using the app”, “Allow only this time”).
  • The “Pixel Launcher” → “Close App” tap is brittle; remove unless there’s a proven crash‑loop case.

Suggested shape:

  • Try id‑based tap with short waits; fallback to text variants; all optional.
📜 Review details

Configuration used: CodeRabbit 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 17e62de and b50c9dd.

📒 Files selected for processing (1)
  • .maestro/helpers/login-with-deeplink.yaml (1 hunks)
⏰ 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). (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (2)
.maestro/helpers/login-with-deeplink.yaml (2)

39-42: Final wait looks good.

Waiting for rooms-list-view for 60s is a solid success signal.


15-18: Open Deep Link already executes on Android
open-deeplink.yaml begins with an unconditional openLink step; the platform: iOS guard only applies to the subsequent tap. No extra Android-specific openLink is required.

Likely an incorrect or invalid review comment.

- runFlow:
file: 'open-deeplink.yaml'
env:
link: ${output.utils.getDeepLink('auth', server || output.data.server, 'userId=', output.login.userId, '&token=', output.login.authToken)}
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 | 🟠 Major

Token in deeplink can leak in logs/artifacts; add safeguards.

Opening a URL with authToken risks exposure in Maestro logs and uploaded CI artifacts. Mitigate by:

  • Masking/redacting the link in logs, or
  • Using a short‑lived/ephemeral code instead of raw token, and/or
  • Revoking the token post‑login (logout API) at test teardown, and ensuring artifacts are private.

I can provide a small teardown helper to revoke tokens after tests if desired.

@diegolmello diegolmello temporarily deployed to approve_e2e_testing October 7, 2025 19:46 — with GitHub Actions Inactive
@diegolmello diegolmello had a problem deploying to experimental_android_build October 7, 2025 19:49 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to official_android_build October 7, 2025 19:49 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to experimental_ios_build October 7, 2025 19:49 — with GitHub Actions Failure
@diegolmello diegolmello merged commit b6b3775 into develop Oct 7, 2025
21 of 25 checks passed
@diegolmello diegolmello deleted the deeplink-login-maestro branch October 7, 2025 20:27
@coderabbitai coderabbitai bot mentioned this pull request Oct 8, 2025
12 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 19, 2025
10 tasks
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.

2 participants