|
1 | 1 | --- |
2 | 2 | name: update-screenshots |
3 | | -description: Download screenshot baselines from the latest CI run and commit them. Use when asked to update, accept, or refresh component screenshot baselines from CI, or after the screenshot-test GitHub Action reports differences. This skill should be run as a subagent. |
| 3 | +description: Update the committed blocks-ci screenshot hashes after the "Screenshots & Tests" check fails, or investigate a screenshot diff reported on a PR. Use when asked to update, accept, or refresh component screenshot baselines from CI. This skill should be run as a subagent. |
4 | 4 | --- |
5 | 5 |
|
6 | 6 | # Update Component Screenshots from CI |
7 | 7 |
|
8 | | -Screenshot baselines are **no longer stored in the repository**. They are managed by an external screenshot service (`hediet-screenshots.azurewebsites.net`). The CI workflow uploads screenshots to this service and diffs them automatically. |
| 8 | +Screenshot **images** are not stored in the repository — they live in an external service |
| 9 | +(`hediet-screenshots.azurewebsites.net`), keyed by commit SHA. But a subset of fixtures is |
| 10 | +pinned by **hash** in [`test/componentFixtures/blocks-ci-screenshots.md`](../../../test/componentFixtures/blocks-ci-screenshots.md), |
| 11 | +and that file **is** committed. When those hashes change, CI fails and you must update the file. |
9 | 12 |
|
10 | | -When the `Checking Component Screenshots` GitHub Action detects changes, it posts a PR comment with before/after comparisons. No manual baseline updates are needed — the screenshots on the `main` branch commit become the new baselines automatically after merge. |
| 13 | +## Two different outcomes, only one of which blocks |
11 | 14 |
|
12 | | -## What Changed |
| 15 | +The `Screenshots & Tests` job in [`.github/workflows/component-fixtures.yml`](../../workflows/component-fixtures.yml) |
| 16 | +produces two independent results: |
13 | 17 |
|
14 | | -- Baseline images were removed from `test/componentFixtures/.screenshots/baseline/`. |
15 | | -- Git LFS is no longer used for screenshot storage. |
16 | | -- The screenshot service stores images keyed by commit SHA and handles diffing. |
| 18 | +| Result | Blocking? | Action | |
| 19 | +| --- | --- | --- | |
| 20 | +| Screenshot **diff report** (PR comment with before/after images) | No — informational | Review the visuals. Nothing to commit. | |
| 21 | +| **blocks-ci hash mismatch** | **Yes — fails the check** | Update `blocks-ci-screenshots.md` and commit. | |
17 | 22 |
|
18 | | -## If Screenshots Need Investigation |
| 23 | +A fixture opts into the blocking gate with `labels: { kind: 'screenshot', blocksCi: true }`. |
| 24 | +Only those fixtures appear in `blocks-ci-screenshots.md`. |
19 | 25 |
|
20 | | -1. Check the PR comment posted by the CI workflow for visual diffs. |
21 | | -2. Download the `screenshots` artifact from the CI run for the raw captured images: |
| 26 | +The failure looks like this: |
| 27 | + |
| 28 | +``` |
| 29 | +##[error]blocks-ci screenshot hashes do not match committed file. See PR comment or job summary for the updated content. |
| 30 | +``` |
| 31 | + |
| 32 | +## Step 1: Get the expected hashes from CI |
| 33 | + |
| 34 | +> **Never regenerate the hashes locally.** They are hashes of the rendered PNG bytes, produced |
| 35 | +> on `ubuntu-latest`. Rendering on macOS or Windows yields different bytes and therefore |
| 36 | +> different hashes, so locally generated values will fail CI. Always copy the values from the |
| 37 | +> CI job. |
| 38 | +
|
| 39 | +Three surfaces carry the same content — use whichever is handy: |
| 40 | + |
| 41 | +- The **PR comment** titled "blocks-ci screenshots changed" (non-fork PRs only) — contains the |
| 42 | + full updated file plus a patch. |
| 43 | +- The **job summary**, which gets the identical body and is the only surface fork PRs receive. |
| 44 | +- The **job log**, whose final step prints a unified diff: |
22 | 45 |
|
23 | 46 | ```bash |
24 | | -gh run download <run-id> --name screenshots --dir .tmp/screenshots |
| 47 | +gh api repos/microsoft/vscode/actions/jobs/<JOB_ID>/logs > "$TMPDIR/ci-job-log.txt" |
| 48 | +grep -n '##\[error\]' "$TMPDIR/ci-job-log.txt" |
25 | 49 | ``` |
26 | 50 |
|
27 | | -3. Compare locally if needed. The artifact contains the full set of captured screenshots. |
| 51 | +Find the failed job id with: |
| 52 | + |
| 53 | +```bash |
| 54 | +gh pr checks <PR> --json name,link,bucket --jq '.[] | select(.name == "Screenshots & Tests")' |
| 55 | +``` |
| 56 | + |
| 57 | +## Step 2: Verify the change is intentional before accepting it |
| 58 | + |
| 59 | +This gate exists to catch **unintended** layout regressions, so accepting new hashes without |
| 60 | +looking at the images defeats its purpose. The images are publicly fetchable by hash, so pull |
| 61 | +both the old (committed) and new (from CI) versions and compare: |
| 62 | + |
| 63 | +```bash |
| 64 | +curl -sL -o old.png "https://hediet-screenshots.azurewebsites.net/images/<OLD_HASH>" |
| 65 | +curl -sL -o new.png "https://hediet-screenshots.azurewebsites.net/images/<NEW_HASH>" |
| 66 | +``` |
| 67 | + |
| 68 | +Then view them, and localize the change rather than eyeballing full screenshots — the delta is |
| 69 | +often only a pixel or two: |
| 70 | + |
| 71 | +```bash |
| 72 | +python3 -c " |
| 73 | +from PIL import Image, ImageChops |
| 74 | +a = Image.open('old.png').convert('RGB'); b = Image.open('new.png').convert('RGB') |
| 75 | +print('diff bbox:', ImageChops.difference(a, b).getbbox()) |
| 76 | +" |
| 77 | +``` |
| 78 | + |
| 79 | +Confirm the delta matches what the PR intends. If the fixture is unrelated to the change, or |
| 80 | +the shift is larger than expected, treat it as a regression and fix the code instead of the |
| 81 | +hashes. |
| 82 | + |
| 83 | +## Step 3: Apply and commit |
| 84 | + |
| 85 | +Edit only the changed lines in `test/componentFixtures/blocks-ci-screenshots.md`, replacing the |
| 86 | +old hash in the image URL with the new one: |
| 87 | + |
| 88 | +```md |
| 89 | +#### editor/inlineChatZoneWidget/InlineChatZoneWidget/Dark |
| 90 | + |
| 91 | +``` |
| 92 | + |
| 93 | +The file is generated by [`build/lib/screenshotBlocksCi.ts`](../../../build/lib/screenshotBlocksCi.ts) |
| 94 | +and compared **byte-for-byte**, so keep the `<!-- auto-generated by CI — do not edit manually -->` |
| 95 | +header, the `#### <fixtureId>` / image-link pairing, the blank line between entries, and the |
| 96 | +`fixtureId` sort order intact. Verify your edit is the exact inverse of the diff CI reported: |
| 97 | + |
| 98 | +```bash |
| 99 | +git diff test/componentFixtures/blocks-ci-screenshots.md |
| 100 | +``` |
| 101 | + |
| 102 | +Then commit and push. The check re-runs and should pass; hashes on `main` become the new |
| 103 | +baseline after merge. |
| 104 | + |
| 105 | +## Investigating further |
| 106 | + |
| 107 | +Raw captured images and the manifest for a run are uploaded as an artifact: |
| 108 | + |
| 109 | +```bash |
| 110 | +gh run download <RUN_ID> --name screenshots --dir .tmp/screenshots |
| 111 | +``` |
| 112 | + |
| 113 | +`manifest.json` maps each `fixtureId` to its `imageHash` and any render errors. |
| 114 | + |
| 115 | +## Related failures from the same job |
| 116 | + |
| 117 | +The check also fails if a fixture **failed to render** (`Fail if fixtures had errors`) or if the |
| 118 | +Playwright fixture tests failed. Those are genuine bugs — updating hashes will not help. Look |
| 119 | +for `::error::<fixtureId>:` in the log, and download the `playwright-test-results` artifact for |
| 120 | +test failures. |
0 commit comments