fix: disable kvm in android e2e workflow #914
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughUpdated MCP server command to use mcp-remote@latest. Generalized three specification paths to a shared folder. Revised mobile CI CocoaPods cache logic to preserve Podfile.lock and reduce destructive cleanup. Adjusted mobile E2E workflows to change emulator flags and switch app install checks to simctl get_app_container for both platforms. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as Mobile CI
participant Repo as Repo
participant Pods as CocoaPods Cache
participant Cocoapods as CocoaPods
CI->>Repo: Checkout
CI->>Pods: Check Podfile.lock presence
alt Pods dir exists and non-empty
CI->>CI: Proceed with existing Pods
else Pods missing/empty
CI->>Pods: Clean Pods dir only (preserve Podfile.lock)
CI->>Cocoapods: pod install
end
sequenceDiagram
participant E2E as Mobile E2E
participant Android as Android Emulator
participant iOS as iOS Simulator
E2E->>Android: Launch emulator (-no-accel)
E2E->>Android: get_app_container <app>
alt container exists
E2E->>E2E: Android ready
else
E2E->>E2E: Install/prepare app
end
E2E->>iOS: Boot simulator
E2E->>iOS: simctl get_app_container <bundleId> app
alt container exists
E2E->>E2E: iOS ready
else
E2E->>E2E: Install/prepare app
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Giga Summary• Disable KVM in Android emulator to resolve GitHub runner startup issues.
Quality Assessment• Improved compatibility in CI environments Quality Score: 7/10 (Threshold: 7/10) 💬 Detailed comments have been added to specific code changes. |
![giga-agent[bot]](https://avatars.githubusercontent.com/in/1202121?s=60&v=4){kind=small}
| force-avd-creation: false | ||
| emulator-options: -no-snapshot-load -no-snapshot-save -wipe-data -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none -camera-front none -skin 1080x1920 -memory 4096 -cores 2 -accel on -qemu -enable-kvm -netdelay none -netspeed full | ||
| emulator-options: -no-snapshot-load -no-snapshot-save -wipe-data -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none -camera-front none -skin 1080x1920 -memory 4096 -cores 2 -no-accel -netdelay none -netspeed full | ||
| disable-animations: true |
There was a problem hiding this comment.
Disabling hardware acceleration (-no-accel vs -accel on) may significantly impact emulator performance. Verify if this trade-off is necessary for the CI environment.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/mobile-e2e.yml (1)
121-121: Forcing -no-accel will drastically slow Android E2E; prefer conditional fallback based on KVM availabilityGood for stability on GitHub runners without KVM, but it can 2–5x test times when acceleration is available. Detect /dev/kvm and only append -no-accel when needed.
Apply this diff to allow conditional acceleration:
- emulator-options: -no-snapshot-load -no-snapshot-save -wipe-data -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none -camera-front none -skin 1080x1920 -memory 4096 -cores 2 -no-accel -netdelay none -netspeed full + emulator-options: -no-snapshot-load -no-snapshot-save -wipe-data -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none -camera-front none -skin 1080x1920 -memory 4096 -cores 2 ${{ env.EMULATOR_ACCEL }} -netdelay none -netspeed fullAdd this step before the emulator step to set EMULATOR_ACCEL:
- name: Detect emulator acceleration shell: bash run: | if [ -e /dev/kvm ]; then echo "EMULATOR_ACCEL=-accel on" >> "$GITHUB_ENV" else echo "EMULATOR_ACCEL=-no-accel" >> "$GITHUB_ENV" fi
🧹 Nitpick comments (4)
.github/workflows/mobile-e2e.yml (3)
184-186: APK path is hardcoded to app-release.apk; this can fail if release isn’t signed (common in CI)
Many React Native projects do not sign Release in CI, producing app-release-unsigned.apk. Current check will fail even if a valid Debug or unsigned APK exists. This is a frequent source of flakes.Apply this diff to locate an installable APK more robustly, preferring a signed release, then debug:
- APK_PATH="app/android/app/build/outputs/apk/release/app-release.apk" - [ -f "$APK_PATH" ] || { echo "❌ APK not found at $APK_PATH"; exit 1; } + # Prefer a signed Release APK; fall back to Debug if available + APK_PATH="$(find app/android/app/build/outputs/apk -type f \( -name 'app-release.apk' -o -name 'app-debug.apk' \) | head -1)" + if [ -z "$APK_PATH" ]; then + echo "❌ No installable APK found (looked for app-release.apk or app-debug.apk)." + echo "Available APKs:" + find app/android/app/build/outputs/apk -type f -name '*.apk' -maxdepth 3 || true + echo "Consider building assembleDebug for emulator or configuring release signing in CI." + exit 1 + fi + echo "Found APK: $APK_PATH"If your E2E relies on debug-only flags (e.g., testIDs, dev menus), consider assembling Debug for emulator runs.
216-216: ADB_TRACE=all may flood logs and leak sensitive metadata; scope it down
Full ADB tracing balloons logs and can inadvertently expose app/device details. It also slows CI.[security]
Apply this diff to scope tracing to install only (or remove entirely if not needed):
- ADB_TRACE: "all" + ADB_TRACE: "adb,install"For deeper troubleshooting, enable broader tracing temporarily on retry paths only.
478-479: Single-shot iOS readiness probe is flaky; add a bounded retry loop
A transient container init delay will fail the run. Poll a few times before giving up to reduce flakes.Apply this diff:
- # Probe container as readiness check instead of listapps - xcrun simctl get_app_container "$SIMULATOR_ID" "$IOS_BUNDLE_ID" app >/dev/null 2>&1 || sleep 5 + # Probe container as readiness check with retries + for i in {1..10}; do + if xcrun simctl get_app_container "$SIMULATOR_ID" "$IOS_BUNDLE_ID" app >/dev/null 2>&1; then + echo "✅ App container available" + break + fi + if [ $i -eq 10 ]; then + echo "❌ App container not available after retries" + exit 1 + fi + echo "Waiting for app container... attempt $i/10" + sleep 3 + done.github/workflows/mobile-ci.yml (1)
161-168: Unreachable “Pods directory incomplete” branch; restructure detection logic
This block is inside a branch that already guarantees Pods exists and is non-empty, so the subsequent “incomplete” checks will never fire.Apply this minimal correction to make the intent clear (log missing Podfile.lock only), or restructure the outer condition:
- # If Pods directory exists but is incomplete, clear only Pods; keep Podfile.lock - if [ ! -d "app/ios/Pods" ] || [ -z "$(ls -A app/ios/Pods 2>/dev/null)" ]; then - echo "⚠️ Pods directory incomplete - clearing Pods (preserving Podfile.lock)" - rm -rf app/ios/Pods - fi + # Pods present and non-empty; just warn if Podfile.lock is missing + :If you want to handle the “incomplete” scenario, perform that check outside the “cache restored successfully” branch:
# Before/after the main branch: if [ ! -d "app/ios/Pods" ] || [ -z "$(ls -A app/ios/Pods 2>/dev/null)" ]; then echo "⚠️ Pods directory missing/empty - clearing Pods (preserving Podfile.lock)" rm -rf app/ios/Pods fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.cursor/mcp.json(1 hunks).giga/specifications.json(1 hunks).github/workflows/mobile-ci.yml(1 hunks).github/workflows/mobile-e2e.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-android
- GitHub Check: e2e-ios
🔇 Additional comments (3)
.giga/specifications.json (3)
9-10: Confirm consumer concatenates directory path + fileName; otherwise lookups will break
Path now points to a directory rather than a file. If the loader expects a file path, this change will 404 at runtime.Action: Verify the code that loads specifications constructs full paths like "${path}/${fileName}" and doesn’t assume path itself is the file. If needed, I can help patch the loader.
14-15: Same concern for technical-specification.mdc
Ensure the consumer resolves directory + fileName, not the raw path.
19-20: Same concern for compliance-verification.mdc
Validate resolution logic to prevent runtime file-not-found errors.
| "args": [ | ||
| "-y", | ||
| "mcp-remote@1.2.3", | ||
| "mcp-remote@latest", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin mcp-remote version to avoid supply‑chain risk and flaky environments
Using @latest makes dev/CI non‑deterministic and increases the blast radius for a compromised or breaking upstream release.
Apply this diff to pin a known-good version (replace 1.2.3 with the intended version used internally):
- "mcp-remote@latest",
+ "[email protected]",Optionally, consider vendoring via a lockfile or preinstall to further control integrity.
📝 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.
| "mcp-remote@latest", | |
| "mcp-remote@1.2.3", |
🤖 Prompt for AI Agents
In .cursor/mcp.json around line 7, the dependency is using the non-deterministic
"mcp-remote@latest"; replace it with a pinned, known-good version (e.g.
"[email protected]") by editing that entry to the intended exact version, update
any lockfile or commit the updated mcp.json, and optionally vendor the package
or add a preinstall/lock mechanism to ensure repeatable builds and reduce
supply-chain risk.
There was a problem hiding this comment.
1.2.3 is not a real version unfortunately. can you confirm?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
transphorm
force-pushed
the
codex/fix-failing-android-e2e-test
branch
from
5e59a56 to
dee2add
Compare
transphorm
merged commit e40a020
into
codex/optimize-android-e2e-test-workflow
transphorm
force-pushed
the
codex/optimize-android-e2e-test-workflow
branch
from
5110508 to
e40a020
Compare
Summary
Testing
yarn lintyarn buildyarn workspace @selfxyz/contracts build(fails: Invalid account: #0 for network: mainnet - Expected string, received undefined)yarn typeshttps://chatgpt.com/codex/tasks/task_b_68a298272fac832dad63e7c82d312d18
Summary by CodeRabbit