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..750a02c259 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 @@ -102,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 @@ -137,7 +125,7 @@ jobs: run: | node -e " const fs = require('fs'); - const { summary: s, results } = JSON.parse(fs.readFileSync('regression-test/.report/summary.json', 'utf8')); + const { summary: s, results, a11y = [] } = JSON.parse(fs.readFileSync('regression-test/.report/summary.json', 'utf8')); const base = 'https://instructure.design/visual-regression/pr-' + process.env.PR_NUMBER; const interesting = results.filter(r => r.status === 'changed' || r.status === 'removed'); let md = ''; @@ -153,6 +141,17 @@ jobs: } md += ''; } + // Name the a11y rules in the comment. Numbers are per offending + // element; the same rule across themes is one row, matching the + // report's overview panel. + let a11yMd = '✅ No violations.'; + if (a11y.length) { + a11yMd = '⚠️ **' + s.a11yViolations + ' violation' + (s.a11yViolations === 1 ? '' : 's') + + '** on ' + s.a11yPages + ' screenshot' + (s.a11yPages === 1 ? '' : 's') + '.\n\n' + + '| Impact | Issue | Rule | Elements | Screenshots |\n|---|---|---|---:|---:|\n' + + a11y.map(r => '| ' + r.impact + ' | ' + r.title + ' | [`' + r.rule + '`](' + r.helpUrl + ') | ' + + r.nodes + ' | ' + r.screens + ' |').join('\n'); + } const lines = [ 'total=' + s.total, 'unchanged=' + s.unchanged, @@ -163,6 +162,7 @@ jobs: ]; let out = lines.join('\n') + '\n'; out += '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 suite line covers the a11y and console-error assertions — 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/docs/contributing/testing/visual-regression.md b/docs/contributing/testing/visual-regression.md index 7d574e6996..85427048b6 100644 --- a/docs/contributing/testing/visual-regression.md +++ b/docs/contributing/testing/visual-regression.md @@ -59,19 +59,20 @@ open .report/index.html `npm run visual-diff` invokes the same `ui-scripts visual-diff` command CI uses. Pass extra flags to override the defaults: -| Flag | Default | Description | -| -------------------------------------------------------------- | --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--actual-dir ` | `cypress/screenshots` | Newly captured screenshots. | -| `--baseline-dir ` | `.baselines` | Baselines to compare against. | -| `--output-dir ` | `visual-report` | Where the HTML report + diff PNGs are written. | -| `--threshold <0..1>` | `0.1` | Per-pixel YIQ color threshold. Lower is stricter. | -| `--fail-on-missing-baseline` / `--no-fail-on-missing-baseline` | on | Whether to exit 1 when actual screenshots have no baseline. | -| `--pr-number ` | — | Rendered in the report header as a link to the PR. | -| `--pr-url ` | — | Target of the PR link. | -| `--meta ` | — | JSON mapping screenshot slug → visited URL path. Enables per-row source-file links in the report. Produced automatically by `spec.cy.ts` via `cy.task('recordMeta', ...)`. | -| `--source-base-url ` | — | GitHub blob URL of `regression-test/src/app` on the branch being reviewed. Combined with `--meta` to build links like `treebrowser/page.tsx`. | -| `--facets ` | — | Comma-separated facet suffixes (e.g. `canvas,light,dark`) rendered as one-click filter chips that match screenshots named `-`. | -| `--app-path ` | — | Path, relative to the report root, where a static export of the app is published (CI uses `app`). Enables the lightbox **HTML** view, which iframes the live rendered page. | +| Flag | Default | Description | +| -------------------------------------------------------------- | --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--actual-dir ` | `cypress/screenshots` | Newly captured screenshots. | +| `--baseline-dir ` | `.baselines` | Baselines to compare against. | +| `--output-dir ` | `visual-report` | Where the HTML report + diff PNGs are written. | +| `--threshold <0..1>` | `0.1` | Per-pixel YIQ color threshold. Lower is stricter. | +| `--fail-on-missing-baseline` / `--no-fail-on-missing-baseline` | on | Whether to exit 1 when actual screenshots have no baseline. | +| `--pr-number ` | — | Rendered in the report header as a link to the PR. | +| `--pr-url ` | — | Target of the PR link. | +| `--meta ` | — | JSON mapping screenshot slug → visited URL path. Enables per-row source-file links in the report. Produced automatically by `spec.cy.ts` via `cy.task('recordMeta', ...)`. | +| `--source-base-url ` | — | GitHub blob URL of `regression-test/src/app` on the branch being reviewed. Combined with `--meta` to build links like `treebrowser/page.tsx`. | +| `--facets ` | — | Comma-separated facet suffixes (e.g. `canvas,light,dark`) rendered as one-click filter chips that match screenshots named `-`. | +| `--app-path ` | — | Path, relative to the report root, where a static export of the app is published (CI uses `app`). Enables the lightbox **HTML** view, which iframes the live rendered page. | +| `--a11y ` | — | JSON of axe violations keyed by screenshot slug. Enables everything in [Accessibility findings](#accessibility-findings) below. Produced automatically by `spec.cy.ts` via `cy.task('recordA11y', ...)`. | ### Interpreting the report @@ -79,15 +80,34 @@ The HTML report is a single self-contained page showing one section per screensh Top-bar controls: -- **Filter buttons** (`All`, `Changed`, `New`, `Removed`, `Unchanged`) — the active one is highlighted. +- **Filter buttons** (`All`, `Changed`, `New`, `Removed`, `Unchanged`, `⚠ A11y`) — the active one is highlighted. - **Search input** — live, debounced substring filter on the screenshot name. +- **Theme chips** — narrow to one theme's screenshots. - **Lightbox** — click any thumbnail to open a fullscreen viewer. Inside: - `Baseline`, `Actual`, `Diff` buttons switch between the three images. - `Slider` mode overlays baseline and actual with a drag handle so you can scrub the boundary back and forth. + - `⚠ N a11y` mode shows the screenshot at full size with every violating element boxed, and the findings docked alongside. Shown only for screenshots with findings. - `HTML` mode iframes the live rendered page (a static export of the app published next to the report) so you can inspect the real DOM, computed styles, and text alongside the pixels. Shown only when the page still exists and the app was published (`--app-path`). - `1:1` / `Fit` toggles between native pixel size (scrollable) and fit-to-window. - `‹` / `›` step through visible rows (respects the current filter). +### Accessibility findings + +axe runs against every page and theme, and the report shows what it found **on the picture**, so a designer can act on it without reading a selector. + +While the page is still on screen, `spec.cy.ts` records each violating element's bounding box, a human-readable name for it, and — for contrast failures — the foreground/background colors and ratios axe computed (see `cypress/support/a11y-capture.ts`). The report then renders: + +- **A run-level overview** at the top of the page: one line per rule with its impact, how many elements it affects, and how many screenshots it appears on. The same rule tripping in `canvas`, `light`, and `dark` collapses to a single line. Click a rule to narrow the list to just the screenshots it affects. +- **A `⚠ N a11y` badge** in each affected screenshot's header, colored by the worst impact on it. That badge is the only a11y marking in the list — rows stay a clean three-up image comparison. **Click it** to open the lightbox's **A11y** view. +- **The A11y view**: the screenshot at full size with every violating element boxed and numbered, colored by impact, and the findings listed beside it. Clicking a card highlights its box and vice versa. Findings are ordered worst-impact-first, so `#1` is always the most serious thing on that screenshot. +- **One card per offending element**, leading with a plain-language headline ("Text contrast is too low") rather than the rule id, naming the element (`button “Archive this item”`), and suggesting a fix. Contrast failures show both colors as swatches with the measured and required ratios. The raw axe output — rule id, selector, and `failureSummary` — sits behind **Technical details**. + +The boxes and cards are rendered into an inert `