diff --git a/.github/workflows/screenshot-comment.yml b/.github/workflows/screenshot-comment.yml index 7f65dd8c..925243ad 100644 --- a/.github/workflows/screenshot-comment.yml +++ b/.github/workflows/screenshot-comment.yml @@ -84,45 +84,100 @@ jobs: run-id: ${{ github.event.workflow_run.id }} github-token: ${{ github.token }} - # --- Are there any valid *_compare.png diff images? --- + # Instrumented render captures from the device-screenshots matrix (screenshot-test.yml). One + # artifact per API band; `pattern` + `merge-multiple` folds both into ./device-screenshots. + # continue-on-error: the emulator job may not have run (older producer) or a band may have + # produced nothing -- a missing/empty artifact must NOT fail this job. The device gallery step + # below simply renders no rows when the dir is empty. + - name: Download device screenshot artifacts + continue-on-error: true + uses: actions/download-artifact@v4 + with: + pattern: device-screenshots-api* + merge-multiple: true + path: device-screenshots + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + + # The committed goldens, uploaded on every run. continue-on-error: an older producer run (before + # this artifact existed) or a download hiccup must NOT fail the job -- the goldens gallery step + # below simply renders nothing when the dir is empty. + - name: Download screenshot goldens artifact + continue-on-error: true + uses: actions/download-artifact@v4 + with: + name: screenshot-goldens + path: goldens + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + + # --- Is there anything to push to the companion branch? --- + # `diff` = valid *_compare.png Roborazzi diff images exist (a regression). + # `device` = instrumented render captures exist (present on every emulator run, not just diffs). + # `golden` = committed goldens downloaded (present on every run -> gallery is always-on). + # `any` = any of the three -- the push + companion branch run when ANY is true. - id: check - name: Check for diff images + name: Check for images to publish shell: bash run: | - mapfile -t files < <(find . -type f -name "*_compare.png") - exist="false" - for f in "${files[@]}"; do + # Roborazzi diff images (only exist on a mismatch). + diff="false" + while IFS= read -r f; do # Reject any path with characters outside the safe set before we touch git / build URLs. - if [[ "$f" =~ ^[a-zA-Z0-9_./-]+$ ]]; then - exist="true" - break - fi - done - echo "exist=$exist" >> "$GITHUB_OUTPUT" + [[ "$f" =~ ^[a-zA-Z0-9_./-]+$ ]] && { diff="true"; break; } + done < <(find . -type f -name "*_compare.png") + + # Device render captures (band_api__{actual,expected}.png) under device-screenshots/. + device="false" + if [[ -d device-screenshots ]]; then + while IFS= read -r f; do + [[ "$f" =~ ^[a-zA-Z0-9_./-]+$ ]] && { device="true"; break; } + done < <(find device-screenshots -type f -name "*.png") + fi - # --- Push diff images to an orphan companion branch (only when there are diffs) --- + # Committed goldens under goldens/. + golden="false" + if [[ -d goldens ]]; then + while IFS= read -r f; do + [[ "$f" =~ ^[a-zA-Z0-9_./-]+$ ]] && { golden="true"; break; } + done < <(find goldens -type f -name "*.png") + fi + + any="false" + [[ "$diff" == "true" || "$device" == "true" || "$golden" == "true" ]] && any="true" + { + echo "diff=$diff" + echo "device=$device" + echo "golden=$golden" + echo "any=$any" + } >> "$GITHUB_OUTPUT" + + # --- Push images to an orphan companion branch (diff images and/or device captures) --- - id: push - name: Push diffs to companion branch - if: steps.check.outputs.exist == 'true' + name: Push images to companion branch + if: steps.check.outputs.any == 'true' shell: bash env: # Keyed by PR number (integer-guarded above), not head_branch -- two fork PRs can share a # branch name and would otherwise overwrite each other's companion branch / gallery. BRANCH_NAME: companion_pr-${{ steps.pr.outputs.number }} run: | - # Orphan branch: no history, just the diff images for this PR head. + # Orphan branch: no history, just the images for this PR head. git branch -D "$BRANCH_NAME" || true git checkout --orphan "$BRANCH_NAME" git rm -rf . > /dev/null - for f in $(find . -type f -name "*_compare.png"); do + # Roborazzi diff images (roborazzi/...) + device render captures (device-screenshots/...) + + # committed goldens (goldens/...). Same safe-set path filter as everywhere else before + # staging fork-run-produced files. + while IFS= read -r f; do if [[ "$f" =~ ^[a-zA-Z0-9_./-]+$ ]]; then git add "$f" fi - done + done < <(find . -type f \( -name "*_compare.png" -o -path "./device-screenshots/*.png" -o -path "./goldens/*.png" \)) git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git commit -m "Add screenshot diff for PR #${{ steps.pr.outputs.number }}" + git commit -m "Add screenshots for PR #${{ steps.pr.outputs.number }}" git push origin "HEAD:$BRANCH_NAME" -f # --- Summary report (ALWAYS): parse results-summary.json into a counts table + verdict line --- @@ -138,7 +193,10 @@ jobs: # unzipped HTML cannot be deep-linked; the run page exposes the artifact downloads). RUN_URL: https://github.com/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }} run: | - summary_json="$(find screenshot-summary -type f -name 'results-summary.json' 2>/dev/null | head -n1)" + # `|| true`: the artifact is absent whenever the producing run failed before writing the + # summary; under `set -e -o pipefail` a bare failing find kills this step before the + # "summary unavailable" fallback below can run. + summary_json="$(find screenshot-summary -type f -name 'results-summary.json' 2>/dev/null | head -n1 || true)" delimiter="$(openssl rand -hex 8)" { echo "body<<${delimiter}" @@ -186,12 +244,44 @@ jobs: } >> "$GITHUB_OUTPUT" echo "${delimiter}" >> "$GITHUB_OUTPUT" + # --- Goldens gallery (ALWAYS, whenever goldens were downloaded): the committed reference PNGs each + # spec asserts against, inlined from the companion branch. This is NOT a diff -- it shows the + # current expected state so a reviewer sees the actual screenshots in the PR even on a clean pass + # (the whole point of this section). goldens/ came from our own upload-artifact, but apply the + # same ^[a-zA-Z0-9_./-]+$ safe-set filter as the other galleries before building any URL. + - id: goldens + name: Generate goldens gallery + if: steps.check.outputs.golden == 'true' + shell: bash + env: + # Must match the push step's branch key (PR number, not head_branch). + BRANCH_NAME: companion_pr-${{ steps.pr.outputs.number }} + run: | + files=$(find goldens -type f -name "*.png" | grep -E "^[a-zA-Z0-9_./-]+$" || true) + delimiter="$(openssl rand -hex 8)" + { + echo "body<<${delimiter}" + echo "" + echo "### Screenshots" + echo "" + echo "Committed reference screenshots each spec verifies against (current expected state)." + echo "" + echo "| File name | Image |" + echo "|-----------|-------|" + } >> "$GITHUB_OUTPUT" + for f in $files; do + name=$(basename "$f") + url="https://github.com/${{ github.repository }}/blob/$BRANCH_NAME/$f" + echo "| \`$name\` | ![]($url?raw=true) |" >> "$GITHUB_OUTPUT" + done + echo "${delimiter}" >> "$GITHUB_OUTPUT" + # --- Gallery (ONLY when there are diffs): inline before/after/diff images from companion branch. # Emits a markdown fragment that the comment step appends below the summary, so a regression # comment shows BOTH the counts AND the images in one sticky comment. - id: gallery name: Generate diff gallery - if: steps.check.outputs.exist == 'true' + if: steps.check.outputs.diff == 'true' shell: bash env: # Must match the push step's branch key (PR number, not head_branch). @@ -219,6 +309,77 @@ jobs: done echo "${delimiter}" >> "$GITHUB_OUTPUT" + # --- Device render gallery (ALWAYS, whenever captures exist): the actual pixels each API band + # drew on the emulator. This is NOT a regression diff -- it is render evidence, so it shows on + # every run that produced captures, independent of the Roborazzi diff above. One row per (band, + # case): actual, and expected beside it if the spec wrote one. Filenames are + # band_api__{actual,expected}.png (see MirageBandScreenshotTest); they arrive from a + # fork-triggered run's artifact, so parse them as UNTRUSTED -- same ^[a-zA-Z0-9_./-]+$ safe-set + # filter as the diff gallery, and pull SDK/case/kind out with pure bash (no eval). + - id: device_gallery + name: Generate device render gallery + if: steps.check.outputs.device == 'true' + shell: bash + env: + # Must match the push step's branch key (PR number, not head_branch). + BRANCH_NAME: companion_pr-${{ steps.pr.outputs.number }} + run: | + delimiter="$(openssl rand -hex 8)" + { + echo "body<<${delimiter}" + echo "" + echo "### Device rendering" + echo "" + echo "Actual pixels captured on the emulator per API band (API 30 = GLES mirage + legacy blur, API 34 = AGSL + RenderEffect). Render evidence, not a golden diff." + echo "" + echo "| Band | Case | Actual | Expected |" + echo "|------|------|--------|----------|" + } >> "$GITHUB_OUTPUT" + + # Collect one logical row per api+case, then emit actual|expected columns for each. + # Assoc arrays keyed by "|"; value is the safe repo-relative path to that PNG. + declare -A actual expected + declare -A seen + while IFS= read -r f; do + # Safe-set gate FIRST (path is fork-run-produced). Skip anything with odd chars. + [[ "$f" =~ ^[a-zA-Z0-9_./-]+$ ]] || continue + bn=$(basename "$f" .png) + # Expect band_api__. Enforce the shape with a regex; ignore non-matching. + [[ "$bn" =~ ^band_api([0-9]+)_(.+)_(actual|expected)$ ]] || continue + api="${BASH_REMATCH[1]}" + case="${BASH_REMATCH[2]}" + kind="${BASH_REMATCH[3]}" + key="${api}|${case}" + seen["$key"]=1 + if [[ "$kind" == "actual" ]]; then + actual["$key"]="$f" + else + expected["$key"]="$f" + fi + done < <(find device-screenshots -type f -name "*.png") + + # Stable order: sort the composite keys (api numeric-ish then case). + for key in $(printf '%s\n' "${!seen[@]}" | sort); do + api="${key%%|*}" + case="${key#*|}" + a="${actual[$key]:-}" + e="${expected[$key]:-}" + if [[ -n "$a" ]]; then + aurl="https://github.com/${{ github.repository }}/blob/$BRANCH_NAME/$a?raw=true" + acell="![]($aurl)" + else + acell="_(none)_" + fi + if [[ -n "$e" ]]; then + eurl="https://github.com/${{ github.repository }}/blob/$BRANCH_NAME/$e?raw=true" + ecell="![]($eurl)" + else + ecell="_(none)_" + fi + echo "| API ${api} | \`${case}\` | ${acell} | ${ecell} |" >> "$GITHUB_OUTPUT" + done + echo "${delimiter}" >> "$GITHUB_OUTPUT" + # --- Sticky comment: find the bot's previous comment (if any) by marker text --- - name: Find existing comment uses: peter-evans/find-comment@v3 @@ -228,9 +389,10 @@ jobs: comment-author: 'github-actions[bot]' body-includes: '' - # --- Single sticky comment, ALWAYS posted on pull_request runs: summary + optional gallery. - # steps.gallery.outputs.body is empty on clean runs (the step was skipped), so the comment is - # just the summary; on a regression it appends the inline image gallery. + # --- Single sticky comment, ALWAYS posted on pull_request runs: summary + optional galleries. + # Order: summary -> goldens (always, the committed reference shots) -> diff gallery (regression + # only) -> device gallery (emulator captures only). Each gallery output is empty when its step + # was skipped, so the comment degrades to just the summary and whatever galleries have content. - name: Comment screenshot report uses: peter-evans/create-or-update-comment@v4 with: @@ -240,7 +402,9 @@ jobs: body: | ${{ steps.summary.outputs.body }} + ${{ steps.goldens.outputs.body }} ${{ steps.gallery.outputs.body }} + ${{ steps.device_gallery.outputs.body }} # --- Housekeeping: prune companion_* branches older than 30 days so they don't pile up --- - name: Cleanup outdated companion branches diff --git a/.github/workflows/screenshot-test.yml b/.github/workflows/screenshot-test.yml index 39e47c4b..446365d4 100644 --- a/.github/workflows/screenshot-test.yml +++ b/.github/workflows/screenshot-test.yml @@ -91,7 +91,23 @@ jobs: uses: actions/upload-artifact@v4 with: name: screenshot-summary - path: cloudy/build/test-results/roborazzi/debug/results-summary.json + # The KMP android host test writes under .../roborazzi/androidHostTest/ (not .../debug/); + # glob the variant segment so a task rename cannot silently drop the artifact again. + path: cloudy/build/test-results/roborazzi/**/results-summary.json + if-no-files-found: warn + retention-days: 7 + + # The committed goldens themselves (checked in under androidHostTest/assets/screenshots/). verify + # only writes PNGs to build/ on a mismatch, so a clean PR had no images to show; uploading the + # goldens gives the comment workflow an ALWAYS-ON "Screenshots" gallery -- the current state each + # spec asserts against, visible in the PR regardless of diff. Just checked-out files (~8 small + # PNGs), so this is a plain copy with no extra Gradle task. + - name: Upload screenshot goldens + if: always() + uses: actions/upload-artifact@v4 + with: + name: screenshot-goldens + path: cloudy/src/androidHostTest/assets/screenshots/*.png if-no-files-found: warn retention-days: 7