ci(e2e): harden Maestro workflow for reliability#2176
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
10dc6aa to
9876038
Compare
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
There was a problem hiding this comment.
Pull request overview
Hardens the Maestro E2E GitHub Actions workflow to reduce flakiness on development pushes by improving Android build stability and ensuring a deterministic E2E login user exists in the dev database.
Changes:
- Add an API seed script + package script to idempotently create/refresh an E2E test user (email verified, password refreshed).
- Update the Maestro workflow with a secrets preflight + DB seeding step, and Android runner hardening (disk cleanup + swap).
- Increase Android Gradle/JVM memory settings for the EAS local build profile.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/api/src/db/seed-e2e-user.ts |
New seed script to create/refresh the E2E user in the dev DB. |
packages/api/package.json |
Adds db:seed:e2e-user script entrypoint for CI usage. |
apps/expo/eas.json |
Bumps Gradle/JVM heap settings and disables some Gradle concurrency for more reliable Android builds. |
.github/workflows/e2e-tests.yml |
Adds secrets preflight gating + seeds the E2E user before running Maestro; adds Android runner disk/swap hardening. |
| const existing = await db | ||
| .select({ id: schema.users.id }) | ||
| .from(schema.users) | ||
| .where(eq(schema.users.email, normalizedEmail)) | ||
| .limit(1); | ||
|
|
||
| if (existing.length > 0) { | ||
| const userId = existing[0]!.id; | ||
| await db | ||
| .update(schema.users) | ||
| .set({ passwordHash, emailVerified: true, updatedAt: new Date() }) | ||
| .where(eq(schema.users.id, userId)); | ||
| console.log(`E2E user refreshed: ${normalizedEmail} (id=${userId})`); | ||
| } else { | ||
| const [inserted] = await db | ||
| .insert(schema.users) | ||
| .values({ | ||
| email: normalizedEmail, | ||
| passwordHash, | ||
| emailVerified: true, | ||
| firstName: 'E2E', | ||
| lastName: 'Automation', | ||
| role: 'USER', | ||
| }) | ||
| .returning({ id: schema.users.id }); | ||
| console.log(`E2E user created: ${normalizedEmail} (id=${inserted?.id})`); | ||
| } |
There was a problem hiding this comment.
The script claims to be an idempotent upsert, but the current select-then-insert flow is not atomic. Because the workflow seeds from both iOS and Android jobs (and potentially from multiple concurrent runs), two processes can both see “no user” and then race to insert, causing a unique-constraint failure on users.email. Use a single INSERT ... ON CONFLICT DO UPDATE (Drizzle .onConflictDoUpdate targeting users.email) or otherwise handle the unique violation and fall back to an UPDATE to make this truly safe under concurrency.
| const isStandardPostgresUrl = (url: string) => { | ||
| try { | ||
| const u = new URL(url); | ||
| const host = u.hostname.toLowerCase(); | ||
| const isNeonTech = host === 'neon.tech' || host.endsWith('.neon.tech'); | ||
| const isNeonCom = host === 'neon.com' || host.endsWith('.neon.com'); | ||
| return u.protocol === 'postgres:' && !isNeonTech && !isNeonCom; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
isStandardPostgresUrl() only treats URLs with protocol postgres: as “standard Postgres”. Many non-Neon connection strings use postgresql:// (protocol postgresql:), which would be misclassified and routed through the Neon HTTP driver. Consider accepting both postgres: and postgresql: (or reusing the existing helper used elsewhere in src/db) to avoid surprising connection failures.
Deploying packrat-guides with
|
| Latest commit: |
d1454e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fbd1a6da.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://fix-e2e-ci-reliability.packrat-guides-6gq.pages.dev |
Deploying packrat-landing with
|
| Latest commit: |
d1454e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b213662.packrat-landing.pages.dev |
| Branch Preview URL: | https://fix-e2e-ci-reliability.packrat-landing.pages.dev |
Two chronic failure modes on `development` were: 1. iOS run wasted ~32 min building the sim app only to fail at the Maestro step when `E2E_TEST_EMAIL` / `E2E_TEST_PASSWORD` secrets were empty. Fail fast with an actionable message before any expensive work runs. 2. Android Gradle OOM during `:app:packageRelease` on `ubuntu-latest`. Free disk, add a 10G swap file, cap parallelism, and bump the heap so R8/packaging fits. Also set `_JAVA_OPTIONS` + the namespaced `ORG_GRADLE_PROJECT_*` env so the heap bump reaches Gradle even when launched indirectly by eas.
Lets us validate the Android Gradle heap/swap fix without waiting for the E2E_TEST_EMAIL/E2E_TEST_PASSWORD secrets to be populated. When secrets are missing the build runs to completion and the Maestro/ simulator/emulator steps are skipped via a step-level if guard.
Removes the dependency on a manually-created preview DB user. Adds `packages/api/src/db/seed-e2e-user.ts` which upserts the E2E account (creating if missing, refreshing password hash + emailVerified flag if present) using NEON_DEV_DATABASE_URL. The workflow now: - Hardcodes TEST_EMAIL to admin+e2e-test-automation@packratai.com - Requires only E2E_TEST_PASSWORD + NEON_DEV_DATABASE_URL secrets (the latter already exists) - Runs the seed step on both iOS and Android jobs immediately before Maestro, so the test account always matches the password CI expects
Both TEST_EMAIL and TEST_PASSWORD now come from repo secrets, and the seed script already reads its inputs purely from env vars — no hard- coded identities anywhere. Pre-flight widened to enumerate exactly which of E2E_TEST_EMAIL / E2E_TEST_PASSWORD / NEON_DEV_DATABASE_URL is missing.
Drops the duplicate bcrypt import — if the app ever swaps the hashing algorithm or salt rounds, the seed stays in sync automatically.
- seed: drop returning({id}) arg (drizzle infers); use non-! destructure
- swap: ubuntu-latest already has /swapfile mounted, use /mnt/swapfile-ci
and dd instead of fallocate (which reports 'Text file busy')
Removed NDK, Python, go, temurin-jdk, azul jdks from the prune list —
the RN Gradle build links against the Android SDK/NDK installed on
the runner ("NDK was not found under …/sdk/ndk/27.3.13750724"), and
Python is load-bearing for some RN build scripts.
Previously we downgraded the secret preflight to a warning so the Android Gradle heap/swap fix could be validated without the secrets being populated. That validation is now done (PR #2176's prior run built the APK cleanly in 35m42s) and the lenient mode produced a misleading green check: every Maestro/emulator step was skipped via if-guards but the job conclusion was still 'success'. Revert to the original behavior: hard-fail with a clear error listing which of E2E_TEST_EMAIL / E2E_TEST_PASSWORD / NEON_DEV_DATABASE_URL is missing, and drop every `has_secrets == 'true'` guard now that the job fails on the very first step when those secrets aren't set.
a20083f to
6b71914
Compare
iOS Maestro run failed with "iOS driver not ready in time" because the xcuitest install/boot on a fresh macos-15 runner takes longer than the 180s default. Set MAESTRO_DRIVER_STARTUP_TIMEOUT=600000ms.
The EAS e2e profile builds the preview variant (app.config.ts appends '.preview' to the bundle id when IS_PREVIEW). Maestro was launching com.andrewbierman.packrat but the installed app is com.andrewbierman.packrat.preview, causing clear-state to fail with: Failed to get app binary directory for bundle ... No such file or directory Matches what .maestro/README.md already documents.
Caches that were missing: - root `node_modules` on both jobs (keyed by bun.lock) - iOS CocoaPods download cache (keyed by package.json + app.config.ts) Expected savings on warm hits: - bun install: ~40s → <5s (both jobs) - pod install: ~3-5 min → <1 min (iOS) Still dominated by xcodebuild/gradle compilation which is CPU-bound on free-tier runners — can't cache those without a paid build cache service or a bigger runner.
reactivecircus/android-emulator-runner@v2.37 runs each line of the
`script:` argument as a separate `sh -c`, which breaks our backslash
line continuations — `maestro test \` was executed by itself,
parsing `\` as the flow path ("Flow path does not exist").
Keep it single-line.
Inline `maestro test` didn't pass `-e APP_ID=...` so flows saw
`${APP_ID}` → undefined → 'Unable to launch app undefined'.
e2e.sh already forwards TEST_EMAIL/TEST_PASSWORD/APP_ID/TRIP_NAME/
PACK_NAME plus date-window vars the same way iOS does.
|
@claude keep working on this |
Previously we downgraded the secret preflight to a warning so the Android Gradle heap/swap fix could be validated without the secrets being populated. That validation is now done (PR #2176's prior run built the APK cleanly in 35m42s) and the lenient mode produced a misleading green check: every Maestro/emulator step was skipped via if-guards but the job conclusion was still 'success'. Revert to the original behavior: hard-fail with a clear error listing which of E2E_TEST_EMAIL / E2E_TEST_PASSWORD / NEON_DEV_DATABASE_URL is missing, and drop every `has_secrets == 'true'` guard now that the job fails on the very first step when those secrets aren't set.
Summary
E2E Tests (Maestro) has been failing on every
developmentpush. Two root causes + one quality-of-life upgrade::app:packageReleaseonubuntu-latest. Fixed by pruning ~15 GB of unused preinstalled toolchains, adding a 10 GB swap file, bumping Gradle heap to 7 GB, and disabling the daemon + parallel workers. Heap is exported viaGRADLE_OPTS,JAVA_OPTS,_JAVA_OPTIONS, andORG_GRADLE_PROJECT_*so it reaches Gradle regardless of howeas build --locallaunches it..tsseed script (packages/api/src/db/seed-e2e-user.ts) that idempotently upserts the test user into the dev DB before Maestro runs. Reuses the app's ownhashPassword()utility so the bcrypt work factor can never drift.E2E_TEST_EMAIL/E2E_TEST_PASSWORD/NEON_DEV_DATABASE_URLis missing and skips Maestro instead of burning 30 min of simulator time.Both email and password are driven purely by repo secrets — nothing hardcoded.
Required secrets (one-time setup)
The seed will create the user on first run and refresh its password +
emailVerifiedflag on every subsequent run.Test plan
🤖 Generated with Claude Code