test: Increase mock photo count to validate Roborazzi diff pipeline#183
Merged
test: Increase mock photo count to validate Roborazzi diff pipeline#183
Conversation
Contributor
📝 WalkthroughWalkthroughExpanded person mock photo URIs from 10 to 28 and removed a GitHub Actions workflow for screenshot-diff reporting; also deleted one NLSearch screenshot test. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner as GitHub Actions Runner
participant Artifacts as Artifact Storage
participant Repo as Repository
participant Shell as Workflow Shell
Note over Runner,Artifacts: (Previous workflow behavior)
Runner->>Artifacts: download `screenshot-diff-reports` artifact
Runner->>Shell: read PR number from `diff-reports/*/NR`
Runner->>Repo: checkout repository (default branch)
Shell->>Shell: find `*_compare.png` files → set exists flag
alt PNGs found
Shell->>Repo: create/checkout `companion_<head>` orphan branch
Shell->>Repo: copy PNGs → commit → push
Shell->>Runner: generate markdown with image links
Runner->>Repo: post or replace PR comment with report
else no PNGs
Shell->>Runner: skip report/comment steps
end
Shell->>Repo: cleanup old `companion_` branches (>30 days)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/screenshot-comparison-comment.yml (1)
81-87: Prefer a glob overls | grepto enumerate_compare.pngfiles.Parsing
lsis fragile (breaks on unusual characters, SC2012) and thels *.png 1>/dev/nullguard can be folded in viashopt -s nullglob.♻️ Suggested refactor
- if ls *.png 1> /dev/null 2>&1; then - for file in $(ls *.png | grep _compare.png); do - URL="https://github.com/${REPO}/blob/${BRANCH}/${file}?raw=true" - DISPLAY_NAME=$(echo $file | sed -r 's/(.{20})/\1<br>/g') - echo "| $DISPLAY_NAME |  |" >> $GITHUB_OUTPUT - done - fi + shopt -s nullglob + for file in *_compare.png; do + URL="https://github.com/${REPO}/blob/${BRANCH}/${file}?raw=true" + DISPLAY_NAME=$(printf '%s' "$file" | sed -r 's/(.{20})/\1<br>/g') + echo "| $DISPLAY_NAME |  |" >> "$GITHUB_OUTPUT" + done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/screenshot-comparison-comment.yml around lines 81 - 87, Replace the fragile ls/grep enumeration with a shell glob using shopt -s nullglob so the guard is folded in; specifically, in the block that currently tests "if ls *.png ..." and loops "for file in $(ls *.png | grep _compare.png); do", enable nullglob, iterate "for file in *_compare.png; do" and remove the ls-based existence check, then reuse REPO, BRANCH, DISPLAY_NAME and GITHUB_OUTPUT as before to build URL and append the row; ensure you unset or restore nullglob if needed after the loop.
🤖 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/workflows/screenshot-comparison-comment.yml:
- Around line 49-66: The "Push to Companion Branch" step flattens PNG/NR files
into the repo root causing name collisions, noisy self-copies, and duplicates
the original diff-reports/ when committing; change the copy logic to only search
./diff-reports (avoid find .) and preserve namespace (e.g., map the path under
diff-reports/ into a unique filename or directory by replacing '/' with '_' or
mirroring the folder structure) so basename collisions cannot overwrite files,
and after copying either remove diff-reports/ (rm -rf diff-reports) before
running git add/commit or avoid flattening entirely and commit directly from
diff-reports/ (drop the flatten step); keep BRANCH_NAME handling the same and
ensure the step no longer triggers cp self-copy warnings by scoping find to
./diff-reports.
- Around line 26-28: The current shell step concatenates all matching "NR" files
into PR_NUMBER (using find ... -exec cat {} \;), which yields invalid/ambiguous
numbers when multiple NR markers exist; change the logic so you choose exactly
one NR file (e.g., use find to locate a single file with -print -quit or list
matches and error if count != 1) and fail the job if none are found; update the
step that sets PR_NUMBER (the variable PR_NUMBER and the find invocation) to
explicitly pick one NR file or bail with a clear error message when zero or
multiple matches are detected.
---
Nitpick comments:
In @.github/workflows/screenshot-comparison-comment.yml:
- Around line 81-87: Replace the fragile ls/grep enumeration with a shell glob
using shopt -s nullglob so the guard is folded in; specifically, in the block
that currently tests "if ls *.png ..." and loops "for file in $(ls *.png | grep
_compare.png); do", enable nullglob, iterate "for file in *_compare.png; do" and
remove the ls-based existence check, then reuse REPO, BRANCH, DISPLAY_NAME and
GITHUB_OUTPUT as before to build URL and append the row; ensure you unset or
restore nullglob if needed after the loop.
🪄 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: 11610669-8168-4155-9b9d-211ab0dfaba9
📒 Files selected for processing (1)
.github/workflows/screenshot-comparison-comment.yml
Roborazzi Snapshot Diff Report
|
- screenshot-comparison-comment.yml - NLSearchScreenshotTest.kt
komodgn
force-pushed
the
test/comment-bot
branch
from
d821e96 to
cdc959e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
Work Description ✏️
Note
The workflow downloads baseline images from the develop branch for comparison. Since develop still contains the old
NLSearchScreenshotTestimages, they are fetched as reference files. These will disappear once this PR is merged and the develop baseline is updated.Unexpected Bulk File Additions
Screenshot 📸
Summary by CodeRabbit
Tests
Chores
No user-facing functionality changed.