From b35329c8f5e134d07c687fbc34de2e97154b5c35 Mon Sep 17 00:00:00 2001 From: Reece Browne <74901996+reecebrowne@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:13:41 +0100 Subject: [PATCH] a11y scan: generate required assets, and fail when a story file can't load (#7201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Two related bugs found while looking at why #7187's a11y check behaves differently on CI than locally. ### 21 stories were never being scanned on CI The scan tasks only depended on `install`, not `prepare`. On a fresh checkout that means the generated icon set (`editor/src/assets/material-symbols-icons.json`, gitignored) doesn't exist, so every story that reaches `LocalIcon` fails to import: ``` Failed to resolve import "../../../assets/material-symbols-icons.json" from "editor/src/core/components/shared/LocalIcon.tsx" ``` On CI that was four story files / 21 stories, every run. It works locally only because our trees already have the file from a previous build. The scan tasks now depend on `prepare`, like the `build:*` tasks do. ### The gate reported those runs as clean Worse than the missing stories: a file that fails to import produces a **failed suite with no assertions**. Every check in `a11y-check.mjs` reads assertions, so the file satisfied the manifest, contributed nothing to compare, and the run printed `✓ no a11y regressions`. An assertion-less failed suite now fails the gate and points at the scan log for the underlying resolve error. `--record` refuses in the same situation, so a baseline can't be written that quietly drops those stories. Also switched the affected-story emptiness test to single quotes, since that list now carries its own per-path quoting (it was producing `[ -z ""a" "b"" ]`). ## Testing - Deleted the generated asset to reproduce a fresh checkout: the gate **fails** with the file named and the cause explained, where before it printed `✓ no a11y regressions` and exited 0. - With the `prepare` dependency the task regenerates the asset itself and the previously-invisible files scan: 21 stories, 35 story-rule pairs, all already baselined. --- .taskfiles/frontend.yml | 14 ++++++------ frontend/.storybook/a11y-check.mjs | 36 +++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/.taskfiles/frontend.yml b/.taskfiles/frontend.yml index 8c32efc5e3..3c2113bf96 100644 --- a/.taskfiles/frontend.yml +++ b/.taskfiles/frontend.yml @@ -184,13 +184,13 @@ tasks: storybook: desc: "Start Storybook dev server" - deps: [install] + deps: [prepare] cmds: - npx storybook dev -p 6006 {{.CLI_ARGS}} storybook:build: desc: "Build static Storybook" - deps: [install] + deps: [prepare] cmds: - npx storybook build {{.CLI_ARGS}} @@ -204,7 +204,7 @@ tasks: storybook:test: desc: "Scan every story in real Chromium: it must render and pass axe" - deps: [install, storybook:browser] + deps: [prepare, storybook:browser] cmds: # Runs each story as a browser test. Pass a filter through, e.g. # task frontend:storybook:test -- Button @@ -212,7 +212,7 @@ tasks: storybook:a11y: desc: "a11y regression gate over every story: fail only on NEW axe violations" - deps: [install, storybook:browser] + deps: [prepare, storybook:browser] cmds: - node .storybook/a11y-scan.mjs - node .storybook/a11y-check.mjs --in .a11y-scan --manifest .a11y-scan/manifest.txt @@ -231,14 +231,14 @@ tasks: Pass a base ref through CLI_ARGS, e.g. task frontend:storybook:a11y:changed -- origin/release - deps: [install, storybook:browser] + deps: [prepare, storybook:browser] vars: BASE: '{{.CLI_ARGS | default "origin/main"}}' CHANGED: sh: node .storybook/a11y-changed.mjs {{.CLI_ARGS | default "origin/main"}} cmds: - cmd: | - if [ -z "{{.CHANGED}}" ]; then + if [ -z '{{.CHANGED}}' ]; then echo "a11y: no story files affected vs {{.BASE}} — nothing to check" exit 0 fi @@ -247,7 +247,7 @@ tasks: storybook:a11y:record: desc: "Re-record the a11y baseline (run after intentionally fixing/adding violations)" - deps: [install, storybook:browser] + deps: [prepare, storybook:browser] cmds: - node .storybook/a11y-scan.mjs - node .storybook/a11y-check.mjs --in .a11y-scan --manifest .a11y-scan/manifest.txt --record diff --git a/frontend/.storybook/a11y-check.mjs b/frontend/.storybook/a11y-check.mjs index 7eef5e079e..43092812ff 100644 --- a/frontend/.storybook/a11y-check.mjs +++ b/frontend/.storybook/a11y-check.mjs @@ -52,6 +52,7 @@ const RULE_URL = /dequeuniversity\.com\/rules\/axe\/[\d.]+\/([a-z0-9-]+)/g; function collect(dir) { const rules = {}; // storyKey -> Set(ruleId) const crashed = []; // storyKey[] — failed for a non-a11y reason + const unloadable = []; // storyFile[] — the file itself never ran const seenFiles = new Set(); let scanned = 0; @@ -68,6 +69,14 @@ function collect(dir) { const idx = norm.search(/editor\/src\//); const file = idx >= 0 ? norm.slice(idx) : norm; seenFiles.add(file); + // A story file that fails to import produces a failed suite with no + // assertions at all. Every other check here reads assertions, so such a + // file satisfies the manifest and contributes nothing — its stories go + // unscanned while the run still reports clean. + if ((tf.assertionResults || []).length === 0 && tf.status !== "passed") { + unloadable.push(file); + continue; + } for (const a of tf.assertionResults || []) { scanned++; if (a.status === "passed") continue; @@ -88,14 +97,14 @@ function collect(dir) { } } } - return { rules, crashed, seenFiles, scanned }; + return { rules, crashed, unloadable, seenFiles, scanned }; } if (!existsSync(inDir)) { console.error(`a11y-check: scan dir not found: ${inDir}`); process.exit(2); } -const { rules, crashed, seenFiles, scanned } = collect(inDir); +const { rules, crashed, unloadable, seenFiles, scanned } = collect(inDir); const observed = {}; for (const [k, set] of Object.entries(rules)) observed[k] = [...set].sort(); @@ -129,6 +138,14 @@ if (record || merge) { merge && existsSync(baselineFile) ? JSON.parse(readFileSync(baselineFile, "utf8")) : {}; + if (unloadable.length) { + console.error( + `a11y-check: refusing to record — ${unloadable.length} story file(s) failed to load:`, + ); + unloadable.slice(0, 20).forEach((f) => console.error(` ${f}`)); + console.error("Their stories never ran, so the baseline would lose them."); + process.exit(2); + } if (crashed.length) { console.error( `a11y-check: refusing to record — ${crashed.length} story(ies) failed for a non-a11y reason:`, @@ -176,6 +193,19 @@ console.log( `${pairs} story-rule pairs (baselined).`, ); +if (unloadable.length) { + console.error( + `\n✖ ${unloadable.length} story file(s) failed to load, so their stories never ran:`, + ); + unloadable.slice(0, 50).forEach((f) => console.error(` ${f}`)); + if (unloadable.length > 50) + console.error(` … and ${unloadable.length - 50} more`); + console.error( + "\nA file that cannot be imported reports no violations at all. The resolve " + + "or transform error is in the scan log (.a11y-scan/scan.log, uploaded as a " + + "run artifact); a missing generated asset is the usual cause.", + ); +} if (crashed.length) { console.error(`\n✖ ${crashed.length} story(ies) failed to render:`); crashed.slice(0, 50).forEach((k) => console.error(` ${k}`)); @@ -191,7 +221,7 @@ if (regressions.length) { "baseline key no longer matches — re-record: task frontend:storybook:a11y:record", ); } -if (crashed.length || regressions.length) process.exit(1); +if (unloadable.length || crashed.length || regressions.length) process.exit(1); if (fixed.length) console.log(