feat: Add CI configuration for building and testing CMP #21
Conversation
…icker across multiple platforms
|
Warning Rate limit exceeded@kez-lab has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new GitHub Actions CI workflow is added that runs automated builds and tests for a Compose Multiplatform TimePicker project across multiple platforms: Android, WebAssembly, Desktop, and iOS. The workflow executes on main branch pushes and pull requests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive CI/CD workflow for building and testing a Compose Multiplatform DateTimePicker module across multiple platforms (Android, WebAssembly, Desktop JVM, and iOS). The workflow automates quality assurance by running platform-specific builds and tests on each push and pull request to the main branch.
Key Changes:
- Added GitHub Actions workflow with matrix-based builds for Linux platforms (Android, WASM, Desktop)
- Configured iOS simulator testing on macOS runners
- Implemented concurrency controls and appropriate permissions for secure CI execution
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| contents: read | ||
|
|
||
| env: | ||
| MODULE: ":datetimepicker" |
There was a problem hiding this comment.
[nitpick] Missing newline after the environment variable declaration. For better YAML readability and consistency, add a blank line after line 17 before the jobs: section.
| MODULE: ":datetimepicker" | |
| MODULE: ":datetimepicker" |
| "build-tools;34.0.0" || true | ||
| yes | sdkmanager --licenses || true |
There was a problem hiding this comment.
The sdkmanager command uses || true to suppress errors, which could mask real installation failures. Additionally, the second command yes | sdkmanager --licenses || true will always succeed even if licenses fail to accept. Consider using proper error handling or at least logging the results:
run: |
sdkmanager "platform-tools" \
"platforms;android-34" \
"build-tools;34.0.0"
yes | sdkmanager --licensesIf you intentionally want to continue on errors, consider adding comments explaining why failures are acceptable.
| "build-tools;34.0.0" || true | |
| yes | sdkmanager --licenses || true | |
| "build-tools;34.0.0" | |
| yes | sdkmanager --licenses |
|
|
||
| - name: Boot iOS Simulator (iPhone 15) | ||
| run: | | ||
| LATEST_IOS=$(xcrun simctl list runtimes | grep "iOS" | tail -n 1 | sed -E 's/.*iOS ([0-9]+\.[0-9]+).*/iOS \1/') |
There was a problem hiding this comment.
The iOS runtime extraction logic may be fragile. The sed command expects a specific format and the extracted runtime string may not match what xcrun simctl expects. Consider using a more robust approach:
LATEST_IOS=$(xcrun simctl list runtimes | grep "iOS" | tail -n 1 | awk '{print $NF}')
echo "Using iOS runtime: $LATEST_IOS"Or use a specific iOS version to ensure consistency across CI runs.
| LATEST_IOS=$(xcrun simctl list runtimes | grep "iOS" | tail -n 1 | sed -E 's/.*iOS ([0-9]+\.[0-9]+).*/iOS \1/') | |
| LATEST_IOS=$(xcrun simctl list runtimes | grep "iOS" | tail -n 1 | awk '{print $NF}') |
| xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS" || true | ||
| xcrun simctl boot "CI-iPhone15" || true |
There was a problem hiding this comment.
Using || true to suppress errors when creating and booting the simulator could hide actual failures. If the simulator fails to boot, the iOS tests will fail anyway but without clear diagnostic information. Consider checking the exit status and providing better error messages:
if ! xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS"; then
echo "Simulator already exists or creation failed"
fi
if ! xcrun simctl boot "CI-iPhone15"; then
echo "Simulator already booted or boot failed"
fi| xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS" || true | |
| xcrun simctl boot "CI-iPhone15" || true | |
| if ! xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS"; then | |
| echo "Simulator already exists or creation failed" | |
| fi | |
| if ! xcrun simctl boot "CI-iPhone15"; then | |
| echo "Simulator already booted or boot failed" | |
| fi |
![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
🧹 Nitpick comments (1)
.github/workflows/integration-build-test.yml (1)
48-50: Document or centralize hard-coded Android API and build-tools versions.Android API level (34) and build-tools version (34.0.0) are hard-coded here and will require manual updates as Android toolchains evolve. Consider:
- Extracting these to a shared gradle configuration (e.g.,
gradle.propertiesorbuildSrc) for consistency with your build scripts.- Adding a comment documenting the rationale (if intentionally pinned).
- Adding a periodic maintenance reminder if versions need to stay current with Android releases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration-build-test.yml(1 hunks)
⏰ 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). (4)
- GitHub Check: Linux • wasm
- GitHub Check: Linux • desktop
- GitHub Check: Linux • android
- GitHub Check: macOS • iOS Simulator Test
🔇 Additional comments (3)
.github/workflows/integration-build-test.yml (3)
1-17: Workflow configuration looks good.Trigger conditions, concurrency controls, and minimal permissions are appropriately configured.
54-81: Gradle build tasks are well-structured.Platform-specific tasks and consistent use of
gradle/gradle-build-action@v3look good. The matrix strategy efficiently covers all three Linux targets.
108-114: Gradle tasks look appropriate.The iOS build and test tasks are well-configured, pending resolution of the simulator boot issues flagged earlier.
| - name: Install Android build tools | ||
| if: matrix.target == 'android' | ||
| run: | | ||
| sdkmanager "platform-tools" \ | ||
| "platforms;android-34" \ | ||
| "build-tools;34.0.0" || true | ||
| yes | sdkmanager --licenses || true |
There was a problem hiding this comment.
Remove error suppression from critical SDK setup commands.
The || true patterns on lines 50–51 mask failures in SDK installation and license acceptance, allowing the workflow to proceed with incomplete or failed setup. This defeats CI safety guarantees.
Replace these commands with explicit error handling, or remove || true to fail fast on problems:
- name: Install Android build tools
if: matrix.target == 'android'
run: |
- sdkmanager "platform-tools" \
- "platforms;android-34" \
- "build-tools;34.0.0" || true
- yes | sdkmanager --licenses || true
+ sdkmanager "platform-tools" \
+ "platforms;android-34" \
+ "build-tools;34.0.0"
+ yes | sdkmanager --licensesIf occasional transient failures are expected, use a retry mechanism (e.g., retry action or script-level backoff) instead of silently ignoring all errors.
📝 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.
| - name: Install Android build tools | |
| if: matrix.target == 'android' | |
| run: | | |
| sdkmanager "platform-tools" \ | |
| "platforms;android-34" \ | |
| "build-tools;34.0.0" || true | |
| yes | sdkmanager --licenses || true | |
| - name: Install Android build tools | |
| if: matrix.target == 'android' | |
| run: | | |
| sdkmanager "platform-tools" \ | |
| "platforms;android-34" \ | |
| "build-tools;34.0.0" | |
| yes | sdkmanager --licenses |
🤖 Prompt for AI Agents
.github/workflows/integration-build-test.yml around lines 45 to 51: the SDK
installation and license acceptance lines silently swallow errors with `||
true`; remove the `|| true` suffixes so failures cause the job to fail fast, and
replace with a simple retry wrapper if transient failures are expected (e.g., a
small shell loop that retries sdkmanager commands N times with sleep/backoff and
exits non-zero on final failure) or use a dedicated retry action; also ensure
the `yes | sdkmanager --licenses` command is allowed to surface errors rather
than being forced to succeed.
| - name: Boot iOS Simulator (iPhone 15) | ||
| run: | | ||
| LATEST_IOS=$(xcrun simctl list runtimes | grep "iOS" | tail -n 1 | sed -E 's/.*iOS ([0-9]+\.[0-9]+).*/iOS \1/') | ||
| echo "Using iOS runtime: $LATEST_IOS" | ||
| xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS" || true | ||
| xcrun simctl boot "CI-iPhone15" || true | ||
| xcrun simctl list | head -n 50 |
There was a problem hiding this comment.
Validate iOS simulator setup and remove error suppression.
The iOS simulator boot sequence masks failures with || true (lines 103–104) and uses fragile regex parsing (line 101) that could fail silently. Additionally, line 105 appears to be debugging output.
Issues:
- If simulator creation or boot fails, tests will still run, likely against stale or missing simulator state, causing cryptic failures later.
- The regex parsing
sed -E 's/.*iOS ([0-9]+\.[0-9]+).*/iOS \1/'breaks if xcrun output format changes. - No validation that simulator boot actually succeeded before proceeding to tests.
- Line 105
xcrun simctl list | head -n 50looks like debug output.
Suggested improvements:
- name: Boot iOS Simulator (iPhone 15)
run: |
LATEST_IOS=$(xcrun simctl list runtimes | grep "iOS" | tail -n 1 | sed -E 's/.*iOS ([0-9]+\.[0-9]+).*/iOS \1/')
echo "Using iOS runtime: $LATEST_IOS"
- xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS" || true
- xcrun simctl boot "CI-iPhone15" || true
- xcrun simctl list | head -n 50
+ xcrun simctl create "CI-iPhone15" "iPhone 15" "$LATEST_IOS" || xcrun simctl delete "CI-iPhone15" 2>/dev/null || true
+ xcrun simctl boot "CI-iPhone15"
+ # Verify simulator is booted
+ sleep 2
+ xcrun simctl list | grep "CI-iPhone15" | grep "(Booted)" || (echo "Simulator failed to boot" && exit 1)Alternatively, consider using a well-maintained action like futureware-tech/simulator-action or similar to abstract this complexity.
🤖 Prompt for AI Agents
.github/workflows/integration-build-test.yml around lines 99 to 105: the current
steps suppress failures with "|| true", use a fragile sed regex to pick the
latest iOS runtime, and leave a debug list command; update the job to (1) fail
the run instead of swallowing errors (remove "|| true"), (2) select the runtime
robustly by parsing JSON from `xcrun simctl list runtimes --json` (or validate
the extracted runtime exists), (3) after creating and booting the simulator
check its state in a retry loop with a timeout and exit non‑zero if it never
reaches "Booted", and (4) remove or gate the `xcrun simctl list | head -n 50`
debug output; alternatively replace the whole sequence with a maintained GitHub
Action such as futureware‑tech/simulator-action to handle creation, boot and
validation.
This pull request introduces a new GitHub Actions workflow for continuous integration (CI) targeting the Compose Multiplatform TimePicker module. The workflow automates building and testing across multiple platforms (Android, WebAssembly, Desktop, and iOS) to ensure code quality and compatibility.
New CI workflow for multiplatform build and test:
.github/workflows/integration-build-test.ymlto define a CI pipeline that builds and tests the:datetimepickermodule on Android, WebAssembly, Desktop (JVM), and iOS platforms.Summary by CodeRabbit