Skip to content

Infra/ci refactoring #177

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.

Sign up for GitHub

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

Merged
komodgn merged 5 commits into from
Apr 18, 2026
Merged

Infra/ci refactoring #177

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b69e08
refactor(ci): Create custom action for Android setup and reduce redun…
komodgn Apr 18, 2026
e6de466
refactor(ci): Separate unit tests into new workflow and add Roborazzi
komodgn Apr 18, 2026
2532847
build(ci): Add skip-ci condition to unit-test workflow
komodgn Apr 18, 2026
ab882e7
Update .github/actions/setup-android/action.yml
komodgn Apr 18, 2026
315f17d
refactor(ci): Remove invalid comment action and focus on artifact upl…
komodgn Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .github/actions/setup-android/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: 'Setup Android Environment'

runs:
using: "composite"
steps:
Comment thread
komodgn marked this conversation as resolved.
- name: Setup JDK 21
uses: actions/setup-java@v4
with:
java-version: '21'
distribution: 'temurin'
cache: gradle

- name: Setup Android SDK
uses: android-actions/setup-android@v3

- name: Cache Gradle Wrapper
uses: actions/cache@v4
with:
path: ~/.gradle/wrapper
key: ${{ runner.os }}-gradle-wrapper-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}
restore-keys: |
${{ runner.os }}-gradle-wrapper-

- 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-
Comment on lines +25 to +32
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 18, 2026

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.


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

@coderabbitai coderabbitai bot Apr 18, 2026

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.


- name: Grant execute permission for gradlew
shell: bash
run: chmod +x gradlew
49 changes: 4 additions & 45 deletions .github/workflows/android-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,10 @@ jobs:
- name: Checkout Repository
uses: actions/checkout@v4

- name: Setup JDK 21
uses: actions/setup-java@v4
with:
java-version: '21'
distribution: 'temurin'
cache: gradle

- name: Setup Android SDK
uses: android-actions/setup-android@v2

- name: Cache Gradle Wrapper
uses: actions/cache@v4
with:
path: ~/.gradle/wrapper
key: ${{ runner.os }}-gradle-wrapper-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}
restore-keys: |
${{ runner.os }}-gradle-wrapper-

- 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: Generate local.properties
run: echo '${{ secrets.LOCAL_PROPERTIES }}' | base64 -d > ./local.properties

- name: Grant execute permission for gradlew
run: chmod +x gradlew
- name: Setup
uses: ./.github/actions/setup-android
env:
LOCAL_PROPERTIES: ${{ secrets.LOCAL_PROPERTIES }}

- name: Code Style Check
id: ktlint
Expand All @@ -61,14 +33,6 @@ jobs:
end=$(date +%s)
echo "time=$((end-start))" >> $GITHUB_OUTPUT

- name: Unit Test
id: test
run: |
start=$(date +%s)
./gradlew testDebugUnitTest --parallel
end=$(date +%s)
echo "time=$((end-start))" >> $GITHUB_OUTPUT

- name: Debug Build with Gradle
id: assemble
run: |
Expand All @@ -91,11 +55,6 @@ jobs:
with:
script: |
const results = {
test: {
status: '${{ steps.test.outcome }}',
name: 'Unit Test',
time: '${{ steps.test.outputs.time }}'
},
assemble: {
status: '${{ steps.assemble.outcome }}',
name: 'Debug Build',
Expand Down
50 changes: 50 additions & 0 deletions .github/workflows/unit-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: unit-test

on:
pull_request:
paths-ignore:
- '**.md'

run-name: "unit-test by ${{ github.actor }}"

permissions:
contents: read
pull-requests: write
actions: write
Comment thread
coderabbitai[bot] marked this conversation as resolved.

jobs:
unit-test:
runs-on: ubuntu-latest
if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-ci') }}
timeout-minutes: 30

steps:
- name: Checkout Repository
uses: actions/checkout@v4

- name: Setup
uses: ./.github/actions/setup-android
env:
LOCAL_PROPERTIES: ${{ secrets.LOCAL_PROPERTIES }}

- 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
Comment on lines +30 to +42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 18, 2026

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.


- 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

@coderabbitai coderabbitai bot Apr 18, 2026

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.