ci(e2e): Add Expo native E2E testing#9186
Conversation
🦋 Changeset detectedLatest commit: 4b3447f The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughExpo Native integration testing was added with Maestro flows, fixture authentication controls, a retrying flow runner, and iOS/Android CI execution. The workflow provisions temporary credentials, captures diagnostics, and cleans up test users. ChangesExpo Native E2E
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved This PR adds CI/e2e testing infrastructure (Maestro flows, workflow updates) for Expo native components. All changes are confined to test fixtures, test files, and GitHub workflows with no production code impact. You can customize Macroscope's approvability policy. Learn more. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
integration/tests/expo-native/config.yaml (1)
8-11: 🩺 Stability & Availability | 🔵 TrivialConfigure animation suppression in the actual local device setup.
Maestro documents
disableAnimationsas cloud-only, while the runner executes localmaestro testcommands. These settings are therefore ignored in this CI path and will not reduce simulator/emulator flakiness. (docs.maestro.dev)🤖 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 `@integration/tests/expo-native/config.yaml` around lines 8 - 11, Replace the ineffective disableAnimations entries in the iOS and Android configuration with the local device setup mechanism supported by the runner, ensuring animations are disabled before local maestro test execution for both platforms. Remove the cloud-only settings rather than retaining them as the suppression path.
🤖 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/expo-native-build.yml:
- Around line 118-129: Update the “Resolve Clerk instance keys” and “Provision
test user via BAPI” steps to stop masking setup failures with continue-on-error.
Ensure key-resolution or user-provisioning failures make the burn-in job fail or
are surfaced as the burn-in outcome, while retaining conditional skipping only
when fork secrets are unavailable.
In `@integration/tests/expo-native/flows/subflows/open-app.yaml`:
- Around line 10-12: Update the retry comments in open-app.yaml (lines 10-12)
and assert-signed-in.yaml (lines 1-2) to state that Maestro polls assertVisible
until its default 7-second timeout; remove wording that implies assertVisible
never retries. No other behavior changes are needed.
In `@integration/tests/expo-native/flows/subflows/sign-in-email-password.yaml`:
- Around line 14-28: Update the email/username input flow around the initial tap
in the sign-in subflow to call eraseText after focusing the field, matching the
password flow. Keep the conditional Select All handling if needed, but ensure
the persisted identifier is cleared unconditionally before inputText enters
CLERK_TEST_EMAIL.
---
Nitpick comments:
In `@integration/tests/expo-native/config.yaml`:
- Around line 8-11: Replace the ineffective disableAnimations entries in the iOS
and Android configuration with the local device setup mechanism supported by the
runner, ensuring animations are disabled before local maestro test execution for
both platforms. Remove the cloud-only settings rather than retaining them as the
suppression path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: c9312e4c-7110-4c0f-a075-501c5f38dc6a
📒 Files selected for processing (15)
.changeset/new-queens-brake.md.github/workflows/expo-native-build.yml.github/workflows/mobile-e2e.ymlintegration/templates/expo-native/App.tsxintegration/templates/expo-native/package.sdk-57.jsonintegration/tests/expo-native/.gitignoreintegration/tests/expo-native/config.yamlintegration/tests/expo-native/flows/sign-in.yamlintegration/tests/expo-native/flows/subflows/_warmup.yamlintegration/tests/expo-native/flows/subflows/assert-signed-in.yamlintegration/tests/expo-native/flows/subflows/assert-signed-out.yamlintegration/tests/expo-native/flows/subflows/open-app.yamlintegration/tests/expo-native/flows/subflows/sign-in-email-password.yamlintegration/tests/expo-native/run-flows.shpackage.json
💤 Files with no reviewable changes (1)
- .github/workflows/mobile-e2e.yml
| - name: Resolve Clerk instance keys | ||
| id: keys | ||
| if: steps.gate.outputs.ok == 'true' | ||
| continue-on-error: true | ||
| env: | ||
| INTEGRATION_STAGING_INSTANCE_KEYS: ${{ secrets.INTEGRATION_STAGING_INSTANCE_KEYS }} | ||
| run: node scripts/resolve-instance-keys.mjs INTEGRATION_STAGING_INSTANCE_KEYS "$E2E_INSTANCE_NAME" | ||
|
|
||
| - name: Provision test user via BAPI | ||
| id: user | ||
| if: steps.keys.outputs.pk != '' | ||
| continue-on-error: true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not silently convert E2E setup failures into skipped coverage.
If key resolution or user provisioning fails, the empty user ID skips every E2E and reporting step while the job succeeds. Reserve skipping for unavailable fork secrets; otherwise report the setup failure as the burn-in outcome or remove these continue-on-error settings.
🤖 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/expo-native-build.yml around lines 118 - 129, Update the
“Resolve Clerk instance keys” and “Provision test user via BAPI” steps to stop
masking setup failures with continue-on-error. Ensure key-resolution or
user-provisioning failures make the burn-in job fail or are surfaced as the
burn-in outcome, while retaining conditional skipping only when fork secrets are
unavailable.
| # Text-only matching from here on: iOS failed combined id+text filters on RN | ||
| # Text elements in CI while id-only and text-only both match. Also wait out | ||
| # clerk-js init ('loading') with a retrying wait; assertVisible never retries. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' integration/tests/expo-native/flows/subflows/open-app.yaml
printf '\n---\n'
sed -n '1,80p' integration/tests/expo-native/flows/subflows/assert-signed-in.yaml
printf '\n---\n'
rg -n "assertVisible|extendedWaitUntil|loading|text-only|id-only|combined id\\+text" integration/tests/expo-native -SRepository: clerk/javascript
Length of output: 2017
🌐 Web query:
Maestro assertVisible retries default timeout documentation
💡 Result:
The default timeout for the assertVisible command in Maestro is 7 seconds [1][2]. When you call assertVisible, Maestro does not fail immediately if the element is not found [1][3]. Instead, it automatically waits and polls for the element to appear, retrying until it becomes visible or the 7-second timeout is reached [1][3]. If you need a longer wait time than the 7-second default, you should use the extendedWaitUntil command, which allows you to specify a custom timeout [1][4]. Key points: - Default Timeout: 7 seconds [1][2]. - Behavior: Continuous polling for the element's visibility until the timeout expires [3]. - Alternative: Use extendedWaitUntil for scenarios requiring a longer duration [1][4].
Citations:
- 1: https://docs.maestro.dev/reference/commands-available/assertvisible
- 2: https://docs.maestro.dev/reference/commands-available/assertvisible.md
- 3: https://docs.maestro.dev/maestro-flows/flow-control-and-logic/wait-commands
- 4: https://docs.maestro.dev/reference/commands-available/extendedwaituntil
Correct the assertVisible retry note. Maestro polls assertVisible until its 7-second default timeout, so the comments in integration/tests/expo-native/flows/subflows/open-app.yaml and integration/tests/expo-native/flows/subflows/assert-signed-in.yaml should say that instead of implying it never retries.
📍 Affects 2 files
integration/tests/expo-native/flows/subflows/open-app.yaml#L10-L12(this comment)integration/tests/expo-native/flows/subflows/assert-signed-in.yaml#L1-L2
🤖 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 `@integration/tests/expo-native/flows/subflows/open-app.yaml` around lines 10 -
12, Update the retry comments in open-app.yaml (lines 10-12) and
assert-signed-in.yaml (lines 1-2) to state that Maestro polls assertVisible
until its default 7-second timeout; remove wording that implies assertVisible
never retries. No other behavior changes are needed.
Adds open-auth-view-button and sign-out-button testIDs to App.tsx so Maestro can drive the JS layer deterministically, and ad-hoc signs the SDK 57 iOS simulator build (CODE_SIGN_IDENTITY=-): unsigned builds have no application-identifier, which breaks Keychain/expo-secure-store and leaves clerk-js loading forever.
Scenario-per-file flows under integration/tests/expo-native covering native AuthView sign-in with native-to-JS session sync, session persistence across restart, AuthView detach/reattach via platform-native dismissal, and UserButton native sign-out followed by a same-process re-sign-in. Shared steps (clean launch, email+password entry, signed-in/out assertions, warmup) live in subflows/. run-flows.sh runs the suite locally via pnpm test:integration:expo-native.
…kflow Extends the existing expo-native-build matrix with run-e2e on the SDK 57 entries (other SDKs can opt in the same way), provisioning a throwaway BAPI test user after the native build, running the Maestro suite in burn-in mode (continue-on-error), cleaning the user up before uploading scrubbed debug artifacts, and deleting the dead mobile-e2e workflow.
1ffe3be to
7b34285
Compare
Which first factor follows the email step depends on instance config and SDK: clerk-ios can present the email-code screen first where clerk-android presents password first. The subflow now accepts either screen instead of hard-gating on the password field, keeping password-then-code instances working unchanged.
1 participant
Description
This PR adds E2E coverage for our
@clerk/expoSDK (finally!) using Maestro.Each flow is a single scenario, and every native interaction is followed by an assertion that the JS layer observed it, so the native-to-JS session sync (and the reverse) is what's actually under test:
sign-in.yaml: signs in through the native
AuthViewwith email + password, asserts the JS side sees the session (useAuth state and user id rendered by the fixture), then signs out from the JS side and asserts the native session is gone.session-persists-across-restart.yaml: signs in, fully kills and relaunches the app without clearing state, and asserts the session is restored from secure-store with no re-auth.
authview-detach-reattach.yaml: opens the
AuthView, dismisses it with the platform-native gesture (Android back button, iOS sheet close), reopens it, and completes a full sign-in on the remounted view.user-button-sign-out-re-sign-in.yaml: opens the
UserButtonaccount sheet, signs out natively, asserts JS observes it, then signs in again in the same app process.Cold launch and initialization are exercised implicitly.
Rather than a new workflow, this reuses the existing
expo-native-build.yml: the SDK 57 matrix entries getrun-e2e: trueand run the Maestro suite after their normal fixture build. If we want e2e on other SDK versions later, it's just a matter of adding the same flag to their matrix entries.Summary by CodeRabbit
New Features
Bug Fixes