Improves screenshot experience for forks

2026-07-09 09:24:12 -05:00 · 2026-05-04 16:22:15 +02:00
parent 1bb9a48221
commit bb81c9c9bb
1 changed files with 88 additions and 86 deletions
--- 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](<service-url>/images/<hash>)` (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<<SCREENSHOT_EOF" >> "$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="<!-- screenshot-report -->"
-      #     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