chore(e2e): Cache app builds. Skip E2E on docs only PR.#9930
Open
yasserfaraazkhan wants to merge 11 commits into
Open
chore(e2e): Cache app builds. Skip E2E on docs only PR.#9930yasserfaraazkhan wants to merge 11 commits into
yasserfaraazkhan wants to merge 11 commits into
Conversation
Reduce E2E cost by path-filtering Matterwick label refresh, caching iOS/Android binaries in S3 keyed by build-impacting content, adopting preboot_ios_simulator, and seeding caches nightly from main. Co-authored-by: Cursor <cursoragent@cursor.com>
CMT runs against a freshly-cut build-release-NNN branch, so its app content never matches main's fingerprint and the S3 build cache would always miss. Revert CMT to its native pattern: build each app once per run and reuse across every server-version leg of the matrix via run_id artifacts. Removes the fingerprint job, per-build cache steps, runner bump, and top-level AWS bucket env from CMT. Nightly prune comment updated to reflect that e2e-builds/ is shared only by PR builds and the nightly seed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
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:
📝 WalkthroughWalkthroughAdds fingerprinted S3 caching for iOS and Android E2E builds, nightly cache maintenance, updated Ruby dependency caching, delegated iOS simulator prebooting, app-impact-based E2E label handling, and Ubuntu 24.04 runners. ChangesE2E build caching
E2E workflow maintenance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant FingerprintAction
participant S3BuildCache
participant S3
participant E2EBuild
Workflow->>FingerprintAction: compute platform fingerprint
FingerprintAction-->>Workflow: return fingerprint
Workflow->>S3BuildCache: restore cache
S3BuildCache->>S3: check and download artifact
S3-->>S3BuildCache: return cache result
S3BuildCache-->>Workflow: return cache-hit
Workflow->>E2EBuild: build when cache misses
E2EBuild->>S3BuildCache: save build artifact
S3BuildCache->>S3: upload fingerprinted artifact
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.github/workflows/e2e-nightly-cache-seed.yml (2)
6-9: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a minimal
permissions:block.No
permissions:block exists anywhere in this workflow, so all jobs run with the default (potentially broad) token permissions. Since every job here only reads the repo and writes to S3 via AWS credentials (never to the GitHub repo), a top-levelpermissions: contents: readwould tighten this per least-privilege.on: schedule: - cron: '0 2 * * *' # 2 AM UTC daily workflow_dispatch: +permissions: + contents: read + jobs:🤖 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/e2e-nightly-cache-seed.yml around lines 6 - 9, Add a top-level permissions block to the workflow containing only contents: read, alongside the existing on configuration. Keep the scheduled and workflow_dispatch triggers unchanged, and do not grant write permissions because repository writes are not required.Source: Linters/SAST tools
12-14: 🧹 Nitpick | 🔵 TrivialNo
timeout-minuteson long-running seed jobs.
build-ios/build-android-detox/build-android-maestrorun full native builds without an explicit timeout, defaulting to GitHub's 360-minute job timeout. A hang (e.g., simulator/gradle daemon stuck) would tie up an expensive runner for hours before failing.Also applies to: 71-77, 130-136
🤖 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/e2e-nightly-cache-seed.yml around lines 12 - 14, Add an explicit timeout-minutes value to the build-ios, build-android-detox, and build-android-maestro jobs in the nightly cache-seed workflow, using an appropriate limit for their native build duration so hung jobs terminate well before GitHub’s default timeout..github/actions/compute-e2e-build-fingerprint/action.yml (1)
25-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winManually maintained path lists are inherently fragile.
The comment at lines 21-24 explicitly acknowledges the risk of missing a path. For React Native projects,
@expo/fingerprintprovides a maintained, dependency-aware alternative to hand-rolled path lists (works with bare RN, not just Expo-managed projects). Worth evaluating if it fits, though not blocking.🤖 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/actions/compute-e2e-build-fingerprint/action.yml around lines 25 - 58, Evaluate replacing the manually maintained platform-specific path arrays in the fingerprint action with `@expo/fingerprint`’s dependency-aware fingerprinting, ensuring it supports this bare React Native project and preserves distinct iOS, Android Detox, and Android Maestro inputs. If adopted, remove the duplicated path lists and integrate the generated fingerprint into the existing action output flow.
🤖 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/actions/s3-build-cache/action.yml:
- Around line 59-75: Update the restore branch around the aws s3 cp and tar xzf
commands so any download, extraction, or cleanup failure is handled as a cache
miss rather than aborting the action under set -eu. Ensure the failed restore
emits cache-hit=false and allows the build to continue, while preserving the
existing successful restore behavior and check-mode existence-only path.
In @.github/workflows/e2e-detox-pr.yml:
- Around line 75-77: Add persist-credentials: false to the checkout step for the
compute-build-fingerprints job, matching the other updated checkout steps while
preserving its existing MOBILE_VERSION ref.
In @.github/workflows/e2e-label-manager.yml:
- Around line 91-97: Update the impact filter in the E2E label-manager workflow
to include the .github/actions/prepare-mobile-build/** path alongside the
existing mobile build action paths, ensuring changes to that action trigger E2E
impact handling.
In @.github/workflows/e2e-nightly-cache-seed.yml:
- Around line 18-20: Update the checkout steps in the build-ios,
build-android-detox, and build-android-maestro jobs to set persist-credentials
to false alongside ref: main, ensuring none of these S3-only jobs retain
repository credentials.
---
Nitpick comments:
In @.github/actions/compute-e2e-build-fingerprint/action.yml:
- Around line 25-58: Evaluate replacing the manually maintained
platform-specific path arrays in the fingerprint action with `@expo/fingerprint`’s
dependency-aware fingerprinting, ensuring it supports this bare React Native
project and preserves distinct iOS, Android Detox, and Android Maestro inputs.
If adopted, remove the duplicated path lists and integrate the generated
fingerprint into the existing action output flow.
In @.github/workflows/e2e-nightly-cache-seed.yml:
- Around line 6-9: Add a top-level permissions block to the workflow containing
only contents: read, alongside the existing on configuration. Keep the scheduled
and workflow_dispatch triggers unchanged, and do not grant write permissions
because repository writes are not required.
- Around line 12-14: Add an explicit timeout-minutes value to the build-ios,
build-android-detox, and build-android-maestro jobs in the nightly cache-seed
workflow, using an appropriate limit for their native build duration so hung
jobs terminate well before GitHub’s default timeout.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77d914ba-3222-4379-a3e6-7e8cddaad306
📒 Files selected for processing (14)
.github/actions/compute-e2e-build-fingerprint/action.yml.github/actions/prepare-ios-build/action.yaml.github/actions/prepare-mobile-build/action.yaml.github/actions/s3-build-cache/action.yml.github/workflows/cmt-provisioner.yml.github/workflows/compatibility-matrix-testing.yml.github/workflows/e2e-android-template.yml.github/workflows/e2e-cancel-on-label-removal.yml.github/workflows/e2e-detox-pr.yml.github/workflows/e2e-detox.yml.github/workflows/e2e-ios-template.yml.github/workflows/e2e-label-manager.yml.github/workflows/e2e-maestro-pr.yml.github/workflows/e2e-nightly-cache-seed.yml
Coverage Comparison Report |
Enterprise allowlist blocks dorny/paths-filter, causing E2E label manager startup_failure so E2E/Run is never added. Use allowlisted actions/github-script + pulls.listFiles with the same path globs. Co-authored-by: Cursor <cursoragent@cursor.com>
mattermost-build
added
E2E/Run
setup-ruby looks for .ruby-version under working-directory; with fastlane/ that file is missing because it lives at the repo root. Co-authored-by: Cursor <cursoragent@cursor.com>
mattermost-build
added
the
E2E/Run
yasserfaraazkhan
removed
the
E2E/Run
Skipping prewarm (copied from Maestro) left pending-install launchd state and caused mass iPhone Detox failures; mic/camera privacy grants diverged from proven main Detox setup on iOS 26.x. Co-authored-by: Cursor <cursoragent@cursor.com>
mattermost-build
added
the
E2E/Run
Contributor
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 `@detox/scripts/preboot_ios_simulator.sh`:
- Around line 107-117: Update kill_app_via_launchd() to track whether the bundle
disappears from launchctl list during the retry loop; only log successful
termination and return zero when cleanup completes, otherwise return non-zero
after the retries so the preboot flow stops treating a timeout as success.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5578d26b-7114-4c84-8b44-1382f0583b1c
📒 Files selected for processing (2)
.github/workflows/e2e-ios-template.ymldetox/scripts/preboot_ios_simulator.sh
Comment on lines
+107
to
+117
| if [ -z "$app_pid" ]; then | ||
| return 0 | ||
| fi | ||
| xcrun simctl spawn "$SIMULATOR_ID" kill -9 "$app_pid" 2>/dev/null || true | ||
| for _ in 1 2 3 4; do | ||
| if ! xcrun simctl spawn "$SIMULATOR_ID" launchctl list 2>/dev/null | grep -q "$BUNDLE_ID"; then | ||
| break | ||
| fi | ||
| sleep 0.25 | ||
| done | ||
| log "Killed app via launchd (PID $app_pid)" |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file shape first, then read the relevant slice.
ast-grep outline detox/scripts/preboot_ios_simulator.sh --view expanded || true
wc -l detox/scripts/preboot_ios_simulator.sh
sed -n '1,180p' detox/scripts/preboot_ios_simulator.shRepository: mattermost/mattermost-mobile
Length of output: 7387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect call sites and the remainder of the script around the helper.
rg -n "kill_app_via_launchd|prewarm_app|Killed app via launchd|launchctl list" detox/scripts/preboot_ios_simulator.sh
sed -n '120,260p' detox/scripts/preboot_ios_simulator.shRepository: mattermost/mattermost-mobile
Length of output: 5322
Handle launchd cleanup timeouts
kill_app_via_launchd() still logs “Killed app” even when the bundle remains in launchctl list after the retry loop. Return non-zero on timeout so the preboot flow doesn’t continue as if cleanup succeeded.
🤖 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 `@detox/scripts/preboot_ios_simulator.sh` around lines 107 - 117, Update
kill_app_via_launchd() to track whether the bundle disappears from launchctl
list during the retry loop; only log successful termination and return zero when
cleanup completes, otherwise return non-zero after the retries so the preboot
flow stops treating a timeout as success.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
E2E currently runs on every Matterwick dispatch, including docs-only changes and re-pushes that don't affect the app.
What's Changed
dorny/paths-filter(e2e-label-manager.yml).mainnightly so matching PRs can reuse cached binaries. Old cache entries are pruned after 21 days.