feat: Maestro E2E test suite POC with GitHub Actions iOS CI#1911
feat: Maestro E2E test suite POC with GitHub Actions iOS CI#1911
Conversation
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
|
It looks like your comment came through empty — did you have a specific question or request about the E2E test implementation? Happy to help with any follow-up. |
|
This comment also came through empty. If you have a specific question or change request about the E2E test implementation, please share the details and I'll take care of it. |
|
@copilot build failing |
|
@coderabbitai Please review this PR for code quality, best practices, and potential issues. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11826
🔴 Critical — Will Break the CI Run1. EAS local iOS build does not produce a
|
| Severity | Issue |
|---|---|
| 🔴 Critical | EAS local build outputs .tar.gz/.ipa, not .app — install step will fail |
| 🔴 Critical | id:-based taps in trip/pack flows won't match (no testIDs in codebase) |
| 🔴 Critical | test-results/ directory not created before Maestro output |
| 🟡 High | Java setup step is unnecessary for iOS builds |
| 🟡 High | No guard for empty Python runtime/device selection output |
| 🟠 Medium | bun-version: latest and eas-version: latest should be pinned |
| 🟠 Medium | No dependency caching |
| 🟠 Medium | 60-min timeout too tight for EAS local builds |
| 🔵 Low | Redundant clearState in clear-state.yaml |
| 🔵 Low | Logout dialog confirmation race condition |
| 🔵 Low | Missing minVersion in config.yaml |
The three critical items will need to be resolved for the CI pipeline to succeed. Happy to help generate the specific diffs for any of them.
[full_review]
🧠 Learnings used
Learnt from: CR
Repo: PackRat-AI/PackRat PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-01T10:08:42.658Z
Learning: Configure required CI/CD environment variables: PACKRAT_NATIVEWIND_UI_GITHUB_TOKEN for GitHub package access, Cloudflare API tokens for API deployment, and Expo credentials for mobile builds
There was a problem hiding this comment.
Pull request overview
Adds a proof-of-concept Maestro E2E suite for the Expo mobile app and wires it into GitHub Actions to run on an iOS simulator, targeting core flows (login → create trip → create pack → logout).
Changes:
- Introduces a Maestro test suite config plus setup/auth/trips/packs flow YAMLs under
.maestro/. - Adds documentation for running Maestro locally and required CI secrets.
- Adds an iOS CI workflow that builds the app, boots a simulator, installs the app, runs Maestro, and uploads artifacts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
.maestro/config.yaml |
Defines suite flow discovery and explicit execution order. |
.maestro/flows/setup/clear-state.yaml |
Attempts to reset app state for deterministic runs. |
.maestro/flows/auth/login-flow.yaml |
Automates email/password login and asserts tab visibility. |
.maestro/flows/auth/logout-flow.yaml |
Automates logout from the Profile tab and asserts auth screen. |
.maestro/flows/trips/create-trip-flow.yaml |
Navigates to Trips and creates a trip. |
.maestro/flows/packs/create-pack-flow.yaml |
Navigates to Packs and creates a pack. |
.maestro/README.md |
Documents structure, prerequisites, and CI usage. |
.github/workflows/e2e-tests.yml |
Adds macOS iOS-simulator CI job to build/install/run Maestro and upload results/artifacts. |
You can also share your feedback on Copilot code review. Take the survey.
| - clearState | ||
| - launchApp: | ||
| clearState: true |
There was a problem hiding this comment.
clear-state.yaml runs both clearState and launchApp with clearState: true, which duplicates the same intent and increases suite time. Consider keeping just the launchApp call with clearState: true (or just clearState) to avoid redundant work.
| - scrollUntilVisible: | ||
| element: | ||
| text: "Sign Out" | ||
| direction: DOWN | ||
|
|
||
| # Tap Sign Out | ||
| - tapOn: | ||
| text: "Sign Out" | ||
|
|
There was a problem hiding this comment.
logout-flow.yaml is looking for and tapping a "Sign Out" button, but the Profile tab’s logout button text in the app is "Log Out" (t('auth.logOut')). As written, scrollUntilVisible/tapOn will never find the element on iOS, so the flow will fail. Update the flow to target the actual "Log Out" label and handle the post-logout alert (e.g., choose "Stay logged out" / "Sign-in again") before asserting the auth screen.
| - runFlow: | ||
| when: | ||
| notVisible: | ||
| text: "New Trip" | ||
| commands: | ||
| - tapOn: | ||
| id: "create-trip-button" | ||
|
|
There was a problem hiding this comment.
This flow falls back to tapOn: { id: "create-trip-button" }, but there is no matching accessibility identifier/testID in the app for the Trips header "+" button (it’s a plain <Pressable> wrapping an icon). On non-empty accounts (server already has trips), the "Create New Trip" empty-state button won’t be visible and this fallback will fail. Either add a stable accessibilityLabel/testID for the header create button in the app code and reference that here, or change the flow strategy to not depend on header UI when the list isn’t empty.
| - runFlow: | ||
| when: | ||
| notVisible: | ||
| text: "Create Pack" | ||
| commands: | ||
| - tapOn: | ||
| id: "create-pack-button" | ||
|
|
There was a problem hiding this comment.
This flow falls back to tapOn: { id: "create-pack-button" }, but there is no matching accessibility identifier/testID in the app for the Packs header "+" button (it’s a plain <Pressable> wrapping an icon). If the test account already has packs (server-side), the empty-state "Create New Pack" button won’t appear and this fallback will fail. Either add a stable accessibilityLabel/testID in the app code for the header create button and reference it here, or adjust the flow to a selector that exists regardless of list emptiness.
| # Submit the form | ||
| - tapOn: | ||
| text: "Create Pack" | ||
|
|
||
| - waitForAnimationToEnd |
There was a problem hiding this comment.
On the pack creation screen, the navigation title is also "Create Pack" (from the Expo Router Stack options), so tapOn: { text: "Create Pack" } can match the header title instead of the submit button, making the test flaky. Prefer a more specific selector (e.g., target the submit button via an accessibility id/label) so the tap reliably hits the form submit control.
| - name: Build iOS app for simulator | ||
| run: | | ||
| cd apps/expo | ||
| eas build \ | ||
| --platform ios \ | ||
| --profile preview \ | ||
| --non-interactive \ | ||
| --local \ | ||
| --output ./build/PackRat.app | ||
| env: |
There was a problem hiding this comment.
The workflow builds with --profile preview but apps/expo/eas.json does not configure an iOS simulator build (no ios.simulator: true in that profile). If EAS outputs an .ipa (device build), xcrun simctl install will fail because it expects a .app. Consider adding a dedicated EAS build profile for simulator builds and using it here (or adjust the workflow to handle the produced artifact type).
| DEVICE_ID=$(xcrun simctl create "PackRat-E2E" "$DEVICE_TYPE" "$IOS_RUNTIME") | ||
| xcrun simctl boot "$DEVICE_ID" | ||
| echo "SIMULATOR_UDID=$DEVICE_ID" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Install app on simulator | ||
| run: | | ||
| xcrun simctl install "$SIMULATOR_UDID" apps/expo/build/PackRat.app | ||
|
|
There was a problem hiding this comment.
After xcrun simctl boot, the workflow immediately installs the app. simctl boot is not guaranteed to wait until the simulator is fully booted, which can cause intermittent install / Maestro startup failures. Add an explicit wait step (e.g., xcrun simctl bootstatus "$DEVICE_ID" -b) before installing/running tests.
.github/workflows/e2e-tests.yml
Outdated
| # Find the latest available iOS 17+ runtime | ||
| IOS_RUNTIME=$(xcrun simctl list runtimes --json \ | ||
| | python3 -c " | ||
| import sys, json | ||
| runtimes = json.load(sys.stdin)['runtimes'] | ||
| ios17 = [r for r in runtimes if r.get('isAvailable') and 'iOS 17' in r.get('name','')] | ||
| ios18 = [r for r in runtimes if r.get('isAvailable') and 'iOS 18' in r.get('name','')] | ||
| chosen = (ios18 or ios17) | ||
| print(chosen[-1]['identifier'] if chosen else '') | ||
| ") |
There was a problem hiding this comment.
IOS_RUNTIME can end up empty if the runner image doesn’t include an iOS 17/18 runtime matching the name filter. In that case simctl create will fail with a hard-to-debug error. Add a guard that errors with a clear message when IOS_RUNTIME is empty, or broaden the selection logic to fall back to the newest available iOS runtime regardless of major version.
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Fixed in c095adf. Root cause: The Fix:
|
|
@coderabbitai Copilot has pushed updates addressing your previous feedback. Please re-review this PR for any remaining issues or approve if it looks good. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me fetch the current state of all relevant files. 🏁 Script executed: Length of output: 9724
✅ Resolved
🔴 Still Critical1.
|
| Severity | Issue | Status |
|---|---|---|
| 🔴 Critical | test-results/ dir not created |
Unresolved |
| 🔴 Critical | id: fallback taps still present |
Unresolved |
| 🟡 High | No bash guard on empty runtime/device vars | Unresolved |
| 🟡 High | Unnecessary Java 17 setup | Unresolved |
| 🟠 Medium | APP_PATH stored as relative path |
New |
| 🟠 Medium | bun-version/eas-version unpinned |
Unresolved |
| 🟠 Medium | No dependency caching | Unresolved |
| 🟠 Medium | 60-min timeout too tight | Unresolved |
| ✅ Resolved | EAS .tar.gz extraction with APP_PATH validation |
Fixed |
| ✅ Resolved | eas.json e2e simulator profile |
Fixed |
| ✅ Resolved | Text-based taps as primary path in trip/pack flows | Fixed |
The test-results/ directory creation and the runtime selection empty-string guards are the two most likely to cause an immediate CI failure — I'd prioritize those along with removing the Java step. Happy to generate the full diffs for any of these.
🧠 Learnings used
Learnt from: CR
Repo: PackRat-AI/PackRat PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-01T10:08:42.658Z
Learning: Configure required CI/CD environment variables: PACKRAT_NATIVEWIND_UI_GITHUB_TOKEN for GitHub package access, Cloudflare API tokens for API deployment, and Expo credentials for mobile builds
|
@copilot CodeRabbit has reviewed this PR and provided feedback. Please address the review comments and push updates. Once you've made the changes, the monitor will ask CodeRabbit to re-review. |
|
@coderabbitai Copilot has pushed updates. Please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8946
All 8 previously flagged items remain unresolved: 🟡 Still High1. Java 17 setup is still presentNot needed for EAS local iOS simulator builds. ~2 min of overhead per run with no benefit. Remove the entire step: # Delete entirely:
- name: Setup Java
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: "17"🟠 Still Medium2.
|
| Severity | Issue | Status |
|---|---|---|
| 🟡 High | Unnecessary Java 17 setup | Unresolved |
| 🟠 Medium | APP_PATH stored as relative path |
Unresolved |
| 🟠 Medium | bun-version/eas-version unpinned |
Unresolved |
| 🟠 Medium | No dependency caching | Unresolved |
| 🟠 Medium | timeout-minutes: 60 too tight for EAS local builds |
Unresolved |
| 🔵 Low | Redundant TEST_EMAIL/TEST_PASSWORD in Maestro step env: |
Unresolved |
| 🔵 Low | Logout dialog confirmation race condition | Unresolved |
| 🔵 Low | Missing minVersion: "1.40.0" in config.yaml |
Unresolved |
| ✅ | Trip/pack flows simplified to id:-only selectors |
Improved |
| ✅ | mkdir -p test-results added |
Fixed |
| ✅ | DEVICE_TYPE empty-string guard via fallback |
Fixed |
| ✅ | testIDs in app; id: taps valid |
Fixed |
| ✅ | EAS .tar.gz extraction with validation |
Fixed |
| ✅ | eas.json e2e simulator profile |
Fixed |
| ✅ | Bash guard for empty IOS_RUNTIME |
Fixed |
| ✅ | Redundant clearState removed |
Fixed |
There are still no CI-breaking issues — all critical blockers are resolved. The remaining 8 items are reliability and best-practice improvements. If useful, I can provide a single consolidated diff for all of them.
|
@coderabbitai Copilot has pushed updates. Please re-review. |
|
@copilot The |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8946
No Changes Detected
As a convenience, here is the complete consolidated diff for all 8 remaining items — this can be applied in a single commit:
# 1. Increase timeout (job level)
timeout-minutes: 90 # was: 60
# 2. Pin bun version
- uses: oven-sh/setup-bun@v1
with:
bun-version: "1.1.38" # was: latest
# 3. Add bun cache — insert before "Install dependencies"
- name: Cache bun dependencies
uses: actions/cache@v4
with:
path: ~/.bun/install/cache
key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }}
restore-keys: ${{ runner.os }}-bun-
# 4. Pin eas version
- uses: expo/expo-github-action@v8
with:
eas-version: "14.x" # was: latest
token: ${{ secrets.EXPO_TOKEN }}
# 5. Remove the entire Java setup step (not needed for iOS simulator builds):
# - name: Setup Java
# uses: actions/setup-java@v4
# with:
# distribution: temurin
# java-version: "17"
# 6. Use absolute APP_PATH in the "Extract iOS simulator build" step:
APP_PATH=$(find "$GITHUB_WORKSPACE/apps/expo/build/extracted" -name "*.app" -maxdepth 3 | head -1)
# 7. Remove redundant TEST_EMAIL/TEST_PASSWORD from Maestro step env:
- name: Run Maestro E2E tests
run: |
mkdir -p test-results
maestro --device "$SIMULATOR_UDID" test \
--format junit \
--output test-results/maestro-results.xml \
.maestro/config.yaml
env:
# TEST_EMAIL and TEST_PASSWORD removed — already set at job level
TRIP_NAME: E2E-Trip-${{ github.run_id }}
PACK_NAME: E2E-Pack-${{ github.run_id }}
# Add to the top of the file
minVersion: "1.40.0"
flows:
- flows/setup/*.yaml
...
# Add waitForAnimationToEnd inside the conditional to prevent re-matching the original button:
- runFlow:
when:
visible:
text: "Log Out"
commands:
- waitForAnimationToEnd # ← add this guard
- tapOn:
text: "Log Out" |
…creens, use stable ID selectors in login-flow Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Fixed in CI failure root cause: The app uses CodeRabbit login-flow feedback addressed:
The other 9 CodeRabbit threads (fork guard, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.maestro/README.md (1)
35-38:⚠️ Potential issue | 🟡 MinorMake the local extraction step runnable as written.
tar -C ./build/extractedwill fail if that directory does not already exist, so these commands still won't reproduce CI from a clean checkout.Suggested doc fix
- - Extract the archive: `tar -xzf ./build/PackRat-sim.tar.gz -C ./build/extracted` - - Install on simulator: `xcrun simctl install booted <path-to-extracted.app>` + - Extract the archive: `mkdir -p ./build/extracted && tar -xzf ./build/PackRat-sim.tar.gz -C ./build/extracted` + - Install on simulator: `xcrun simctl install booted "$(find ./build/extracted -name "*.app" | head -1)"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.maestro/README.md around lines 35 - 38, The extraction step in the README will fail on a clean checkout because the target directory may not exist; update the instructions around the tar extraction (the lines showing `tar -xzf ./build/PackRat-sim.tar.gz -C ./build/extracted` and the preceding build command) to create the destination directory first (e.g., add a step to run `mkdir -p ./build/extracted` before the tar extraction) so the `tar -xzf ... -C ./build/extracted` command is runnable as written.
🧹 Nitpick comments (1)
.github/workflows/e2e-tests.yml (1)
40-54: Pin Bun and EAS instead of trackinglatest.Both setup steps currently change behavior whenever a new release lands, which makes this already-expensive CI job harder to reproduce and debug when it flakes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-tests.yml around lines 40 - 54, CI workflow currently pins actions to floating "latest" for bun-version and eas-version in the "Setup Bun" and "Setup Expo" steps; replace those floating values with explicit, tested versions (e.g., set the bun-version input to a specific release tag like "0.8.8" or whatever you validated and set eas-version to a specific EAS CLI version) and update the action references if needed (oven-sh/setup-bun@v1 and expo/expo-github-action@v8) so the job uses fixed, reproducible versions; ensure to update the workflow commit message and optionally add a short comment explaining why the pins exist for future updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-tests.yml:
- Around line 27-28: The workflow's job timeout is too low (timeout-minutes: 60)
given it runs EAS iOS builds, simulator boot, and Maestro; update the
timeout-minutes value in the e2e tests workflow (the timeout-minutes setting
under the job that uses runs-on: macos-15) to a larger value (e.g., 120 or 180)
to provide sufficient headroom and prevent spurious cancellations.
In @.maestro/README.md:
- Around line 64-66: Update the README bullet that currently says "Every pull
request to `main` or `development`" to match the workflow's path filters: change
it to state that pull requests to `main` or `development` will run the job only
when they touch `apps/expo/**` or `.maestro/**` (so the three bullets read
consistently with the `.github/workflows/e2e-tests.yml` filters).
---
Duplicate comments:
In @.maestro/README.md:
- Around line 35-38: The extraction step in the README will fail on a clean
checkout because the target directory may not exist; update the instructions
around the tar extraction (the lines showing `tar -xzf
./build/PackRat-sim.tar.gz -C ./build/extracted` and the preceding build
command) to create the destination directory first (e.g., add a step to run
`mkdir -p ./build/extracted` before the tar extraction) so the `tar -xzf ... -C
./build/extracted` command is runnable as written.
---
Nitpick comments:
In @.github/workflows/e2e-tests.yml:
- Around line 40-54: CI workflow currently pins actions to floating "latest" for
bun-version and eas-version in the "Setup Bun" and "Setup Expo" steps; replace
those floating values with explicit, tested versions (e.g., set the bun-version
input to a specific release tag like "0.8.8" or whatever you validated and set
eas-version to a specific EAS CLI version) and update the action references if
needed (oven-sh/setup-bun@v1 and expo/expo-github-action@v8) so the job uses
fixed, reproducible versions; ensure to update the workflow commit message and
optionally add a short comment explaining why the pins exist for future updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f24d27a-d68b-4b70-b70d-dbf2f193e2e8
📒 Files selected for processing (10)
.github/workflows/e2e-tests.yml.maestro/README.md.maestro/flows/auth/login-flow.yaml.maestro/flows/auth/logout-flow.yaml.maestro/flows/packs/create-pack-flow.yaml.maestro/flows/trips/create-trip-flow.yamlapps/expo/app/auth/(login)/index.tsxapps/expo/app/auth/index.tsxapps/expo/features/packs/screens/PackListScreen.tsxapps/expo/features/trips/screens/TripListScreen.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/expo/features/packs/screens/PackListScreen.tsx
- .maestro/flows/packs/create-pack-flow.yaml
- apps/expo/features/trips/screens/TripListScreen.tsx
- .maestro/flows/trips/create-trip-flow.yaml
- .maestro/flows/auth/logout-flow.yaml
- .maestro/flows/auth/login-flow.yaml
|
@coderabbitai Copilot has pushed updates. Please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8946
No Changes Detected
There are still no CI-breaking issues — all critical blockers remain fixed. For Copilot's reference, here is the complete consolidated diff to address all 8 remaining items in one commit:
# 1. Increase timeout
timeout-minutes: 90 # was: 60
# 2. Pin bun version
- uses: oven-sh/setup-bun@v1
with:
bun-version: "1.1.38" # was: latest
# 3. Add bun cache — insert before "Install dependencies"
- name: Cache bun dependencies
uses: actions/cache@v4
with:
path: ~/.bun/install/cache
key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }}
restore-keys: ${{ runner.os }}-bun-
# 4. Pin eas version
- uses: expo/expo-github-action@v8
with:
eas-version: "14.x" # was: latest
token: ${{ secrets.EXPO_TOKEN }}
# 5. Remove the entire Java setup step:
# - name: Setup Java
# uses: actions/setup-java@v4
# with:
# distribution: temurin
# java-version: "17"
# 6. Use absolute APP_PATH in the "Extract iOS simulator build" step:
APP_PATH=$(find "$GITHUB_WORKSPACE/apps/expo/build/extracted" -name "*.app" -maxdepth 3 | head -1)
# 7. Remove TEST_EMAIL / TEST_PASSWORD from Maestro step env: (already set at job level)
- name: Run Maestro E2E tests
run: |
mkdir -p test-results
maestro --device "$SIMULATOR_UDID" test \
--format junit \
--output test-results/maestro-results.xml \
.maestro/config.yaml
env:
TRIP_NAME: E2E-Trip-${{ github.run_id }}
PACK_NAME: E2E-Pack-${{ github.run_id }}
minVersion: "1.40.0" # add this line at the top
flows:
- flows/setup/*.yaml
...
- runFlow:
when:
visible:
text: "Log Out"
commands:
- waitForAnimationToEnd # add this guard before the tap
- tapOn:
text: "Log Out" |
|
@copilot CodeRabbit re-review still has 13 inline comments remaining. Please continue addressing the review feedback. |
|
👋 @copilot |
|
@coderabbitai Copilot has pushed updates. Please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8946
No Changes Detected
There are still no CI-breaking issues — all critical blockers remain fixed. Here is the complete consolidated diff for all 8 remaining items, ready to apply in one commit:
# 1. Increase timeout
timeout-minutes: 90 # was: 60
# 2. Pin bun version
- uses: oven-sh/setup-bun@v1
with:
bun-version: "1.1.38" # was: latest
# 3. Add bun cache — insert before "Install dependencies"
- name: Cache bun dependencies
uses: actions/cache@v4
with:
path: ~/.bun/install/cache
key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }}
restore-keys: ${{ runner.os }}-bun-
# 4. Pin eas version
- uses: expo/expo-github-action@v8
with:
eas-version: "14.x" # was: latest
token: ${{ secrets.EXPO_TOKEN }}
# 5. Remove the entire Java setup step (not needed for iOS simulator builds):
# - name: Setup Java
# uses: actions/setup-java@v4
# with:
# distribution: temurin
# java-version: "17"
# 6. Use absolute APP_PATH in the "Extract iOS simulator build" step:
APP_PATH=$(find "$GITHUB_WORKSPACE/apps/expo/build/extracted" -name "*.app" -maxdepth 3 | head -1)
# 7. Remove TEST_EMAIL / TEST_PASSWORD from Maestro step env (already set at job level):
- name: Run Maestro E2E tests
run: |
mkdir -p test-results
maestro --device "$SIMULATOR_UDID" test \
--format junit \
--output test-results/maestro-results.xml \
.maestro/config.yaml
env:
# TEST_EMAIL and TEST_PASSWORD removed — already set at job level
TRIP_NAME: E2E-Trip-${{ github.run_id }}
PACK_NAME: E2E-Pack-${{ github.run_id }}
minVersion: "1.40.0" # add this at the top
flows:
- flows/setup/*.yaml
...
- runFlow:
when:
visible:
text: "Log Out"
commands:
- waitForAnimationToEnd # add this guard before the tap
- tapOn:
text: "Log Out" |
… sync condition, README trigger wording Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
|
@copilot There are still 13 unresolved CodeRabbit inline comments. Please address the remaining review feedback. |
|
@coderabbitai Copilot has pushed updates. Please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 10653
✅ Newly Resolved
🟡 Still High1. Java 17 setup is still presentThe # Remove this entire step:
- name: Setup Java
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: "17"🟠 Still Medium2.
|
| Severity | Issue | Status |
|---|---|---|
| 🟡 High | Unnecessary Java 17 setup | Unresolved |
| 🟠 Medium | APP_PATH stored as relative path |
Unresolved |
| 🟠 Medium | bun-version: latest unpinned |
Unresolved |
| 🟠 Medium | eas-version: latest unpinned |
Unresolved |
| 🟠 Medium | No actions/cache@v4 for bun dependencies |
Unresolved |
| 🔵 Low | Redundant TEST_EMAIL/TEST_PASSWORD in Maestro step env: |
Unresolved |
| 🔵 Low | Missing minVersion: "1.40.0" in config.yaml |
Unresolved |
| ✅ | timeout-minutes increased to 90 |
Fixed |
| ✅ | Logout race condition resolved ("Sync in progress" trigger) |
Fixed |
| ✅ | mkdir -p test-results added |
Fixed |
| ✅ | DEVICE_TYPE empty-string guard via fallback |
Fixed |
| ✅ | testIDs in app; id: taps valid |
Fixed |
| ✅ | EAS .tar.gz extraction with validation |
Fixed |
| ✅ | eas.json e2e simulator profile |
Fixed |
| ✅ | Bash guard for empty IOS_RUNTIME |
Fixed |
| ✅ | Redundant clearState removed |
Fixed |
There are still no CI-breaking issues — all critical blockers remain fixed. The 7 remaining items are reliability and best-practice improvements. The Java step removal is the highest-impact remaining change.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.maestro/flows/auth/logout-flow.yaml (1)
25-31:⚠️ Potential issue | 🟠 MajorScope the confirmation tap to the dialog action.
Line 30 still has the same ambiguity: the
when.visible.text: "Sync in progress"guard controls when this block runs, buttapOn: { text: "Log Out" }can still match the underlying settings row instead of the dialog button. Please switch this step to a dialog-specific selector or a dedicated test/accessibility id for the confirm action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.maestro/flows/auth/logout-flow.yaml around lines 25 - 31, The tap step in runFlow is ambiguous because when.visible.text: "Sync in progress" only gates execution but tapOn: { text: "Log Out" } can match the underlying settings row; update the logout confirmation step in logout-flow.yaml to target the dialog action specifically by replacing the generic tapOn text selector with a dialog-scoped selector or a dedicated accessibility id (e.g., use the dialog button selector or tapOn with the confirm button's accessibilityId like "logout-confirm") so the tap targets the confirmation button in the dialog rather than the settings row..maestro/README.md (1)
35-38:⚠️ Potential issue | 🟡 MinorCreate the extraction directory and show the resolved
.apppath.Line 37 will fail on a clean machine because
tar -C ./build/extractedexpects that directory to already exist. The install step is also still a placeholder, so local setup does not quite mirror the CI flow yet.Suggested doc fix
- - Extract the archive: `tar -xzf ./build/PackRat-sim.tar.gz -C ./build/extracted` - - Install on simulator: `xcrun simctl install booted <path-to-extracted.app>` + - Extract the archive: `mkdir -p ./build/extracted && tar -xzf ./build/PackRat-sim.tar.gz -C ./build/extracted` + - Install on simulator: `xcrun simctl install booted "$(find ./build/extracted -name "*.app" -type d | head -1)"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.maestro/README.md around lines 35 - 38, The README's extraction and install steps assume ./build/extracted exists and use a placeholder path; before running tar -xzf ./build/PackRat-sim.tar.gz -C ./build/extracted add a mkdir -p ./build/extracted to create the directory, and replace the placeholder <path-to-extracted.app> in the xcrun simctl install command by resolving and echoing the actual .app path (e.g., find ./build/extracted -name "*.app" -print -quit) so the documented install step uses the real extracted .app file instead of a placeholder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.maestro/README.md:
- Around line 7-20: The fenced code block showing the directory tree (the block
that begins with ``` and contains the .maestro/ tree) needs a language tag to
satisfy markdownlint; update that opening fence to include a tag such as "text"
or "plaintext" (e.g., change ``` to ```text) so the README's tree block is
lint-clean while preserving the existing content.
---
Duplicate comments:
In @.maestro/flows/auth/logout-flow.yaml:
- Around line 25-31: The tap step in runFlow is ambiguous because
when.visible.text: "Sync in progress" only gates execution but tapOn: { text:
"Log Out" } can match the underlying settings row; update the logout
confirmation step in logout-flow.yaml to target the dialog action specifically
by replacing the generic tapOn text selector with a dialog-scoped selector or a
dedicated accessibility id (e.g., use the dialog button selector or tapOn with
the confirm button's accessibilityId like "logout-confirm") so the tap targets
the confirmation button in the dialog rather than the settings row.
In @.maestro/README.md:
- Around line 35-38: The README's extraction and install steps assume
./build/extracted exists and use a placeholder path; before running tar -xzf
./build/PackRat-sim.tar.gz -C ./build/extracted add a mkdir -p ./build/extracted
to create the directory, and replace the placeholder <path-to-extracted.app> in
the xcrun simctl install command by resolving and echoing the actual .app path
(e.g., find ./build/extracted -name "*.app" -print -quit) so the documented
install step uses the real extracted .app file instead of a placeholder.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42828add-8b06-4580-a8c0-6a76a592b876
📒 Files selected for processing (3)
.github/workflows/e2e-tests.yml.maestro/README.md.maestro/flows/auth/logout-flow.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e-tests.yml
| ``` | ||
| .maestro/ | ||
| ├── config.yaml # Maestro suite configuration & flow order | ||
| └── flows/ | ||
| ├── setup/ | ||
| │ └── clear-state.yaml # Clears app state before test suite | ||
| ├── auth/ | ||
| │ ├── login-flow.yaml # Sign in with email and password | ||
| │ └── logout-flow.yaml # Sign out from the app | ||
| ├── trips/ | ||
| │ └── create-trip-flow.yaml # Create a new trip | ||
| └── packs/ | ||
| └── create-pack-flow.yaml # Create a new pack | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced structure block.
markdownlint is flagging this block already. Mark it as text/plaintext so the README stays lint-clean.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.maestro/README.md around lines 7 - 20, The fenced code block showing the
directory tree (the block that begins with ``` and contains the .maestro/ tree)
needs a language tag to satisfy markdownlint; update that opening fence to
include a tag such as "text" or "plaintext" (e.g., change ``` to ```text) so the
README's tree block is lint-clean while preserving the existing content.
|
@coderabbitai Copilot has pushed updates. Please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 10653
No Changes Detected
There are still no CI-breaking issues — all critical blockers remain fixed. Here is the complete consolidated diff for all 7 remaining items, ready to apply in one commit:
# 1. Pin bun version
- uses: oven-sh/setup-bun@v1
with:
bun-version: "1.1.38" # was: latest
# 2. Add bun cache — insert before "Install dependencies"
- name: Cache bun dependencies
uses: actions/cache@v4
with:
path: ~/.bun/install/cache
key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }}
restore-keys: ${{ runner.os }}-bun-
# 3. Pin eas version
- uses: expo/expo-github-action@v8
with:
eas-version: "14.x" # was: latest
token: ${{ secrets.EXPO_TOKEN }}
# 4. Remove the entire Java setup step (not needed for iOS simulator builds):
# - name: Setup Java
# uses: actions/setup-java@v4
# with:
# distribution: temurin
# java-version: "17"
# 5. Use absolute APP_PATH in the "Extract iOS simulator build" step:
APP_PATH=$(find "$GITHUB_WORKSPACE/apps/expo/build/extracted" -name "*.app" -maxdepth 3 | head -1)
# 6. Remove TEST_EMAIL / TEST_PASSWORD from Maestro step env (already set at job level):
- name: Run Maestro E2E tests
run: |
mkdir -p test-results
maestro --device "$SIMULATOR_UDID" test \
--format junit \
--output test-results/maestro-results.xml \
.maestro/config.yaml
env:
# TEST_EMAIL and TEST_PASSWORD removed — already set at job level
TRIP_NAME: E2E-Trip-${{ github.run_id }}
PACK_NAME: E2E-Pack-${{ github.run_id }}
# 7. Add minVersion at the top of the file
minVersion: "1.40.0"
flows:
- flows/setup/*.yaml
- flows/auth/*.yaml
- flows/trips/*.yaml
- flows/packs/*.yaml |
|
@copilot CodeRabbit re-reviewed and found 14 inline comments remaining. Please address all outstanding review feedback. |
e2esimulator build profile toeas.jsone2e-tests.ymlto use--profile e2e, extract.tar.gz, validate.apppathPACKRAT_NATIVEWIND_UI_GITHUB_TOKENto EAS local build stepmacos-14→macos-15(Xcode 16)testIDselectors to auth screens; updatelogin-flow.yamlto useid:selectorse2e-tests.yml: Increasetimeout-minutes60 → 90 to avoid spurious cancellations on slow runnerslogout-flow.yaml: Fix ambiguous sync-conflict conditionwhen.visible: "Log Out"→when.visible: "Sync in progress"(matchest('profile.syncInProgress'))README.md: Tighten PR trigger description to accurately reflect the path-filter in the workflowOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit
Tests
Documentation
Chores