-
Notifications
You must be signed in to change notification settings - Fork 34
[DON-2137] Spike: Parallelize CI build workflow #2522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Split the monolithic Build job into independent parallel jobs: - Detekt: Static analysis - Lint: Android lint checks - UnitTests: Unit test execution - BuildAndTokens: Build + token generation/validation This reduces wall-clock time from ~7min to ~4min by running Detekt, Lint, and Unit Tests in parallel instead of sequentially. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Co-authored-by: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR parallelizes the CI build workflow by splitting a single monolithic Build job into four independent parallel jobs (Detekt, Lint, UnitTests, and BuildAndTokens), aiming to reduce wall-clock time by ~43% while accepting a 14% increase in runner minutes.
Key Changes:
- Extracted static analysis (Detekt, Lint) and unit tests into separate parallel jobs
- Reduced individual job timeout from 30 to 15-20 minutes
- Reorganized workflow structure with parallel job groups
This commit makes two key improvements to the CI workflow: 1. **Combine Detekt and Lint into single Static Analysis job** - Reduces number of parallel jobs from 6 to 5 - Saves on duplicate checkout and JDK setup overhead - Expected completion time: ~4 minutes (vs ~2.5 min for longest individual job) 2. **Parallelize screenshot tests by variant** - Splits the monolithic Screenshots job (~12 min) into 4 parallel jobs - Each variant (default, dark mode, RTL, themed) runs independently - Expected completion time: ~3-4 minutes per variant (vs 12 min sequential) - Added Screenshots-Collect job to merge results and commit changes - Uses GitHub Actions artifacts to transfer screenshots between jobs Expected impact: - Overall wall-clock time: ~4-5 minutes (down from ~12 min) - This addresses the screenshot bottleneck identified in DON-2137 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
Addresses Copilot review feedback: - Use loop to iterate over variants instead of repetitive if blocks - Add proper error checking with compgen to detect missing artifacts - Add informative logging for each variant - Removes error suppression (2>/dev/null) for better debugging This makes the code more maintainable and easier to add/remove variants in the future. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Co-authored-by: Claude <[email protected]>
Co-authored-by: Claude <[email protected]>
Addresses Copilot review feedback about code duplication. Changes: - Consolidate 4 separate screenshot jobs into 1 matrix job - All 4 variants still run in parallel (matrix default behavior) - Reduced code from ~117 lines to ~38 lines - Easier to add/remove variants (just update the matrix list) - Updated Screenshots-Collect to depend on the single matrix job Performance: No change - all 4 variants continue running in parallel 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Co-authored-by: Claude <[email protected]>
| pull-requests: write | ||
| contents: write | ||
| timeout-minutes: 20 | ||
| timeout-minutes: 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we confident about dropping the timeout to 5 minutes? That’s a pretty aggressive reduction (20 → 5). If we’re confident that, after splitting, each job will consistently finish within 5 minutes, then I’m happy to keep it.
| - name: RTL | ||
| flag: rtl | ||
| - name: themed | ||
| flag: themed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker
Is there a way to make this dynamic, instead of setting all variants manually could we read existing variants in the repo and extract the variant names
| with: | ||
| java-version: '17' | ||
| distribution: 'adopt' | ||
| cache: gradle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker
I noticed we set up the JDK in each job. Could we extract that into a reusable action and call it from the jobs that need it? That way we only update the Java version in one place instead of touching every job.
|
|
||
| - name: Screenshot Tests | ||
| id: screenshotTests | ||
| - name: Merge screenshot results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could we rename this from merge to combine? We usually say “merged PR”, so “merge” here might make people think we’re actually merging screenshots rather than just combining them.
Summary
This PR implements a proof of concept for parallelizing the CI build workflow to reduce PR validation time.
Changes
Expected Results
Documentation
Test plan
🤖 Generated with Claude Code