From bb81c9c9bb14e98b3abdcbc415ba07db12def05e Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Mon, 4 May 2026 16:22:15 +0200 Subject: [PATCH] Improves screenshot experience for forks --- .github/workflows/screenshot-test.yml | 174 +++++++++++++------------- 1 file changed, 88 insertions(+), 86 deletions(-) diff --git a/.github/workflows/screenshot-test.yml b/.github/workflows/screenshot-test.yml index 937f95e7005..921f1ff85ed 100644 --- a/.github/workflows/screenshot-test.yml +++ b/.github/workflows/screenshot-test.yml @@ -22,7 +22,6 @@ jobs: screenshots: name: Checking Component Screenshots runs-on: ubuntu-latest - continue-on-error: true # TODO(hediet): Remove once screenshot pipeline is stable steps: - name: Checkout uses: actions/checkout@v6 @@ -65,7 +64,6 @@ jobs: run: ./node_modules/.bin/component-explorer render --project ./test/componentFixtures/component-explorer.json - name: Log fixture errors - if: always() run: | MANIFEST="test/componentFixtures/.screenshots/current/manifest.json" if [ ! -f "$MANIFEST" ]; then @@ -78,7 +76,9 @@ jobs: if (!errs.length) { console.log('No fixture errors.'); process.exit(0); } console.log(errs.length + ' fixture(s) with errors:'); for (const f of errs) { - console.log('::error::' + f.fixtureId + ': ' + (f.error || 'unknown error')); + const msg = f.error?.message || (typeof f.error === 'string' ? f.error : null) || 'unknown error'; + console.log('::error::' + f.fixtureId + ': ' + msg); + if (f.error?.stack) { console.log(' stack: ' + f.error.stack); } if (f.events?.length) { for (const e of f.events) { console.log(' event: ' + JSON.stringify(e)); } } @@ -88,6 +88,12 @@ jobs: - name: Check blocks-ci screenshots id: blocks-ci + # The service URL is baked into the committed blocks-ci-screenshots.md + # as `![screenshot](/images/)` (see + # build/lib/screenshotBlocksCi.ts). It is passed here so the script + # regenerates the markdown using the same URL prefix; otherwise the + # equality check against the committed file would always fail. No HTTP + # call is made — the URL is purely a string written into the markdown. run: | node build/lib/screenshotBlocksCi.ts \ test/componentFixtures/.screenshots/current/manifest.json \ @@ -104,41 +110,14 @@ jobs: echo "BLOCKS_CI_EOF" >> "$GITHUB_OUTPUT" } - - name: Upload screenshots + - name: Upload screenshots as artifact uses: actions/upload-artifact@v7 with: name: screenshots path: test/componentFixtures/.screenshots/current/ - - name: Skip notice (fork PR) - if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository - run: echo "::notice::Skipping screenshot upload and diff — OIDC is not available for fork PRs" - - - name: Get OIDC token - id: oidc - if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository - run: | - TOKEN=$(curl -sS -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ - "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=https://hediet-screenshots.azurewebsites.net" \ - | jq -r .value) - echo "::add-mask::$TOKEN" - echo "token=$TOKEN" >> "$GITHUB_OUTPUT" - - - name: Upload screenshots to service - if: steps.oidc.outputs.token - run: | - cd test/componentFixtures/.screenshots/current - zip -qr "$GITHUB_WORKSPACE/screenshots.zip" . - curl -sS -f -X POST "https://hediet-screenshots.azurewebsites.net/upload" \ - -H "Content-Type: application/zip" \ - -H "Authorization: Bearer $SCREENSHOT_SERVICE_TOKEN" \ - --data-binary @"$GITHUB_WORKSPACE/screenshots.zip" - env: - SCREENSHOT_SERVICE_TOKEN: ${{ steps.oidc.outputs.token }} - - - name: Diff screenshots against merge base - id: diff - if: steps.oidc.outputs.token + - name: Determine base SHA + id: base run: | if [ "${{ github.event_name }}" = "pull_request" ]; then # For PRs, diff against the merge-base with the target branch. @@ -155,12 +134,79 @@ jobs: fi echo "base_sha=$BASE_SHA" >> "$GITHUB_OUTPUT" echo "Using base SHA: $BASE_SHA (base for ${{ github.sha }})" + + - name: Get OIDC token (non-fork only) + id: oidc + if: github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) + run: | + TOKEN=$(curl -sS -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ + "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=https://hediet-screenshots.azurewebsites.net" \ + | jq -r .value) + echo "::add-mask::$TOKEN" + echo "token=$TOKEN" >> "$GITHUB_OUTPUT" + + - name: Upload screenshots to service + if: steps.oidc.outputs.token + run: | + cd test/componentFixtures/.screenshots/current + zip -qr "$GITHUB_WORKSPACE/screenshots.zip" . + # Tolerate service outages: warn instead of failing the step so the + # rest of the workflow (and CI) is not blocked when the screenshot + # service is unreachable. + set +e + STATUS=$(curl -sS -o /tmp/screenshot-upload.out -w '%{http_code}' \ + -X POST "https://hediet-screenshots.azurewebsites.net/upload" \ + -H "Content-Type: application/zip" \ + -H "Authorization: Bearer $SCREENSHOT_SERVICE_TOKEN" \ + --data-binary @"$GITHUB_WORKSPACE/screenshots.zip") + CURL_EXIT=$? + set -e + if [ "$CURL_EXIT" -ne 0 ]; then + echo "::warning::Screenshot service unreachable (curl exit $CURL_EXIT) -- skipping upload." + elif [ "${STATUS:0:1}" != "2" ]; then + echo "::warning::Screenshot service returned HTTP $STATUS -- skipping upload." + cat /tmp/screenshot-upload.out || true + else + echo "Uploaded screenshots (HTTP $STATUS)." + fi + env: + SCREENSHOT_SERVICE_TOKEN: ${{ steps.oidc.outputs.token }} + + - name: Fetch base commit manifest + id: base_manifest + env: + BASE_SHA: ${{ steps.base.outputs.base_sha }} + run: | + BASE_MANIFEST=/tmp/base-manifest.json + URL="https://hediet-screenshots.azurewebsites.net/commits/${{ github.repository_owner }}/${{ github.event.repository.name }}/$BASE_SHA" + # Tolerate service outages: bash `set -e` would otherwise abort this + # step if curl can't connect (since the `STATUS=$(...)` substitution + # propagates the non-zero exit). Capture the exit code explicitly + # and emit a warning instead. + set +e + STATUS=$(curl -sS -o "$BASE_MANIFEST" -w '%{http_code}' "$URL") + CURL_EXIT=$? + set -e + if [ "$CURL_EXIT" -ne 0 ]; then + echo "::warning::Screenshot service unreachable (curl exit $CURL_EXIT) -- skipping diff." + elif [ "$STATUS" = "200" ]; then + echo "path=$BASE_MANIFEST" >> "$GITHUB_OUTPUT" + echo "Fetched base manifest for $BASE_SHA" + else + echo "::warning::Base manifest unavailable (HTTP $STATUS) -- skipping diff." + cat "$BASE_MANIFEST" || true + fi + + - name: Diff screenshots + id: diff + if: steps.base_manifest.outputs.path + run: | BODY=$(node build/lib/screenshotDiffReport.ts \ https://hediet-screenshots.azurewebsites.net \ - ${{ github.repository_owner }} \ - ${{ github.event.repository.name }} \ - "$BASE_SHA" \ - ${{ github.sha }}) + "${{ steps.base.outputs.base_sha }}" \ + ${{ github.sha }} \ + "${{ steps.base_manifest.outputs.path }}" \ + test/componentFixtures/.screenshots/current/manifest.json) if [ -n "$BODY" ]; then echo "has_changes=true" >> "$GITHUB_OUTPUT" echo "body<> "$GITHUB_OUTPUT" @@ -170,13 +216,11 @@ jobs: echo "No screenshot changes to report." fi if [ -f .tmp/screenshotDiffReport.json ]; then - echo "::group::Compare response JSON" + echo "::group::Diff JSON" cat .tmp/screenshotDiffReport.json echo "::endgroup::" fi continue-on-error: true - env: - SCREENSHOT_SERVICE_TOKEN: ${{ steps.oidc.outputs.token }} - name: Write job summary if: steps.diff.outputs.has_changes == 'true' || steps.blocks-ci.outputs.match == 'false' @@ -195,8 +239,8 @@ jobs: COMMENT_BODY: ${{ steps.diff.outputs.body }} BLOCKS_CI_CONTENT: ${{ steps.blocks-ci.outputs.content }} - - name: Post PR comment - if: github.event_name == 'pull_request' + - name: Post PR comment (non-fork PR only) + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository uses: actions/github-script@v9 with: script: | @@ -262,26 +306,18 @@ jobs: env: COMMENT_BODY: ${{ steps.diff.outputs.body }} BLOCKS_CI_CONTENT: ${{ steps.blocks-ci.outputs.content }} - BASE_SHA: ${{ steps.diff.outputs.base_sha }} + BASE_SHA: ${{ steps.base.outputs.base_sha }} CURRENT_SHA: ${{ github.sha }} - name: Fail if blocks-ci hashes changed if: steps.blocks-ci.outputs.match == 'false' run: | - echo "::error::blocks-ci screenshot hashes do not match committed file. See PR comment for updated content." + echo "::error::blocks-ci screenshot hashes do not match committed file. See PR comment or job summary for the updated content." echo "" echo "Diff between committed and expected blocks-ci-screenshots.md:" diff -u test/componentFixtures/blocks-ci-screenshots.md /tmp/blocks-ci-updated.md || true exit 1 - # - name: Compare screenshots - # id: compare - # run: | - # ./node_modules/.bin/component-explorer screenshot:compare \ - # --project ./test/componentFixtures \ - # --report ./test/componentFixtures/.screenshots/report - # continue-on-error: true - # - name: Prepare explorer artifact # run: | # mkdir -p /tmp/explorer-artifact/screenshot-report @@ -296,14 +332,6 @@ jobs: # name: component-explorer # path: /tmp/explorer-artifact/ - # - name: Upload screenshot report - # if: steps.compare.outcome == 'failure' - # uses: actions/upload-artifact@v7 - # with: - # name: screenshot-diff - # path: | - # test/componentFixtures/.screenshots/current/ - # test/componentFixtures/.screenshots/report/ # - name: Set check title # env: @@ -339,29 +367,3 @@ jobs: # {"state":"$STATE","target_url":"$DETAILS_URL","description":"$TITLE","context":"Component Screenshots"} # EOF - # - name: Post PR comment - # if: github.event_name == 'pull_request' - # env: - # GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # run: | - # COMMENT_MARKER="" - # BODY="$COMMENT_MARKER"$'\n' - # - # if [ -f test/componentFixtures/.screenshots/report.md ]; then - # BODY+=$(cat test/componentFixtures/.screenshots/report.md) - # BODY+=$'\n\n' - # BODY+="📦 [Download the \`screenshot-diff\` artifact](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) to review images." - # else - # BODY+="## Screenshots ✅"$'\n\n' - # BODY+="No visual changes detected." - # fi - # - # # Find existing comment - # EXISTING=$(gh api "repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/comments" \ - # --paginate --jq ".[] | select(.body | startswith(\"$COMMENT_MARKER\")) | .id" | head -1) - # - # if [ -n "$EXISTING" ]; then - # gh api "repos/${{ github.repository }}/issues/comments/$EXISTING" -X PATCH -f body="$BODY" - # else - # gh pr comment "${{ github.event.pull_request.number }}" --body "$BODY" - # fi