Skip to content

Infra/ci refactoring#177

Merged
komodgn merged 5 commits intodevelopfrom
infra/ci-refactoring
Apr 18, 2026
Merged

Infra/ci refactoring#177
komodgn merged 5 commits intodevelopfrom
infra/ci-refactoring

Conversation

@komodgn
Copy link
Copy Markdown
Owner

@komodgn komodgn commented Apr 18, 2026

Related issue

Work Description ✏️

Screenshot 📸

Summary by CodeRabbit

  • Chores
    • Streamlined Android CI/CD pipeline with a reusable build environment setup component
    • Separated unit test execution into its own workflow with automated test artifact collection and reporting
    • Added visual regression detection reporting to pull requests

komodgn added 3 commits April 18, 2026 13:51
- Extract unit test logic from android-ci.yml into a dedicated unit-test.yml.
- Add Roborazzi screenshot recording and artifact upload to the unit test workflow.
- Use shared setup-android custom action for environment consistency.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 18, 2026

Warning

Rate limit exceeded

@komodgn has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 15 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 15 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2290d3b8-ed1e-42b3-ad31-7efa8735286a

📥 Commits

Reviewing files that changed from the base of the PR and between 2532847 and 315f17d.

📒 Files selected for processing (2)
  • .github/actions/setup-android/action.yml
  • .github/workflows/unit-test.yml
📝 Walkthrough

Walkthrough

This change refactors GitHub Actions CI workflows by extracting common Android environment setup (JDK, SDK, Gradle caches, local.properties) into a reusable composite action, then integrating a dedicated unit-test workflow that runs tests with Roborazzi screenshot testing and automatic artifact/comment reporting.

Changes

Cohort / File(s) Summary
Android Environment Setup
.github/actions/setup-android/action.yml
New composite action that installs JDK 21, configures Android SDK, sets up Gradle caches, generates local.properties from environment, and makes gradlew executable.
CI Workflows
.github/workflows/android-ci.yml, .github/workflows/unit-test.yml
Refactored android-ci.yml to replace inline setup logic with the new setup-android action and removed unit test job. Added new unit-test.yml workflow that runs tests via testDebugUnitTest, uploads test/Roborazzi artifacts, and posts automated Roborazzi comparison comments on pull requests.

Sequence Diagram

