Skip to content

Commit 778561f

Browse files
balzssclaude
andcommitted
fix(regression-test): assert console errors across all captured themes
Each test now visits the page once per theme, and every cy.visit re-stubs console.error. The afterEach only saw the final visit's stub, so console errors during the canvas or light render were silently dropped. Accumulate the call count across visits and assert the total. Also refresh stale docs: correct the visual-regression guide link in the regression-test README, and update the cypress.config comment and the guide's capture step to reflect screenshots being taken directly in the spec (per theme) rather than in an afterEach hook. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9a5385a commit 778561f

4 files changed

Lines changed: 22 additions & 7 deletions

File tree

docs/contributing/testing/visual-regression.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ InstUI runs visual regression tests on every pull request. They guard against un
1212

1313
On every PR:
1414

15-
1. **Build & capture.** GitHub Actions builds InstUI, builds the `regression-test` Next.js app as a static site, and serves it. Cypress visits every page under `regression-test/src/app/*`, waits for it to render, and calls `cy.screenshot()` in an `afterEach` hook, producing one PNG per test case.
15+
1. **Build & capture.** GitHub Actions builds InstUI, builds the `regression-test` Next.js app as a static site, and serves it. Cypress visits every page under `regression-test/src/app/*` once per theme (via the `?theme=` query param), waits for each to render, and calls `cy.screenshot()` directly in the spec, producing one PNG per page/theme combination.
1616
2. **Diff.** The `ui-scripts visual-diff` command compares each screenshot against the matching baseline fetched from the `visual-baselines` branch of the repo. It uses [pixelmatch](https://github.com/mapbox/pixelmatch) with antialiasing detection enabled, so typical font-rendering noise doesn't register as a diff.
1717
3. **Publish.** The diff script writes an HTML report (with baseline / actual / diff for every row, plus a lightbox viewer) and the action publishes it to the `gh-pages` branch under `visual-regression/pr-<N>/`. The report is reachable at `https://instructure.design/visual-regression/pr-<N>/index.html`.
1818
4. **Comment.** A sticky PR comment summarizes the result, links to the report, and inlines each changed row's diff image inside a collapsed `<details>` block.

regression-test/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ A small Next.js app that imports `@instructure/ui` locally and exposes one page
88

99
Each page is captured once **per theme** (`canvas`, `light`, `dark`), so screenshots are named `<slug>-<theme>.png`. The theme is selected with the `?theme=<key>` query param, which `src/app/layout.tsx` reads and applies via `InstUISettingsProvider`. Add or remove themes with the `THEMES` array in `cypress/e2e/spec.cy.ts` (this multiplies the screenshot/baseline count).
1010

11-
See the [visual regression testing guide](../docs/testing/visual-regression.md) for the full CI pipeline, the diff report UI, and tuning notes.
11+
See the [visual regression testing guide](../docs/contributing/testing/visual-regression.md) for the full CI pipeline, the diff report UI, and tuning notes.
1212

1313
## Why npm instead of pnpm?
1414

regression-test/cypress.config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ export default defineConfig({
3131
trashAssetsBeforeRuns: true,
3232
video: false,
3333
// Never capture Cypress' automatic failure screenshots (the error overlay).
34-
// Visual regression baselines come solely from the deterministic afterEach
35-
// `cy.screenshot`; a failure screenshot has a different filename and would
36-
// otherwise leak into the diff as a spurious "new" image.
34+
// Visual regression baselines come solely from the deterministic
35+
// `cy.screenshot` calls in the spec; a failure screenshot has a different
36+
// filename and would otherwise leak into the diff as a spurious "new" image.
3737
screenshotOnRunFailure: false,
3838
e2e: {
3939
viewportWidth: 1280,

regression-test/cypress/e2e/spec.cy.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,33 @@ import type { Result, RunOnly } from 'axe-core'
2626

2727
type ConsoleErrorStub = Cypress.Agent<sinon.SinonStub<any[], any>>
2828
let windowErrorSpy: ConsoleErrorStub | undefined
29+
// Running total of console.error calls across every visit in the current test.
30+
// Each test visits the page once per theme; each cy.visit replaces
31+
// windowErrorSpy with a fresh stub, so we fold the outgoing stub's count into
32+
// this total before it's lost. Without this, the afterEach assertion would only
33+
// observe errors from the test's final visit.
34+
let consoleErrorCount = 0
2935

3036
Cypress.on('window:before:load', (win) => {
3137
// Stub console.error before your application code runs
3238
// This allows you to capture errors even if they happen very early
39+
if (windowErrorSpy) {
40+
consoleErrorCount += windowErrorSpy.callCount
41+
}
3342
windowErrorSpy = cy.stub(win.console, 'error')
3443
})
3544

45+
beforeEach(() => {
46+
consoleErrorCount = 0
47+
windowErrorSpy = undefined
48+
})
49+
3650
afterEach(() => {
37-
// After each test, assert that console.error was not called
51+
// After each test, assert that console.error was not called on any visit.
3852
// Add a small wait if your application might log errors asynchronously
3953
cy.wait(100).then(() => {
40-
expect(windowErrorSpy).to.have.callCount(0)
54+
const total = consoleErrorCount + (windowErrorSpy?.callCount ?? 0)
55+
expect(total, 'console.error call count across all visits').to.equal(0)
4156
})
4257
})
4358

0 commit comments

Comments
 (0)