Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCI workflow updated: manual trigger added; iOS job pins Rust 1.90.0, builds/uploads native xcframework; Android CI replaces custom emulator script with android-emulator-runner and inline APK install; Android CI script removed; minor script path, Maestro port-forward, test flow, README, and rust-toolchain changes. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as android-emulator-runner
participant Emulator as Android Emulator
participant CI_Script as inline CI script
participant Gradle as Gradle
participant ADB as adb
participant Maestro as Maestro CLI / Instrumentation
GH->>Runner: start emulator (API 34, google_atd, x86_64, Pixel 6)
Runner->>Emulator: boot with emulator options (extended timeout, opts)
Runner->>GH: emulator ready
GH->>CI_Script: run inline script
CI_Script->>Gradle: ./gradlew assembleDebug
Gradle-->>CI_Script: debug APK
CI_Script->>ADB: install APK on Emulator
CI_Script->>Maestro: bun run maestro:android
Maestro->>Emulator: run instrumentation / embedded driver APKs
Maestro->>ADB: perform CLI-managed port-forwarding
sequenceDiagram
participant GH as GitHub Actions
participant Rust as Rust toolchain (1.90.0)
participant Build as native iOS core build script
participant Archiver as tar + upload-artifact
participant Runner as downstream job (maestro_ios)
GH->>Rust: setup Rust toolchain 1.90.0
GH->>Build: cd rust && ./build_native_xcframework.sh
Build-->>Archiver: produce ios/CapacitorFFmpegNativeCore.xcframework
Archiver->>GH: upload tarball artifact
Runner->>GH: download artifact
Runner->>Archiver: extract tarball into ios/
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
example-app/README.md (1)
44-44: Mention the local Maestro prerequisite here.This paragraph explains the CLI/script split well, but
bun run maestro:androidstill fails fast unless Maestro CLI is already installed locally. Adding that prerequisite next to this note would make the README easier to follow from a fresh checkout.✍️ Suggested wording
The Android script bootstraps Maestro's embedded driver APKs explicitly and starts the instrumentation runner before running the flows. The CLI still owns the ADB port-forward setup so local runs match CI without fighting Maestro's session setup. +Install Maestro CLI first before running `bun run maestro:android`; the script expects the local CLI artifacts under `$HOME/.maestro`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example-app/README.md` at line 44, Update the README paragraph about the Android script to explicitly list the local prerequisite that the Maestro CLI must be installed before running "bun run maestro:android"; edit the section containing the sentence about bootstrapping Maestro's embedded driver APKs and ADB port-forwarding to add a short note like “Prerequisite: Maestro CLI installed locally (required for bun run maestro:android to succeed)”, so readers see the requirement next to the CLI/script explanation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@example-app/README.md`:
- Line 44: Update the README paragraph about the Android script to explicitly
list the local prerequisite that the Maestro CLI must be installed before
running "bun run maestro:android"; edit the section containing the sentence
about bootstrapping Maestro's embedded driver APKs and ADB port-forwarding to
add a short note like “Prerequisite: Maestro CLI installed locally (required for
bun run maestro:android to succeed)”, so readers see the requirement next to the
CLI/script explanation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14a65aff-aa15-431a-9a0d-8c531d322af4
📒 Files selected for processing (4)
.github/workflows/test.ymlexample-app/README.mdexample-app/scripts/run-maestro-android-ci.shexample-app/scripts/run-maestro-android.sh
💤 Files with no reviewable changes (2)
- example-app/scripts/run-maestro-android.sh
- example-app/scripts/run-maestro-android-ci.sh
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/test.yml (2)
118-121: Build the debug APK before booting the emulator.
example-app/scripts/run-maestro-android.shonly needs the app to already be installed. KeepingassembleDebuginside the emulator block means the emulator sits booted while Gradle compiles, which adds runtime and flake surface without changing the handoff tobun run maestro:android.♻️ Possible split
+ - name: Build Android debug APK + run: ./example-app/android/gradlew -p ./example-app/android assembleDebug - name: Run Android Maestro flows uses: reactivecircus/android-emulator-runner@v2 with: api-level: 34 target: google_atd arch: x86_64 profile: pixel_6 emulator-boot-timeout: 900 emulator-options: -no-window -gpu swiftshader_indirect -no-snapshot -noaudio -no-boot-anim -camera-back none -camera-front none disable-animations: true script: | - ./example-app/android/gradlew -p ./example-app/android assembleDebug adb install -r example-app/android/app/build/outputs/apk/debug/app-debug.apk cd example-app && bun run maestro:android🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 118 - 121, Move the Gradle build out of the emulator boot step so the APK is compiled before the emulator starts: run ./example-app/android/gradlew -p ./example-app/android assembleDebug first (so the debug APK is ready), then boot the emulator and run adb install -r example-app/android/app/build/outputs/apk/debug/app-debug.apk followed by cd example-app && bun run maestro:android; ensure the workflow step referencing example-app/scripts/run-maestro-android.sh is only run after the assembleDebug step completes.
54-55: Consider pinning the Rust toolchain to a specific version instead ofstable.The workflow currently uses
dtolnay/rust-toolchain@stable, which tracks the latest stable Rust release. This can cause unexpected build failures if a new stable version introduces breaking changes affectingbuild_native_xcframework.sh. The repository has no pinned Rust version file (rust-toolchainorrust-toolchain.toml), so either create one to establish a consistent toolchain version across the project, or specify an explicit version directly in this workflow step.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 54 - 55, The workflow step named "Setup Rust" currently uses dtolnay/rust-toolchain@stable which floats; pin the toolchain by either creating a rust-toolchain or rust-toolchain.toml in the repo with your chosen channel/version (e.g., 1.xx.x) or replace the action reference dtolnay/rust-toolchain@stable with an explicit tag/version (e.g., dtolnay/rust-toolchain@1.xx.x) so builds of the "Setup Rust" step and the script build_native_xcframework.sh use a consistent, reproducible Rust toolchain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 118-121: Move the Gradle build out of the emulator boot step so
the APK is compiled before the emulator starts: run
./example-app/android/gradlew -p ./example-app/android assembleDebug first (so
the debug APK is ready), then boot the emulator and run adb install -r
example-app/android/app/build/outputs/apk/debug/app-debug.apk followed by cd
example-app && bun run maestro:android; ensure the workflow step referencing
example-app/scripts/run-maestro-android.sh is only run after the assembleDebug
step completes.
- Around line 54-55: The workflow step named "Setup Rust" currently uses
dtolnay/rust-toolchain@stable which floats; pin the toolchain by either creating
a rust-toolchain or rust-toolchain.toml in the repo with your chosen
channel/version (e.g., 1.xx.x) or replace the action reference
dtolnay/rust-toolchain@stable with an explicit tag/version (e.g.,
dtolnay/rust-toolchain@1.xx.x) so builds of the "Setup Rust" step and the script
build_native_xcframework.sh use a consistent, reproducible Rust toolchain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68ee1f99-59c1-4bbc-b96f-0678e522b74b
📒 Files selected for processing (2)
.github/workflows/test.ymlrust/build_x264_ios_sim_arm64.sh
|
Superseded by #8, which targets main and contains the current green Maestro CI work. |
What
Why
How
Testing
Build source code and test iton commit8c6026e24143425949Build code and test,build_android,build_ios,guard_swiftpm_version,maestro_android,maestro_iosNot Tested
Summary by CodeRabbit