chore(ci): refactor template android/ios workflows#329
Conversation
perms: reduce to read-only support workflow_dispatch matrix: fail-fast=false rm pnpm setup node only for yarn add pods/gradle cache env-based iOS scheme
android: consolidate to matrix, add gradle cache, JDK v5 setup-java ios: consolidate to matrix, add CocoaPods repo+Pods cache, Ruby 3.2, workflow_dispatch
WalkthroughAdds Xcode setup to the iOS E2E workflow, overhauls the Nitro CLI test workflow with permissions, triggers, caching, codegen, artifact download, and scheme handling; consolidates Android and iOS template build workflows into matrix jobs with caching and updated toolchains; and adds explicit error handling to the E2E build script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/Upstream Workflow
participant GH as GitHub Actions
participant JobIOS as test-ios-build
participant JobAndroid as test-android-build
participant Art as Artifacts
participant Cache as Caches (CocoaPods/Gradle)
Dev->>GH: Trigger (workflow_dispatch or upstream success)
GH->>JobIOS: Start iOS job (macOS-latest, fail-fast: false)
GH->>JobAndroid: Start Android job (ubuntu-latest, fail-fast: false)
rect rgb(240,245,255)
note right of JobIOS: New steps
JobIOS->>Art: Download generated package
JobIOS->>Cache: Restore CocoaPods cache
JobIOS->>JobIOS: Setup Xcode (where applicable), setup Node/Yarn
JobIOS->>JobIOS: bunx nitro-codegen && build && post-script
JobIOS->>JobIOS: xcodebuild using SCHEME env
JobIOS-->>Cache: Save CocoaPods cache (on success)
end
rect rgb(240,255,240)
note right of JobAndroid: New steps
JobAndroid->>Art: Download generated package
JobAndroid->>Cache: Restore Gradle cache
JobAndroid->>JobAndroid: setup Node/Yarn
JobAndroid->>JobAndroid: bunx nitro-codegen && build && post-script
JobAndroid->>JobAndroid: ./gradlew assembleDebug --build-cache
JobAndroid-->>Cache: Save Gradle cache (on success)
end
note over JobIOS,JobAndroid: Logs include package structure listing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.github/workflows/e2e-tests.yml (2)
91-106: Fix: run E2E from the generated package directory (current step runs from repo root).The emulator step invokes
${{ matrix.pm }} android:e2e ...withoutcdinto${{ env.WORKING_DIR }}. The script likely lives in the generated package’s package.json, so this will fail to find the npm/bun script.Apply:
script: | + cd "${{ env.WORKING_DIR }}" # Wait for system to settle adb wait-for-device shell 'while [[ -z $(getprop sys.boot_completed) ]]; do sleep 1; done' # Retry once if necessary - ${{ matrix.pm }} android:e2e ${{ env.WORKING_DIR }}/example ${{ matrix.package-type }} + ${{ matrix.pm }} android:e2e ${{ env.WORKING_DIR }}/example ${{ matrix.package-type }}
183-186: Fix: set working-directory for iOS E2E invocation.Same issue as Android:
${{ matrix.pm }} ios:e2eis executed from repo root instead of the generated package directory.- - name: Run tests - run: | - ${{ matrix.pm }} ios:e2e ${{ env.WORKING_DIR }}/example ${{ matrix.package-type }} + - name: Run tests + working-directory: ${{ env.WORKING_DIR }} + run: | + ${{ matrix.pm }} ios:e2e ${{ env.WORKING_DIR }}/example ${{ matrix.package-type }}scripts/e2e-maestro.sh (2)
3-3: Harden script with strict mode.Enable early failure and safer bash behavior.
-#!/bin/bash +#!/bin/bash +set -euo pipefail
44-46: Bug: simulator UUID regex is wrong (expects parentheses, misses lowercase).
simctl listoutputs UUIDs in square brackets and often lowercase hex. Current grep returns empty ID.- iphone16Id=$(xcrun simctl list devices | grep "iPhone 16 (" | grep -E '\(Booted\)|\(Shutdown\)' | head -1 | grep -E -o '\([0-9A-F-]{36}\)' | tr -d '()') + iphone16Id=$(xcrun simctl list devices | grep "iPhone 16 (" | grep -E '\(Booted\)|\(Shutdown\)' | head -1 | grep -E -o '\[[0-9a-fA-F-]{36}\]' | tr -d '[]') + if [ -z "${iphone16Id:-}" ]; then + echo "⚠️ iPhone 16 simulator not found. Falling back to first available iPhone simulator..." + iphone16Id=$(xcrun simctl list devices | grep -E 'iPhone .* \((Booted|Shutdown)\)' | head -1 | grep -E -o '\[[0-9a-fA-F-]{36}\]' | tr -d '[]' || true) + fi + if [ -z "${iphone16Id:-}" ]; then + echo "❌ No suitable iPhone simulator found." + exit 1 + fi.github/workflows/test-nitro-cli.yml (1)
193-198: Avoid duplicate Gradle caching.You use both
setup-java@v5cache and an explicit cache. Keep one.Same options as Android template:
- Either remove the explicit “Cache Gradle” step, or
- Drop
cache: 'gradle'from setup-java and keep the explicit cache (widen the key to includegradle.properties).Also applies to: 203-212
🧹 Nitpick comments (6)
.github/workflows/e2e-tests.yml (1)
168-175: Optional: also cache CocoaPods spec repos for speed.Caching only Pods helps; adding
~/.cocoapods/reposfurther reduces cold start times.with: - path: ${{ env.WORKING_DIR }}/example/ios/Pods + path: | + ~/.cocoapods/repos + ${{ env.WORKING_DIR }}/example/ios/Podsscripts/e2e-maestro.sh (1)
80-86: Minor: ensure device is actually booted before proceeding.You already boot if needed; add a short wait to avoid race conditions.
else echo "✅ Simulator already booted" fi - # Wait for 10 seconds - sleep 10 + # Wait for SpringBoard to settle + xcrun simctl bootstatus "$iphone16Id" -b || sleep 10assets/template/.github/workflows/ios-build.yml (1)
67-70: Optional: make new-arch toggle more robust.Current sed expects an exact string. Consider handling absent or boolean variants.
- run: sed -i "" "s/ENV\['RCT_NEW_ARCH_ENABLED'\] = '1'/ENV['RCT_NEW_ARCH_ENABLED'] = '0'/g" example/ios/Podfile + run: | + if grep -q "RCT_NEW_ARCH_ENABLED" example/ios/Podfile; then + sed -i "" "s/ENV\['RCT_NEW_ARCH_ENABLED'\].*/ENV['RCT_NEW_ARCH_ENABLED'] = '0'/g" example/ios/Podfile + else + echo "ENV['RCT_NEW_ARCH_ENABLED'] = '0'" >> example/ios/Podfile + fiassets/template/.github/workflows/android-build.yml (1)
52-55: Optional: resilient new-arch toggle.Handle missing property and whitespace.
- run: sed -i "s/newArchEnabled=true/newArchEnabled=false/g" example/android/gradle.properties + run: | + if grep -qE '^\s*newArchEnabled=' example/android/gradle.properties; then + sed -i "s/^\s*newArchEnabled\s*=.*/newArchEnabled=false/g" example/android/gradle.properties + else + echo "newArchEnabled=false" >> example/android/gradle.properties + fi.github/workflows/test-nitro-cli.yml (2)
96-105: Optional: cache CocoaPods spec repos too.Mirrors the iOS template; speeds up cold runs.
with: - path: | - ~/.cocoapods/repos - ${{ env.WORKING_DIR }}/example/ios/Pods + path: | + ~/.cocoapods/repos + ${{ env.WORKING_DIR }}/example/ios/Pods
58-63: Optional: point bundler cache at the example/ios Gemfile.If the generated package has a Gemfile in
example/ios, set the step working directory.- name: Setup Ruby and CocoaPods uses: ruby/setup-ruby@v1 with: ruby-version: '3.2' bundler-cache: true + working-directory: ${{ env.WORKING_DIR }}/example/ios
📜 Review details
Configuration used: CodeRabbit UI
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 (5)
.github/workflows/e2e-tests.yml(1 hunks).github/workflows/test-nitro-cli.yml(8 hunks)assets/template/.github/workflows/android-build.yml(2 hunks)assets/template/.github/workflows/ios-build.yml(4 hunks)scripts/e2e-maestro.sh(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
.github/workflows/e2e-tests.yml (1)
src/generate-nitro-package.ts (1)
setupWorkflows(475-499)
assets/template/.github/workflows/android-build.yml (1)
src/generate-nitro-package.ts (2)
setupWorkflows(475-499)syncExampleAppConfigurations(345-457)
.github/workflows/test-nitro-cli.yml (1)
src/generate-nitro-package.ts (1)
setupWorkflows(475-499)
assets/template/.github/workflows/ios-build.yml (1)
src/generate-nitro-package.ts (1)
setupWorkflows(475-499)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Generate with yarn - module
- GitHub Check: Generate with bun - view
| if: github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success' | ||
| runs-on: macOS-latest | ||
| strategy: |
There was a problem hiding this comment.
Bug: manual dispatch path lacks a run-id for artifact download.
When workflow_dispatch triggers this workflow, ${{ github.event.workflow_run.id }} is undefined and the download step will fail.
Add an optional input and use it when manually dispatched:
on:
workflow_run:
workflows: ['Generate Packages']
types:
- completed
- workflow_dispatch:
+ workflow_dispatch:
+ inputs:
+ upstream_run_id:
+ description: 'Run ID of the upstream "Generate Packages" workflow'
+ required: false
+ type: stringThen select the run-id:
- run-id: ${{ github.event.workflow_run.id }}
+ run-id: ${{ github.event.workflow_run.id || inputs.upstream_run_id }}And guard the step when neither is present:
- - name: Download generated package
+ - name: Download generated package
+ if: github.event_name == 'workflow_dispatch' && inputs.upstream_run_id || github.event_name == 'workflow_run'Also applies to: 38-46
| - name: Setup JDK 17 | ||
| uses: actions/setup-java@v4 | ||
| uses: actions/setup-java@v5 | ||
| with: | ||
| distribution: 'zulu' | ||
| java-version: 17 | ||
| java-package: jdk | ||
| cache: gradle | ||
| java-version: '17' | ||
| cache: 'gradle' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid double Gradle caching.
You’re using both setup-java’s cache: gradle and an explicit Gradle cache step. Keep one to prevent redundant restores/saves.
Option A (keep setup-java cache, remove explicit cache):
- - name: Cache Gradle
- uses: actions/cache@v4
- with:
- path: |
- ~/.gradle/caches
- ~/.gradle/wrapper
- key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/**/*.gradle*') }}
- restore-keys: |
- ${{ runner.os }}-gradle-Option B (remove cache: gradle and keep explicit cache with broader key):
- with:
- distribution: 'zulu'
- java-version: '17'
- cache: 'gradle'
+ with:
+ distribution: 'zulu'
+ java-version: '17'And widen cache key:
- key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/**/*.gradle*') }}
+ key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/**/*.gradle*', 'example/android/**/gradle.properties') }}Also applies to: 63-72
🤖 Prompt for AI Agents
In assets/template/.github/workflows/android-build.yml around lines 56-62 (and
similarly lines 63-72), the workflow currently enables Gradle caching twice by
setting actions/setup-java@v5 with cache: 'gradle' and also using an explicit
Gradle cache step; remove the redundancy by choosing one approach: either remove
the explicit Gradle cache step and rely on setup-java's cache: 'gradle', or
remove cache: 'gradle' from setup-java and keep the explicit cache step, then
widen the explicit cache key to include OS, Gradle wrapper version, and
gradle.properties/hash of build.gradle files to make it more robust; update only
the workflow caching sections accordingly to avoid duplicate restores/saves.
| - name: Setup Ruby (bundle) | ||
| uses: ruby/setup-ruby@v1 | ||
| with: | ||
| ruby-version: 2.7.2 | ||
| ruby-version: '3.2' | ||
| bundler-cache: true | ||
| working-directory: example/ios | ||
|
|
There was a problem hiding this comment.
Fix: ruby/setup-ruby working-directory is misplaced.
ruby/setup-ruby@v1 doesn’t accept working-directory under with. Put it at the step level so Bundler caches against example/ios/Gemfile.
- - name: Setup Ruby (bundle)
- uses: ruby/setup-ruby@v1
- with:
- ruby-version: '3.2'
- bundler-cache: true
- working-directory: example/ios
+ - name: Setup Ruby (bundle)
+ uses: ruby/setup-ruby@v1
+ with:
+ ruby-version: '3.2'
+ bundler-cache: true
+ working-directory: example/iosCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
assets/template/.github/workflows/ios-build.yml around lines 71 to 77: the step
config places working-directory under the `with:` block for ruby/setup-ruby, but
that action expects working-directory at the step level; move
`working-directory: example/ios` out of the `with:` map to be a sibling of
`uses:` and `with:` so the action executes in example/ios and the bundler-cache
targets example/ios/Gemfile.
This refactors the Android and iOS template workflows for clarity, speed, and consistency.
Android
iOS
Summary by CodeRabbit
New Features
Bug Fixes
Chores