Uh oh!
There was an error while loading. Please reload this page.
test(expo): Improve Maestro E2E reliability and runtime - #9259
Conversation
🦋 Changeset detectedLatest commit: 5d3e40f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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.
|
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/electron@clerk/electron-passkeys@clerk/eslint-plugin@clerk/expo@clerk/expo-google-signin@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/vuecommit: |
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe native-build workflow labels E2E matrix runs, extends their timeout, validates matching React versions, removes fixture-app artifact transfers, and installs built iOS and Android applications directly from fixture outputs. Maestro flows clear keychain state, retry warmup, and record execution results. The sign-in form handles email-code verification for additional sign-in statuses, and the fixture updates React versions. A changeset records the update. Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@integration/tests/expo-native/run-flows.sh`:
- Around line 53-55: Condense the warmup comment immediately above the warmup
logic to one terse line, preserving both the cold-start/a11y-tree rationale and
the instruction to abort on repeated failure.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ea16be0f-4bb7-4e33-8f6f-c93c97ee38b5
📒 Files selected for processing (5)
.github/workflows/expo-native-build.ymlintegration/tests/expo-native/flows/subflows/_warmup.yamlintegration/tests/expo-native/flows/subflows/open-app.yamlintegration/tests/expo-native/run-android-flows.shintegration/tests/expo-native/run-flows.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/expo-native-build.yml
| # Warmup: parse the JS bundle + populate the a11y tree once so the first real | ||
| # flow doesn't flake on cold-start; force-stop afterwards so the first | ||
| # launchApp clearState doesn't race a still-foregrounded app process. | ||
| maestro test ${debug_args+"${debug_args[@]}"} flows/subflows/_warmup.yaml || true | ||
| # flow doesn't flake on cold-start. A repeated failure means the app is not | ||
| # runnable, so abort instead of timing out and retrying every real flow. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Condense the warmup comment.
Keep the rationale, but reduce this three-line block to one terse line.
Proposed fix
-# Warmup: parse the JS bundle + populate the a11y tree once so the first real-# flow doesn't flake on cold-start. A repeated failure means the app is not-# runnable, so abort instead of timing out and retrying every real flow.+# Retry warmup once; abort if the app is still not runnable.As per coding guidelines, “keep warranted comments to one terse line rather than a verbose block.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Warmup: parse the JS bundle + populate the a11y tree once so the first real | |
| # flow doesn't flake on cold-start; force-stop afterwards so the first | |
| # launchApp clearState doesn't race a still-foregrounded app process. | |
| maestro test${debug_args+"${debug_args[@]}"} flows/subflows/_warmup.yaml ||true | |
| # flow doesn't flake on cold-start. A repeated failure means the app is not | |
| # runnable, so abort instead of timing out and retrying every real flow. | |
| # Retry warmup once; abort if the app is still not runnable. |
🤖 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/run-flows.sh` around lines 53 - 55, Condense
the warmup comment immediately above the warmup logic to one terse line,
preserving both the cold-start/a11y-tree rationale and the instruction to abort
on repeated failure.
Source: Coding guidelines
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Uh oh!
There was an error while loading. Please reload this page.
Description
Improves Expo Maestro E2E reliability and caches native builds. Runtime drops from ~18m to ~11m on iOS and ~14m on Android when uncached, and ~8m/~10m on cache hits.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change