sequenceDiagram
    actor PR as Pull Request Event
    participant GH as GitHub Actions
    participant Checkout as Checkout Code
    participant SetupAndroid as Setup Android Action
    participant Gradle as Gradle Test Task
    participant Artifacts as Artifact Upload
    participant RoboComm as Roborazzi Comment Action
    
    PR->>GH: Trigger unit-test workflow
    GH->>Checkout: Clone repository
    Checkout-->>GH: Repository ready
    GH->>SetupAndroid: Execute setup-android action
    SetupAndroid->>SetupAndroid: Install JDK 21
    SetupAndroid->>SetupAndroid: Setup Android SDK
    SetupAndroid->>SetupAndroid: Configure Gradle caches
    SetupAndroid->>SetupAndroid: Generate local.properties
    SetupAndroid-->>GH: Environment configured
    GH->>Gradle: Run testDebugUnitTest
    Gradle->>Gradle: Execute unit tests
    Gradle-->>GH: Test results & Roborazzi screenshots
    GH->>Artifacts: Upload JUnit reports & screenshots
    Artifacts-->>GH: Artifacts stored (7-day retention)
    GH->>RoboComm: Post Roborazzi report comment
    RoboComm->>PR: Comment with comparison link
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops of joy for cleaner pipelines!
Setup actions reused, no more duplicate lines,
Roborazzi screenshots dancing in place,
Screenshots compared with automated grace,
CI workflows nimble, modular, and fine! 📸✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Infra/ci refactoring' is generic and does not clearly specify the main changes: reusable Android setup action, separate unit-test workflow, and Roborazzi integration. Consider a more descriptive title like 'Extract Android setup action and refactor unit tests with Roborazzi integration' to clearly communicate the primary changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The pull request implements the primary objectives from issue #176: modularizes CI by extracting a reusable Android setup action, separates unit tests into a dedicated workflow, integrates Roborazzi for automated screenshot testing, and verifies build stability through the new workflows.
Out of Scope Changes check ✅ Passed All changes directly support the stated objective of refactoring CI workflows and integrating Roborazzi; no unrelated or out-of-scope modifications were detected in the file changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch infra/ci-refactoring

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/actions/setup-android/action.yml:
- Around line 1-5: The composite action metadata is missing the required
description field; add a top-level description string in action.yml (next to the
existing name: 'Setup Android Environment') to satisfy actionlint and
Marketplace requirements—update the YAML metadata for this composite action so
it includes description in addition to name and runs/steps.
- Around line 24-31: The primary cache key in the "Cache Build Cache" step uses
`${{ github.sha }}` which makes every run create a unique cache and prevents
hits; change the primary `key` to a reusable identifier (for example base it on
`${{ runner.os }}` plus the existing `hashFiles('**/build.gradle*',
'**/gradle-wrapper.properties')` or include `${{ github.ref }}`/branch instead
of `github.sha`) and leave the current `restore-keys` as the fallback so the
cache can be reused across commits/branches; if you intended per-commit
snapshots, gate uploads (saves) to the default branch only (e.g., only save when
on the main branch) to avoid thrashing the repository cache quota.
- Around line 33-36: The "Generate local.properties" step in action.yml inlines
the secret via `${{ env.LOCAL_PROPERTIES }}` which risks shell injection and
quoting issues; update the step to read the environment variable from the shell
instead (use $LOCAL_PROPERTIES inside the run script), piping that value to
base64 -d and writing to ./local.properties, and if you prefer explicitness add
an inputs.local-properties and map it to the step's env so the composite action
doesn't rely on caller env inheritance.

In @.github/workflows/unit-test.yml:
- Around line 44-50: The workflow uses a mutable third-party action reference
takahirom/roborazzi-comment-action@v1 which can be repointed; pin this to a
specific full 40-character commit SHA instead (replace
takahirom/roborazzi-comment-action@v1 with
takahirom/roborazzi-comment-action@<full-40-char-sha>) so the Post Roborazzi
Comment step always runs a fixed commit while preserving the same inputs
(github-token, comment-identifier, report-dir); obtain the canonical commit SHA
from the action's repository and update the uses value accordingly.
- Around line 30-42: The "Post Roborazzi Comment" GitHub Actions step currently
only gates on github.event_name == 'pull_request' so it is skipped when the test
step fails; update the "Post Roborazzi Comment" step to also include if:
always() (e.g., combine with the existing event_name check or add its own if) so
the comment runs regardless of prior step failures while still restricting
execution to pull_request events, ensuring screenshot diff comments are posted
even when ./gradlew testDebugUnitTest fails.
- Around line 10-13: Remove the unnecessary actions: write permission from the
permissions block and leave contents: read and pull-requests: write as they are;
ensure the workflow only requests the minimal scopes by removing the actions:
write key and any references that rely on it, and also verify the step that uses
takahirom/roborazzi-comment-action@v1 actually exists or replace it with a
published action (confirm its permissions needs) and confirm that
actions/upload-artifact@v4 continues to work with the internal artifact token
without actions: write.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c3754423-a502-4b49-8f09-f214500d9607

📥 Commits

Reviewing files that changed from the base of the PR and between cb589ad and 2532847.

📒 Files selected for processing (3)
  • .github/actions/setup-android/action.yml
  • .github/workflows/android-ci.yml
  • .github/workflows/unit-test.yml

