From f635ee3cc0c213470a3e150222593f3f252d9902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20S=C3=A1ros?= Date: Thu, 30 Jul 2026 12:40:24 +0200 Subject: [PATCH 1/5] ci: keep visual-regression baselines refreshing and surface Cypress failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The baseline job ran Cypress without continue-on-error, so failing a11y/console assertions aborted the job before the "Push baselines" step β€” silently freezing the visual-baselines branch on every merge since the dark/light theme suite landed. Decouple baseline capture from the assertion gate: Cypress runs with continue-on-error, the screenshots always flatten and push (if: always()), and a trailing step fails the job for visibility if Cypress failed. Baselines now refresh on every merge regardless of the a11y/console outcome. Also add a "Cypress assertions" pass/fail line to the sticky PR comment so failures are visible even when the visual diff is clean, and remove the leftover TEMP diff-demo fixture step and regression-test/cypress/diff-demo/. Refs: INSTUI-5137 πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/visual-baselines.yml | 19 ++++++++++ .github/workflows/visual-regression.yml | 19 ++-------- regression-test/cypress/diff-demo/README.md | 35 ------------------ regression-test/cypress/diff-demo/actual.png | Bin 2069 -> 0 bytes .../cypress/diff-demo/baseline.png | Bin 2048 -> 0 bytes 5 files changed, 23 insertions(+), 50 deletions(-) delete mode 100644 regression-test/cypress/diff-demo/README.md delete mode 100644 regression-test/cypress/diff-demo/actual.png delete mode 100644 regression-test/cypress/diff-demo/baseline.png diff --git a/.github/workflows/visual-baselines.yml b/.github/workflows/visual-baselines.yml index f48cc6cfec..fe558821b7 100644 --- a/.github/workflows/visual-baselines.yml +++ b/.github/workflows/visual-baselines.yml @@ -42,7 +42,15 @@ jobs: run: npx cypress install working-directory: regression-test + # Capturing baselines is decoupled from the a11y/console assertions: the + # screenshots are valid renders regardless of whether those assertions + # pass, so a failing test must never block the baseline refresh (that's + # what silently froze the baselines branch before). We let Cypress fail + # here, always publish the screenshots below, then re-surface the failure + # in a trailing step so master still shows red and notifies. - name: Run Cypress + id: cypress + continue-on-error: true uses: cypress-io/github-action@v7 env: ELECTRON_EXTRA_LAUNCH_ARGS: "--remote-debugging-port=9222" @@ -53,11 +61,13 @@ jobs: working-directory: regression-test - name: Flatten screenshots + if: always() run: | mkdir -p baseline-publish/screenshots find regression-test/cypress/screenshots -name '*.png' -exec cp {} baseline-publish/screenshots/ \; - name: Push baselines to visual-baselines branch + if: always() run: | cd baseline-publish git init -b visual-baselines @@ -67,3 +77,12 @@ jobs: git commit -m "update baselines from ${GITHUB_SHA}" git remote add origin "https://x-access-token:${{ secrets.GITHUB_TOKEN }}@github.com/${GITHUB_REPOSITORY}.git" git push --force origin visual-baselines + + # Baselines have been refreshed by this point; now fail loudly if the + # suite had regressions (a11y/console violations or a broken page) so the + # failure is visible on master rather than silently swallowed. + - name: Fail if Cypress reported failures + if: steps.cypress.outcome == 'failure' + run: | + echo "::error::Baselines were published, but the regression suite had failing tests (a11y/console/spec). See the Run Cypress step logs." + exit 1 diff --git a/.github/workflows/visual-regression.yml b/.github/workflows/visual-regression.yml index 20f18e7b3b..c703dc248d 100644 --- a/.github/workflows/visual-regression.yml +++ b/.github/workflows/visual-regression.yml @@ -77,19 +77,6 @@ jobs: mkdir -p regression-test/.actual find regression-test/cypress/screenshots -name '*.png' -exec cp {} regression-test/.actual/ \; - # TEMP (remove before merge): this PR renames every screenshot with a - # `-` suffix, so nothing matches the old baselines and the report - # has zero "changed" rows β€” leaving the new diff visualization impossible - # to preview. Inject one synthetic baseline/actual pair that differs in a - # few localized spots so the report always has a "changed" example. - # Fixtures live in regression-test/cypress/diff-demo/. Delete this step - # and that directory to revert. - - name: TEMP inject diff-demo fixture - run: | - mkdir -p regression-test/.baselines regression-test/.actual - cp regression-test/cypress/diff-demo/baseline.png regression-test/.baselines/diff-demo.png - cp regression-test/cypress/diff-demo/actual.png regression-test/.actual/diff-demo.png - - name: Diff and generate report id: diff working-directory: regression-test @@ -173,7 +160,9 @@ jobs: message: | ## Visual regression report - ${{ steps.summary.outputs.status }} + **Cypress assertions (a11y + console errors):** ${{ steps.cypress.outcome == 'failure' && format('❌ **Failing** β€” see the [run logs]({0}/{1}/actions/runs/{2}).', github.server_url, github.repository, github.run_id) || 'βœ… Passing' }} + + **Visual diff:** ${{ steps.summary.outputs.status }} | Status | Count | |---|---| @@ -186,7 +175,7 @@ jobs: ${{ steps.summary.outputs.diff_images }} - Baselines come from the `visual-baselines` branch. They refresh on every merge to `master`. + Baselines come from the `visual-baselines` branch. They refresh on every merge to `master`. The Cypress assertions line above covers a11y (axe) and console-error checks β€” a ❌ there means the suite found real issues even if the visual diff is clean. - name: Fail if regressions or Cypress failed if: steps.diff.outcome == 'failure' || steps.cypress.outcome == 'failure' diff --git a/regression-test/cypress/diff-demo/README.md b/regression-test/cypress/diff-demo/README.md deleted file mode 100644 index e785cfd590..0000000000 --- a/regression-test/cypress/diff-demo/README.md +++ /dev/null @@ -1,35 +0,0 @@ -# diff-demo (TEMPORARY) - -A synthetic `baseline.png` / `actual.png` pair used only to preview the visual -regression report's **diff visualization** while this PR is open. - -## Why it exists - -This PR renames every captured screenshot with a `-` suffix -(`alert-dark.png` etc.), so none of them match the old baselines on the -`visual-baselines` branch. The result is a report full of _added_ / _removed_ -rows and **zero _changed_ rows** β€” meaning the reworked pixel-highlight diff -image never renders. This fixture forces one deterministic _changed_ row so the -new rendering can be reviewed in the published CI report. - -The two images are identical except for a few localized spots (a recolored -button and icon, a widened text line, and a 1px hairline underline) so the diff -exercises both the dimmed backdrop and the dilated highlight. - -## How it's wired in - -The `TEMP inject diff-demo fixture` step in -`.github/workflows/visual-regression.yml` copies `baseline.png` into -`.baselines/diff-demo.png` and `actual.png` into `.actual/diff-demo.png` just -before the diff step runs. - -## Removing it (before merge) - -1. Delete this directory (`regression-test/cypress/diff-demo/`). -2. Delete the `TEMP inject diff-demo fixture` step in - `.github/workflows/visual-regression.yml`. - -## Regenerating the images - -The images were produced by a throwaway `pngjs` script; see the PR discussion. -Any two near-identical PNGs with a small localized delta work equally well. diff --git a/regression-test/cypress/diff-demo/actual.png b/regression-test/cypress/diff-demo/actual.png deleted file mode 100644 index e7be9b7bd3acf0e6fd2c1c2003485ff604ebb9cb..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2069 zcmeAS@N?(olHy`uVBq!ia0y~yU}|7sU|hn%1{7H+F=ZYD1N$9M7srqa#*_|G%J1x29NkW}A=riH52Dse(HUj@n%d;}-TXSM+_QDH6ST_tYfK6RvY+?49|3 z&ga?t>;L_fXJkllV`exog^i&>lb3-ZSc-vRnF#}fPbLGyj7tm*21|hoyargo(-W<~ zPsjfkV`#`f`M&=9?U+r-u9ewYQ2*nd76ZfmN$2hVPV&A+XICrFDgA!jp7qChxw=1! z4DpArzSN7qbor&?Y3c@L5evhEL$-6}4o$xd^x#vDblX2aoEaG6CLxE#V2C-Hhii`a z&u3z&iQR5~{o()R#3Z0og6DSa-O0yrz?(X04ir3fe?%A_+`oG-raeELs00j*$m(KQ zhKA`&h;cH(Oab&$)t?`l3^DU=&wkDDe>PF^3nHj Whk|0|^wYrVp25@A&t;ucLK6T+xF9Y7 diff --git a/regression-test/cypress/diff-demo/baseline.png b/regression-test/cypress/diff-demo/baseline.png deleted file mode 100644 index 440633165b3a1c25d0edec75eb959bdb6b6540df..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2048 zcmeAS@N?(olHy`uVBq!ia0y~yU}|7sU|hn%1{7H+F=ZYD1N(7L7srqa#wWs>jSG6}A!S);pb)}iB@gm{+B_MTUa?0aj4-uXJ>d2{OR3a`JJhB_JQT0(EByNV;lazUoKzIao~7C z)s835yKLrdT0`AXb7x@4yYuv;h}|4eG`a711+tFe!1E>4wTAN013K;H+1m^Z^4EX< z)B97399K9Klgz_apZ~@(FxaV6Za>I`zJjZlXK!a@_y=-B&8~rT!w-=;@82_hNGB>o z0Ta{n-@6$YYW#?DGErs@mRyW8@`tuBfXV$!0cVVye`R0z`uR-y@pG69xceC7kDeBP zzTx%N*UK61bWvWgD*sul*T4Kj&GL5dX@7q|G_<+@y`P^k;sU^?P8*cyr From bf4ec17df05826fb1942b9d15e695d1521f69e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20S=C3=A1ros?= Date: Thu, 30 Jul 2026 13:27:31 +0200 Subject: [PATCH 2/5] feat(ui-scripts): surface axe a11y violations in the visual-diff report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regression suite already ran axe per page and theme, but the violations were only printed to the Cypress log and discarded. Persist them and render them in the report so designers can triage accessibility issues alongside the pixels. - regression-test: a new recordA11y Cypress task writes cypress/a11y.json keyed by "-" (mirroring meta.json); the spec records each violation's rule id, impact, help text, helpUrl, and offending node selector/summary. - ui-scripts visual-diff: a new --a11y flag reads that file and adds a per-row "a11y" badge, an "A11y" filter chip, a header count, and a collapsible list of violations (rule linked to its axe helpUrl, impact, selector, contrast summary) per screenshot. a11yPages/a11yViolations are added to summary.json. - visual-regression.yml passes --a11y cypress/a11y.json to the diff step. Additive: without --a11y the report is unchanged. Unit-tested esc() and a11yFor(), and verified end to end against a real suite run (15 flagged pages). Refs: INSTUI-5137 πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/visual-regression.yml | 1 + .../lib/__node_tests__/visual-diff.test.ts | 74 ++++++++- .../ui-scripts/lib/commands/visual-diff.ts | 147 ++++++++++++++++-- regression-test/.gitignore | 1 + regression-test/cypress.config.ts | 18 +++ regression-test/cypress/e2e/spec.cy.ts | 24 +++ 6 files changed, 250 insertions(+), 15 deletions(-) diff --git a/.github/workflows/visual-regression.yml b/.github/workflows/visual-regression.yml index c703dc248d..0af9fda1eb 100644 --- a/.github/workflows/visual-regression.yml +++ b/.github/workflows/visual-regression.yml @@ -89,6 +89,7 @@ jobs: --pr-number ${{ github.event.pull_request.number }} --pr-url ${{ github.event.pull_request.html_url }} --meta cypress/meta.json + --a11y cypress/a11y.json --source-base-url https://github.com/${{ github.repository }}/blob/${{ github.head_ref }}/regression-test/src/app --facets canvas,light,dark --app-path app diff --git a/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts b/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts index c302de0eb0..b9b857efda 100644 --- a/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts +++ b/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts @@ -29,7 +29,9 @@ import { indexByName, sourceLinkFor, appUrlFor, - dilateMask + dilateMask, + esc, + a11yFor } from '../commands/visual-diff.ts' // Build a w*h changed-mask with the given filled rectangles set to 1. @@ -259,3 +261,73 @@ describe('dilateMask', () => { expect(countSet(out)).toBe(countSet(src)) }) }) + +describe('esc', () => { + it('escapes HTML-significant characters', () => { + expect(esc('a & b')).toBe( + '<span class="x">a & b</span>' + ) + }) + + it('leaves plain text untouched', () => { + expect(esc('color-contrast')).toBe('color-contrast') + }) +}) + +describe('a11yFor', () => { + const a11y = { + 'button-dark': [ + { + id: 'color-contrast', + impact: 'serious', + help: 'Elements must meet minimum color contrast ratio thresholds', + helpUrl: 'https://dequeuniversity.com/rules/axe/color-contrast', + nodes: [ + { + target: '.css-x-baseButton__children', + html: 'primary-inverse', + summary: 'Expected 4.5:1 but got 1.13' + } + ] + } + ] + } + + it('returns empty output and count 0 when there are no violations', () => { + expect(a11yFor('alert-dark.png', a11y)).toEqual({ + badge: '', + details: '', + count: 0 + }) + expect(a11yFor('button-dark.png', null)).toEqual({ + badge: '', + details: '', + count: 0 + }) + }) + + it('builds a badge and details keyed by slug (name minus .png)', () => { + const { badge, details, count } = a11yFor('button-dark.png', a11y) + expect(count).toBe(1) + expect(badge).toContain('class="pill a11y"') + expect(badge).toContain('⚠ 1 a11y') + // rule id links to the axe helpUrl, impact and selector are rendered + expect(details).toContain( + 'href="https://dequeuniversity.com/rules/axe/color-contrast"' + ) + expect(details).toContain('color-contrast') + expect(details).toContain('a11y-impact serious') + expect(details).toContain('.css-x-baseButton__children') + expect(details).toContain('Expected 4.5:1 but got 1.13') + }) + + it('pluralizes the violation count', () => { + const many = { + 'x-dark': [...a11y['button-dark'], ...a11y['button-dark']] + } + expect(a11yFor('x-dark.png', many).badge).toContain('⚠ 2 a11y') + expect(a11yFor('x-dark.png', many).details).toContain( + '2 accessibility violations' + ) + }) +}) diff --git a/packages/ui-scripts/lib/commands/visual-diff.ts b/packages/ui-scripts/lib/commands/visual-diff.ts index 5c0e5aff11..f94a2bbe07 100644 --- a/packages/ui-scripts/lib/commands/visual-diff.ts +++ b/packages/ui-scripts/lib/commands/visual-diff.ts @@ -55,10 +55,33 @@ type Args = { sourceBaseUrl?: string facets?: string appPath?: string + a11y?: string } type Meta = Record +// Accessibility violations captured by the spec's axe run, keyed by screenshot +// slug (name minus `.png`, e.g. `button-dark`). Written by the `recordA11y` +// Cypress task; see regression-test/cypress.config.ts. +type A11yNode = { target: string; html: string; summary: string } +type A11yViolation = { + id: string + impact: string | null + help: string + helpUrl: string + nodes: A11yNode[] +} +type A11y = Record + +/** @internal β€” exported only for tests; not part of the package's public API. */ +export function esc(s: string): string { + return s + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') +} + /** @internal β€” exported only for tests; not part of the package's public API. */ export function sourceLinkFor( name: string, @@ -260,12 +283,62 @@ export function thumb(mode: string, name: string): string { return `` } +/** + * Build the a11y badge (for the row header) and the collapsible violation list + * (rule, impact, offending selector, and axe's failure summary) for a + * screenshot. Returns empty strings and count 0 when there are no violations. + * + * @internal β€” exported only for tests; not part of the package's public API. + */ +export function a11yFor( + name: string, + a11y: A11y | null +): { badge: string; details: string; count: number } { + const slug = name.replace(/\.png$/, '') + const violations = a11y?.[slug] ?? [] + const count = violations.length + if (!count) return { badge: '', details: '', count: 0 } + + const plural = count === 1 ? '' : 's' + const badge = `⚠ ${count} a11y` + + const items = violations + .map((v) => { + const impact = v.impact + ? `${esc(v.impact)} ` + : '' + const rule = v.helpUrl + ? `${esc(v.id)}` + : `${esc(v.id)}` + const nodes = v.nodes + .map( + (n) => + `
  • ${esc(n.target)}${ + n.summary + ? `
    ${esc(n.summary)}
    ` + : '' + }
    • ` + ) + .join('') + return `
  • ${impact}${rule} β€” ${esc( + v.help + )}
      ${nodes}
    • ` + }) + .join('') + + const details = `
    ${count} accessibility violation${plural}
      ${items}
    ` + return { badge, details, count } +} + function row( r: Result, meta: Meta | null, sourceBaseUrl?: string, facets: string[] = [], - appPath?: string + appPath?: string, + a11y: A11y | null = null ): string { const b = r.status === 'added' ? '' : thumb('baseline', r.name) const a = r.status === 'removed' ? '' : thumb('actual', r.name) @@ -283,14 +356,20 @@ function row( const htmlUrl = r.status === 'removed' ? '' : appUrlFor(r.name, meta, facets, appPath) const htmlAttr = htmlUrl ? ` data-html-url="${htmlUrl}"` : '' + const { + badge: a11yBadge, + details: a11yDetails, + count: a11yCount + } = a11yFor(r.name, a11y) return `
    + r.name + }" data-has-both="${hasBoth}" data-a11y="${a11yCount}"${htmlAttr}>

    ${r.name}

    ${badgeFor( - r.status - )}${pixelMeta}${source}
    + r.status + )}${a11yBadge}${pixelMeta}${source}
    Baseline
    ${b}
    Actual
    ${a}
    Diff
    ${d}
    + ${a11yDetails}
    ` } @@ -302,14 +381,15 @@ function renderHtml( meta?: Meta | null, sourceBaseUrl?: string, facets: string[] = [], - appPath?: string + appPath?: string, + a11y: A11y | null = null ): string { const prBadge = prNumber && prUrl ? `PR #${prNumber}` : prNumber - ? `PR #${prNumber}` - : '' + ? `PR #${prNumber}` + : '' // Land on the "Changed" view by default so reviewers see regressions first, // but fall back to "All" when nothing changed (otherwise the list is blank). const defaultFilter = summary.changed > 0 ? 'changed' : 'all' @@ -318,7 +398,8 @@ function renderHtml( ['changed', 'Changed'], ['added', 'New'], ['removed', 'Removed'], - ['unchanged', 'Unchanged'] + ['unchanged', 'Unchanged'], + ['a11y', '⚠ A11y'] ] return ` Visual regression report @@ -340,7 +421,19 @@ function renderHtml( .fail { background: #ffebe9; color: #cf222e; } .new { background: #ddf4ff; color: #0969da; } .gone { background: #fff1e5; color: #9a6700; } + .a11y { background: #fbefff; color: #8250df; } .meta { font-size: 12px; color: #666; } + .a11y-details { padding: 10px 16px; border-top: 1px solid #f0f0f0; font-size: 13px; } + .a11y-details > summary { cursor: pointer; color: #8250df; font-weight: 600; } + .a11y-list { margin: 10px 0 2px; padding-left: 18px; } + .a11y-list > li { margin-bottom: 8px; } + .a11y-nodes { margin: 4px 0 0; padding-left: 16px; list-style: circle; } + .a11y-nodes code.sel { font-size: 12px; color: #57606a; word-break: break-all; } + .a11y-summary { white-space: pre-wrap; color: #666; font-size: 12px; margin: 2px 0 6px; } + .a11y-impact { padding: 1px 6px; border-radius: 8px; font-size: 10px; font-weight: 700; text-transform: uppercase; color: #fff; } + .a11y-impact.critical, .a11y-impact.serious { background: #cf222e; } + .a11y-impact.moderate { background: #9a6700; } + .a11y-impact.minor { background: #57606a; } .filter { display: flex; flex-wrap: wrap; align-items: center; gap: 6px; margin-top: 8px; } .filter button { font-size: 12px; padding: 4px 10px; border: 1px solid #ddd; background: #fff; border-radius: 4px; cursor: pointer; } .filter button.active { background: #0969da; color: #fff; border-color: #0969da; } @@ -378,7 +471,12 @@ function renderHtml( OK: ${summary.unchanged} Changed: ${summary.changed} New: ${summary.added} - Removed: ${summary.removed} + Removed: ${summary.removed}${ + summary.a11yPages + ? ` + ⚠ A11y: ${summary.a11yPages}` + : '' + }
    ${statusFilters @@ -402,7 +500,7 @@ function renderHtml( }
    ${results - .map((r) => row(r, meta ?? null, sourceBaseUrl, facets, appPath)) + .map((r) => row(r, meta ?? null, sourceBaseUrl, facets, appPath, a11y)) .join('')}