Skip to content

ci: flatten device render captures to survive spaces in AGP's output path - #155

Merged
l2hyunwoo merged 1 commit into
mainfrom
ci/device-gallery-whitespace-fix
Jul 11, 2026
Merged

ci: flatten device render captures to survive spaces in AGP's output path#155
l2hyunwoo merged 1 commit into
mainfrom
ci/device-gallery-whitespace-fix

Conversation

@l2hyunwoo

@l2hyunwoo l2hyunwoo commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

The device render gallery never showed up in the PR comment, even when the emulator jobs produced captures. On #152 the "Device render (API 30/34)" jobs passed and uploaded non-empty artifacts, but the comment's "Generate device render gallery" step was skipped.

Root cause

AGP's additionalTestOutputDir pull nests the captures under a device-named subdirectory that contains a space, e.g. androidDeviceTest/connected/emulator-5554 - 11/band_api30_duotone_actual.png. The gallery's safe-set filter, ^[a-zA-Z0-9_./-]+$, rejects any path with a space, so it treats every device capture as untrusted and silently drops it. Goldens and diff images sit in a flat, space-free path, so they were unaffected, which is why the goldens gallery has been showing up fine while the device gallery stayed empty.

Fix

Flatten the device captures into a space-free directory before filtering: find device-screenshots -name "*.png" -exec cp {} device-flat/ \;, then run the existing check/push/gallery filters against that flat tree. A flat, space-free path keeps the blob URL, the push step, and the check step all consistent with the filters already in place, and the original subdirectory carries no information the gallery uses (it only parses the band_api<SDK>_<case>_<kind> basename).

Verified against a reproduction of the actual artifact tree from the bug report, plus an adversarial case (a space inside the filename itself, not just the directory), which is still rejected.

Summary by CodeRabbit

  • Bug Fixes
    • Improved device screenshot comparison and publishing workflows to reliably process screenshots with spaces in their names.
    • Updated device render galleries and branch previews to include valid screenshots consistently.
    • Prevented unrelated or unsafe image files from being included in generated comparisons and previews.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9786214d-3de6-47b6-83b3-f888c138c363

📥 Commits

Reviewing files that changed from the base of the PR and between f54a327 and 8f57fc7.

📒 Files selected for processing (1)
  • .github/workflows/screenshot-comment.yml

📝 Walkthrough

Walkthrough

The screenshot workflow now flattens device captures into device-flat/, applies basename-only safe-set checks, stages space-free images, and generates the device gallery from the flattened directory.

Changes

Screenshot workflow

Layer / File(s) Summary
Flatten and publish device captures
.github/workflows/screenshot-comment.yml
Device PNGs are copied into device-flat/, validated by basename, and staged from space-free sources for companion-branch publishing.
Use flattened captures in the device gallery
.github/workflows/screenshot-comment.yml
The gallery parses safe basenames and enumerates PNGs from device-flat/.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: skydoves

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main CI change: flattening device render captures to handle spaces in AGP output paths.
Description check ✅ Passed The description explains the problem, root cause, fix, and verification, so it covers the key template content despite omitting boilerplate sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/device-gallery-whitespace-fix

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.

@github-actions

Copy link
Copy Markdown

Snapshot diff report

✅ 8 screenshots verified, no changes.

Total Unchanged Changed Added
8 8 0 0

Full screenshot report (artifact)

Screenshots

Committed reference screenshots each spec verifies against (current expected state).

File name Image
passthrough_radius8.png
smoke_radius0.png
passthrough_radius16.png
smoke_radius24.png
state_disabled.png
state_enabled.png
passthrough_radius0.png
passthrough_radius24.png

@l2hyunwoo
l2hyunwoo marked this pull request as ready for review July 11, 2026 15:04
@l2hyunwoo
l2hyunwoo requested a review from skydoves as a code owner July 11, 2026 15:04
@l2hyunwoo
l2hyunwoo merged commit 53baa89 into main Jul 11, 2026
4 of 5 checks passed
@l2hyunwoo
l2hyunwoo deleted the ci/device-gallery-whitespace-fix branch July 11, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant