From 77b7012993f8de13a5dcbd4d528ea75c489c91cd Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Fri, 15 May 2026 15:03:57 -0400 Subject: [PATCH] ci: add resolution guidance to failing snapshot PR comment (#489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci: add resolution guidance to failing snapshot PR comment When snapshots differ from the main baseline, the comment now includes a short blockquote explaining both resolution paths: - merge the latest main (in case upstream baselines have moved) - add the update-snapshots label to acknowledge intentional changes * ci: wait for main baseline workflow before downloading artifact Before downloading the snapshot-baselines artifact on PR runs, resolve main's current HEAD SHA and check if the snapshot-tests.yml run for that exact commit is still in-progress or queued. If so, poll every 10 s (up to 10 min) until it completes, then proceed. This eliminates the race condition where a PR job starts while main's baseline upload is still in-flight, causing it to pull the previous (stale) artifact and produce false snapshot failures. The wait targets only the run for the current HEAD SHA — not an older in-progress run from a different commit — so two rapid commits to main can't trick the check into waiting for the wrong run. Also bumps job timeout-minutes from 20 → 30 to accommodate the wait. --------- Co-authored-by: openhands --- .github/workflows/snapshot-tests.yml | 51 ++++++++++++++++++- .../scripts/post-snapshot-comment.mjs | 9 ++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/.github/workflows/snapshot-tests.yml b/.github/workflows/snapshot-tests.yml index 22f75a2226..fafed6c5c2 100644 --- a/.github/workflows/snapshot-tests.yml +++ b/.github/workflows/snapshot-tests.yml @@ -26,7 +26,7 @@ jobs: snapshot-tests: name: Visual Snapshot Tests runs-on: ubuntu-24.04 - timeout-minutes: 20 + timeout-minutes: 30 steps: - name: Check out repository @@ -64,6 +64,55 @@ jobs: # ── PR BRANCH: download main baselines, compare, report diffs ──────── + # Wait for the snapshot-tests.yml run triggered by the *current* HEAD + # commit of main to complete before we download baselines. Without this, + # a PR job that starts while main's baseline upload is still in flight + # downloads the previous (stale) artifact and produces false failures. + # + # We resolve main's HEAD SHA first so we wait only for that specific run — + # not for an older in-progress run from a different commit. + - name: Wait for main baseline workflow if in progress + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + MAIN_SHA=$(gh api "/repos/${{ github.repository }}/git/ref/heads/main" \ + --jq '.object.sha' 2>/dev/null || echo "") + if [ -z "$MAIN_SHA" ]; then + echo "Could not resolve main HEAD SHA — skipping wait." + exit 0 + fi + echo "main HEAD SHA: $MAIN_SHA" + + # Find the snapshot-tests.yml run triggered by exactly this commit. + RUN_ID=$(gh api \ + "/repos/${{ github.repository }}/actions/workflows/snapshot-tests.yml/runs?branch=main&per_page=20" \ + --jq "[.workflow_runs[] | select(.head_sha == \"$MAIN_SHA\")] | .[0].id // empty") + + if [ -z "$RUN_ID" ]; then + echo "No snapshot-tests run found for main SHA $MAIN_SHA — proceeding." + exit 0 + fi + + STATUS=$(gh api "/repos/${{ github.repository }}/actions/runs/$RUN_ID" --jq '.status') + echo "Run $RUN_ID status: $STATUS" + if [ "$STATUS" = "completed" ]; then + echo "Already completed — proceeding." + exit 0 + fi + + echo "Waiting for run $RUN_ID to complete (max 10 min)..." + for i in $(seq 1 60); do + sleep 10 + STATUS=$(gh api "/repos/${{ github.repository }}/actions/runs/$RUN_ID" --jq '.status') + echo " [$((i * 10))s] status: $STATUS" + if [ "$STATUS" = "completed" ]; then + echo "Main baseline workflow completed." + exit 0 + fi + done + echo "::warning::Timed out waiting for main baseline workflow — proceeding with latest available artifact." + - name: Find latest snapshot-baselines artifact from main if: github.event_name == 'pull_request' id: find-run diff --git a/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs b/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs index b9079e745a..be4bc7cd90 100644 --- a/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs +++ b/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs @@ -252,6 +252,15 @@ function buildComment(changed, newSnapshots, unchanged, commitSha) { "", ]; + if (hasDifferences && !SNAPSHOTS_APPROVED) { + lines.push( + `> **How to resolve:**`, + `> - **Unintentional diffs** — the baselines on \`main\` may have moved since this branch was created. Merge the latest \`main\` into this branch and re-run CI.`, + `> - **Intentional changes** — add the \`update-snapshots\` label. CI will pass and the new screenshots become the baseline when this PR merges.`, + "", + ); + } + // Changed snapshots if (changed.length > 0) { lines.push(