Comment thread .github/actions/setup-android/action.yml
Comment on lines +24 to +31
- name: Cache Build Cache
uses: actions/cache@v4
with:
path: ~/.gradle/caches/build-cache-1
key: ${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}-${{ github.sha }}
restore-keys: |
${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}
${{ runner.os }}-build-cache-
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Build-cache primary key includes github.sha — it will never be a cache hit.

Because ${{ github.sha }} is unique per commit, the primary key never matches an existing cache; every run pushes a brand-new cache entry, which bloats the repo's 10 GB cache quota and evicts other caches (including the wrapper cache on Line 20). The restore-keys fallback does give you a warm cache, but you likely want the primary key to be reusable (e.g., keyed by branch or by the same hash set without github.sha) and let restore-keys handle cross-branch fallback.

Suggested key strategy
           path: ~/.gradle/caches/build-cache-1
-          key: ${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}-${{ github.sha }}
+          key: ${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}-${{ github.ref_name }}
           restore-keys: |
             ${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}
             ${{ runner.os }}-build-cache-

If the github.sha suffix was intentional (e.g., to force per-commit snapshots for a save-always strategy), consider gating saves to the default branch only to avoid PR caches churning storage.

📝 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.

Suggested change
- name: Cache Build Cache
uses: actions/cache@v4
with:
path: ~/.gradle/caches/build-cache-1
key: ${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}-${{ github.sha }}
restore-keys: |
${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}
${{ runner.os }}-build-cache-
- name: Cache Build Cache
uses: actions/cache@v4
with:
path: ~/.gradle/caches/build-cache-1
key: ${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}-${{ github.ref_name }}
restore-keys: |
${{ runner.os }}-build-cache-${{ hashFiles('**/build.gradle*', '**/gradle-wrapper.properties') }}
${{ runner.os }}-build-cache-
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/actions/setup-android/action.yml around lines 24 - 31, The primary
cache key in the "Cache Build Cache" step uses `${{ github.sha }}` which makes
every run create a unique cache and prevents hits; change the primary `key` to a
reusable identifier (for example base it on `${{ runner.os }}` plus the existing
`hashFiles('**/build.gradle*', '**/gradle-wrapper.properties')` or include `${{
github.ref }}`/branch instead of `github.sha`) and leave the current
`restore-keys` as the fallback so the cache can be reused across
commits/branches; if you intended per-commit snapshots, gate uploads (saves) to
the default branch only (e.g., only save when on the main branch) to avoid
thrashing the repository cache quota.

Comment on lines +33 to +36
- name: Generate local.properties
shell: bash
run: |
echo '${{ env.LOCAL_PROPERTIES }}' | base64 -d > ./local.properties
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid inlining secret into shell via ${{ }}; pass through env.

Interpolating ${{ env.LOCAL_PROPERTIES }} directly into the shell script is a classic script-injection pattern — any special shell metacharacter in the value (unlikely for base64, but brittle) would be evaluated by bash, not by the echo. Additionally, wrapping a secret in single quotes inside an interpolated line can prematurely close the quote if the value ever contains '. The idiomatic and safer form is to reference the env var from within the shell:

Proposed fix
         - name: Generate local.properties
           shell: bash
           run: |
-              echo '${{ env.LOCAL_PROPERTIES }}' | base64 -d > ./local.properties
+              printf '%s' "$LOCAL_PROPERTIES" | base64 -d > ./local.properties

Note: since this is a composite action, env.LOCAL_PROPERTIES is already available to the step's shell as $LOCAL_PROPERTIES because callers set it via env: at the step that uses the action, and composite steps inherit that env. If you want to be explicit and decoupled from the caller's env propagation behavior, declare it as an inputs.local-properties and map it to the step's env.

📝 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.

Suggested change
- name: Generate local.properties
shell: bash
run: |
echo '${{ env.LOCAL_PROPERTIES }}' | base64 -d > ./local.properties
- name: Generate local.properties
shell: bash
run: |
printf '%s' "$LOCAL_PROPERTIES" | base64 -d > ./local.properties
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/actions/setup-android/action.yml around lines 33 - 36, The "Generate
local.properties" step in action.yml inlines the secret via `${{
env.LOCAL_PROPERTIES }}` which risks shell injection and quoting issues; update
the step to read the environment variable from the shell instead (use
$LOCAL_PROPERTIES inside the run script), piping that value to base64 -d and
writing to ./local.properties, and if you prefer explicitness add an
inputs.local-properties and map it to the step's env so the composite action
doesn't rely on caller env inheritance.

Comment thread .github/workflows/unit-test.yml
Comment on lines +30 to +42
- name: Run Unit Tests and Record Screenshots
id: test
run: ./gradlew testDebugUnitTest --parallel --stacktrace

- name: Upload Test Reports and Screenshots
if: always()
uses: actions/upload-artifact@v4
with:
name: test-reports
path: |
**/build/reports/tests
**/build/outputs/roborazzi
retention-days: 7
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider if: always() on the artifact upload — already covered; but also ensure test failures still surface.

The Upload Test Reports and Screenshots step correctly uses if: always() so artifacts survive test failures — good. However, Post Roborazzi Comment at Line 44 only checks github.event_name == 'pull_request' and not if: always(), so on a failed ./gradlew testDebugUnitTest the job fails at Line 32 and the comment step is skipped — which is likely the opposite of what you want for screenshot-diff review (the comment is most valuable exactly when screenshots changed/failed).

Proposed fix
             - name: Post Roborazzi Comment
-              if: github.event_name == 'pull_request'
+              if: always() && github.event_name == 'pull_request'
               uses: takahirom/roborazzi-comment-action@v1
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/unit-test.yml around lines 30 - 42, The "Post Roborazzi
Comment" GitHub Actions step currently only gates on github.event_name ==
'pull_request' so it is skipped when the test step fails; update the "Post
Roborazzi Comment" step to also include if: always() (e.g., combine with the
existing event_name check or add its own if) so the comment runs regardless of
prior step failures while still restricting execution to pull_request events,
ensuring screenshot diff comments are posted even when ./gradlew
testDebugUnitTest fails.

Comment thread .github/workflows/unit-test.yml Outdated
Comment on lines +44 to +50
- name: Post Roborazzi Comment
if: github.event_name == 'pull_request'
uses: takahirom/roborazzi-comment-action@v1
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-identifier: "roborazzi-report"
report-dir: "**/build/outputs/roborazzi"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Pin third-party action to a commit SHA.

takahirom/roborazzi-comment-action@v1 is a mutable tag from a third-party publisher and receives a GITHUB_TOKEN with pull-requests: write. GitHub's hardening guide for Actions recommends pinning third-party actions to a full-length commit SHA to prevent supply-chain tampering via tag re-pointing. First-party actions/* and android-actions/* usage in this PR is lower risk, but this one writes PR comments on your behalf and is worth pinning.

uses: takahirom/roborazzi-comment-action@<full-40-char-sha>  # v1.x.y
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/unit-test.yml around lines 44 - 50, The workflow uses a
mutable third-party action reference takahirom/roborazzi-comment-action@v1 which
can be repointed; pin this to a specific full 40-character commit SHA instead
(replace takahirom/roborazzi-comment-action@v1 with
takahirom/roborazzi-comment-action@<full-40-char-sha>) so the Post Roborazzi
Comment step always runs a fixed commit while preserving the same inputs
(github-token, comment-identifier, report-dir); obtain the canonical commit SHA
from the action's repository and update the uses value accordingly.

komodgn and others added 2 commits April 18, 2026 14:38
- Add description.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@komodgn komodgn merged commit be76239 into develop Apr 18, 2026
3 checks passed
@komodgn komodgn deleted the infra/ci-refactoring branch April 18, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Chore] Refactor CI workflows and integrate Roborazzi for automated testing

1 participant