From 4d42f7263389c3ead579cf99a9fe7d03e21fe709 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 14:29:52 +0000 Subject: [PATCH] fix: six bugs found by using 0.4.0 on real builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each of these was reached by running the tool rather than reading it, and each is a case where a report said something that was not true. audit --browser audited nothing unless it was given a directory. The runner takes the build directory so it can serve the pages over loopback; passing nothing is how a caller says these pages came off a running site. The audit command passed the directory the user typed, which under auto-detection is nothing — so `eaa-kit audit --browser`, the form the README leads with, navigated Chromium to a filesystem path and failed every page. The collection stage now reports the directory it settled on. A baseline stopped suppressing a barrier when anything else on the page changed, and diff reported that same untouched barrier as fixed. The element fingerprint hashed the whole outerHTML, and for the document-level rules the failing element is , whose outerHTML is the entire page. It now covers the opening tag only; axe-core's selector already separates elements that share one. This moves the baseline and JSON report schema versions to 2 and the SARIF fingerprint key to eaaKit/v2, because reading either across the boundary produces exactly the falsehood being fixed. A run where every page failed opened its summary with "No violations" in green. The count was of pages attempted rather than pages audited. A browser run reported criteria as unevaluable and then advised --browser, because the coverage view consulted jsdom's blind-spot table without asking which engine had run. A filter that excluded every page was reported as an empty build directory, with framework advice naming another directory to audit. One unreachable page was counted in the singular and conjugated in the plural. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012xCGmHyxdTMCF2GFwr4CAm --- CHANGELOG.md | 72 +++++++++++++++++++ docs/baseline.md | 16 +++-- docs/reports.md | 31 ++++---- examples/baseline.json | 6 +- examples/report.json | 20 +++--- examples/report.sarif | 6 +- src/audit/baseline.ts | 19 ++++- src/audit/collect.ts | 23 ++++++ src/audit/coverage.ts | 12 +++- src/audit/diff.ts | 5 +- src/audit/fingerprint.ts | 61 +++++++++++++++- src/audit/report/console.ts | 30 ++++++-- src/audit/report/json.ts | 9 ++- src/audit/report/sarif.ts | 6 +- src/cli/audit.ts | 13 +++- src/cli/baseline.ts | 6 +- src/cli/pages.ts | 29 ++++++++ src/statement/findings.ts | 2 +- tests/astro/integration.test.ts | 3 +- tests/audit/baseline.test.ts | 29 ++------ tests/audit/coverage.test.ts | 67 ++++++++++++++++++ tests/audit/diff.test.ts | 10 +-- tests/audit/fingerprint.test.ts | 98 ++++++++++++++++++++++++++ tests/audit/report/console.test.ts | 38 ++++++++++ tests/audit/report/json.test.ts | 8 +-- tests/audit/report/sarif.test.ts | 2 +- tests/audit/runners/playwright.test.ts | 41 +++++++++++ tests/cli/audit.test.ts | 18 ++++- tests/cli/baseline.test.ts | 27 +++++-- tests/cli/diff.test.ts | 3 +- tests/cli/pages.test.ts | 80 +++++++++++++++++++++ tests/cli/statement.test.ts | 3 +- tests/fixtures/statement/audit.json | 2 +- tests/statement/findings.test.ts | 10 ++- 34 files changed, 700 insertions(+), 105 deletions(-) create mode 100644 tests/audit/fingerprint.test.ts create mode 100644 tests/cli/pages.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index df30067..0074011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,78 @@ move: the JSON report's `schemaVersion` and the baseline file's. Both are bumped a field is removed, renamed, or changes meaning — new fields may appear without one, so consumers must ignore what they do not recognise. +## Unreleased + +### Fixed + +- **`audit --browser` audited nothing at all unless it was given a directory.** The + runner takes the build directory so it can serve the pages over loopback; passing + nothing is how a caller says these pages came off a running site and already have + somewhere to be fetched from. The audit command passed the directory *the user typed* — + which under auto-detection is nothing, because auto-detection is the thing that works it + out. So `eaa-kit audit --browser`, the form the README leads with, skipped the server and + navigated Chromium to `/home/you/site/dist/index.html`: a filesystem path, not a URL. + Every page came back `Cannot navigate to invalid URL`. + + The collection stage now reports the directory it settled on, and both commands pass that + along. A real crawl still passes nothing, because serving a crawled page back out of a + copy on disk would audit the markup with the server that produced it cut out of the + picture. `--browser` with an explicit directory was never affected, which is why the + suite did not see it: every browser test named one. + +- **A run where every page failed reported "No violations".** The count in that sentence was + of pages attempted, not pages audited, so a run that could read none of them opened its + summary with a clean result in green — the exact fail-open the rest of the tool is built + to refuse — with the error line beneath it. It now says nothing was audited, and where + only some pages failed the counts are over the pages that were actually read, so + "no violations across 2 pages" cannot describe a run that opened one. + +- **A baseline stopped suppressing a barrier when anything else on the page changed**, and + `diff` reported that same untouched barrier as **fixed**. The identity of a violating + element hashed the element's whole outer markup, and for the document-level rules — + `html-has-lang`, `document-title`, and every other rule that fails against `` — the + element's outer markup is the entire page. Adding one paragraph gave them a new identity, + and all three consumers of that identity believed it: the build went red on barriers a + team had accepted, `diff` announced a missing `` as fixed while the page still had + none, and SARIF churned its fingerprints so code scanning closed an alert and opened an + identical one on every edit. + + The hash now covers the element's opening tag and not its descendants. Where two elements + share an opening tag, axe-core's selector already tells them apart — it qualifies an + ambiguous match with `:nth-child(…)` — so nothing that could be distinguished before + stops being distinguishable, and a barrier that is genuinely fixed still changes identity. + This moves both file contracts; see **Changed**. + +- **A browser run reported criteria as unevaluable and advised `--browser`.** The coverage + view consulted the table of rules jsdom is structurally blind to without asking which + engine had run, so a report produced in real Chromium said colour contrast and target + size were beyond it, and closed by suggesting the flag that run had been given. It now + believes a browser run's own results: a rule reaches no verdict there only if that run + said so. + +- **A filter that excluded every page was reported as an empty build directory**, together + with framework advice naming another directory to audit — a fix for a path that was never + wrong. `--include`/`--exclude` matching nothing now says so, and echoes the patterns it + was given. + +- One unreachable page was counted in the singular and conjugated in the plural: "1 page + could not be reached, and were not audited". + +### Changed + +- **The baseline file's `schemaVersion` moves to 2** and the JSON report's to 2, because + `fingerprint` changed meaning in both. Neither is read across the boundary: a baseline + written by 0.4.0 records identities under the old rule, and matching them against the new + one would suppress nothing while looking as though it had, so it is refused with the + command that rewrites it. `diff` likewise refuses to compare a 0.4.0 report against a + newer one — that comparison is precisely the one that reports every document-level + barrier as both new and fixed. SARIF's partial fingerprint key moves to `eaaKit/v2` for + the same reason, which is what tells code scanning these are a new scheme rather than + defects that moved. + + Re-record a baseline with `eaa-kit baseline`, and read the new file before committing it: + it lists what this run found, which is not necessarily what the old one accepted. + ## 0.4.0 — 2026-09-01 ### Added diff --git a/docs/baseline.md b/docs/baseline.md index a916349..e9bf61c 100644 --- a/docs/baseline.md +++ b/docs/baseline.md @@ -50,13 +50,13 @@ to become one. ```jsonc { - "schemaVersion": 1, + "schemaVersion": 2, "createdOn": "2026-08-27", "entries": [ { "page": "index.html", "ruleId": "image-alt", - "fingerprint": "f0e17d2582e9a5b3", // rule + selector + markup, not the path + "fingerprint": "f0e17d2582e9a5b3", // rule + selector + opening tag, not the path "selector": "img", // the rest is for whoever reads the file "help": "Images must have alternative text", "impact": "critical", @@ -70,8 +70,16 @@ to become one. The fingerprint deliberately excludes the page path — it is the same one SARIF uses — so moving a page does not invalidate the entry's identity, though the `page` field does have -to match. Entries are sorted, so the file diffs cleanly and two people regenerating it get -the same result. +to match. It also excludes anything nested inside the failing element, so an entry for a +document-level rule such as `html-has-lang` survives an edit elsewhere on the page. +Entries are sorted, so the file diffs cleanly and two people regenerating it get the same +result. + +A baseline written by an earlier version is refused rather than read: `schemaVersion` 1 +recorded fingerprints under the old rule, and matching them against the new one would +suppress nothing while looking as though it had. Record it again with `eaa-kit baseline`, +then read the new file before committing it — it lists what this run found, which is not +necessarily what the old one accepted. ## In code scanning diff --git a/docs/reports.md b/docs/reports.md index d24da2c..4f720c3 100644 --- a/docs/reports.md +++ b/docs/reports.md @@ -10,7 +10,7 @@ shareable ones. ### Compatibility -- `schemaVersion` is an integer, currently `1`. +- `schemaVersion` is an integer, currently `2`. - It is bumped only when a field is **removed, renamed, or changes meaning**. - New fields may be added without a bump, so **consumers must ignore fields they do not recognise**. @@ -33,7 +33,7 @@ A complete generated document is checked in at ```jsonc { - "schemaVersion": 1, + "schemaVersion": 2, "tool": { "name": "eaa-kit", "version": "0.1.0", @@ -182,18 +182,25 @@ at its limit. A tool deciding whether a clean report means anything should read fetched; an errored one was fetched and then could not be audited. They are different failures with different fixes, and summing them would name neither. -`completeness` was added after `schemaVersion` 1 and does not move it: new fields may +`completeness` was added without moving `schemaVersion`: new fields may appear without a bump, and consumers must ignore what they do not recognise. A consumer -written against version 1 that has never seen this field should treat its absence as -unknown rather than as a complete run. +written against an earlier version that has never seen this field should treat its absence +as unknown rather than as a complete run. ### `fingerprint` Each node carries the same identity the baseline file records and SARIF sends as a partial -fingerprint: a hash of the rule, the selector and the element's own markup, and deliberately -not of the page it was found on. Two consumers already depended on it agreeing — SARIF, so -that moving a page does not close one code-scanning alert and open an identical one, and -the baseline, so an accepted violation stays accepted when the surrounding page changes. +fingerprint: a hash of the rule, the selector and the element's own opening tag, and +deliberately not of the page it was found on. Two consumers already depended on it +agreeing — SARIF, so that moving a page does not close one code-scanning alert and open an +identical one, and the baseline, so an accepted violation stays accepted when the +surrounding page changes. + +The tag, not the element's whole markup. axe-core reports the failing element's outerHTML, +which for `<html>` — the element every document-level rule fails against — is the entire +page. Hashing that made `html-has-lang` and `document-title` change identity on any edit +to the page they were on, which is precisely what these two consumers exist to survive. +Where two elements share an opening tag, axe-core's selector already tells them apart. It is emitted so a third does not have to reimplement the hash and risk disagreeing with them. `eaa-kit diff` matches on it. @@ -213,7 +220,7 @@ the site — which is the same mistake as adding `inapplicable` to `passes`, and `notEvaluated` criteria a `--browser` run would decide. The remainder need a person whatever engine runs. -Added after `schemaVersion` 1 and does not move it. +Added without moving `schemaVersion`. ## Comparing two runs @@ -285,7 +292,7 @@ Real results from the fixture run above, abbreviated: } } ], - "partialFingerprints": { "eaaKit/v1": "4c2a13ab4c8c0365" } + "partialFingerprints": { "eaaKit/v2": "b6128eff391a4be6" } }, { "ruleId": "image-alt", @@ -300,7 +307,7 @@ Real results from the fixture run above, abbreviated: } } ], - "partialFingerprints": { "eaaKit/v1": "f0e17d2582e9a5b3" } + "partialFingerprints": { "eaaKit/v2": "f0e17d2582e9a5b3" } } ], "invocations": [{ "executionSuccessful": true, "toolExecutionNotifications": [] }], diff --git a/examples/baseline.json b/examples/baseline.json index 2703287..4db6532 100644 --- a/examples/baseline.json +++ b/examples/baseline.json @@ -1,11 +1,11 @@ { - "schemaVersion": 1, + "schemaVersion": 2, "createdOn": "2026-09-01", "entries": [ { "page": "index.html", "ruleId": "html-has-lang", - "fingerprint": "4c2a13ab4c8c0365", + "fingerprint": "b6128eff391a4be6", "selector": "html", "help": "<html> element must have a lang attribute", "impact": "serious", @@ -23,7 +23,7 @@ { "page": "index.html", "ruleId": "link-name", - "fingerprint": "e444708c0ffb67e0", + "fingerprint": "3943063f55791bff", "selector": "a", "help": "Links must have discernible text", "impact": "serious", diff --git a/examples/report.json b/examples/report.json index 2e4b709..8ff69d1 100644 --- a/examples/report.json +++ b/examples/report.json @@ -1,5 +1,5 @@ { - "schemaVersion": 1, + "schemaVersion": 2, "tool": { "name": "eaa-kit", "version": "0.4.0", @@ -1239,7 +1239,7 @@ "h1" ], "failureSummary": "Fix all of the following:\n Axe encountered an error; test the page for this type of problem manually", - "fingerprint": "b0b4dd68d2510979" + "fingerprint": "7ffc01343cb0cb30" } ], "reason": "engine-limitation", @@ -1347,7 +1347,7 @@ "h1" ], "failureSummary": "Fix all of the following:\n Axe encountered an error; test the page for this type of problem manually", - "fingerprint": "4113902b5dad38cd" + "fingerprint": "7ffc01343cb0cb30" } ], "reason": "engine-limitation", @@ -1455,7 +1455,7 @@ "p" ], "failureSummary": "Fix all of the following:\n Axe encountered an error; test the page for this type of problem manually", - "fingerprint": "3c34e40dd1b3284f" + "fingerprint": "14375e3de91b7aa0" } ], "reason": "engine-limitation", @@ -1562,7 +1562,7 @@ "html" ], "failureSummary": "Fix any of the following:\n The <html> element does not have a lang attribute", - "fingerprint": "4c2a13ab4c8c0365" + "fingerprint": "b6128eff391a4be6" } ] }, @@ -1590,7 +1590,7 @@ "a" ], "failureSummary": "Fix all of the following:\n Element is in tab order and does not have accessible text\n\nFix any of the following:\n Element does not have text that is visible to screen readers\n aria-label attribute does not exist or is empty\n aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty\n Element has no title attribute", - "fingerprint": "e444708c0ffb67e0" + "fingerprint": "3943063f55791bff" } ] } @@ -1606,7 +1606,7 @@ "html" ], "failureSummary": "Fix any of the following:\n No valid skip link found\n Page does not have a heading\n Page does not have a landmark region", - "fingerprint": "f65f3f04548c6741" + "fingerprint": "39d78859512e8246" } ], "reason": "needs-review", @@ -1622,7 +1622,7 @@ "p" ], "failureSummary": "Fix all of the following:\n Axe encountered an error; test the page for this type of problem manually", - "fingerprint": "e2edcc8d05a5ddea" + "fingerprint": "14375e3de91b7aa0" } ], "reason": "engine-limitation", @@ -1659,7 +1659,7 @@ "a" ], "failureSummary": null, - "fingerprint": "861d5c841eb71dae" + "fingerprint": "530d99c32a585135" } ], "reason": "engine-limitation", @@ -1741,7 +1741,7 @@ "p" ], "failureSummary": "Fix all of the following:\n Axe encountered an error; test the page for this type of problem manually", - "fingerprint": "e300b0a374169cf7" + "fingerprint": "14375e3de91b7aa0" } ], "reason": "engine-limitation", diff --git a/examples/report.sarif b/examples/report.sarif index 00cf47e..189d0ec 100644 --- a/examples/report.sarif +++ b/examples/report.sarif @@ -1117,7 +1117,7 @@ } ], "partialFingerprints": { - "eaaKit/v1": "4c2a13ab4c8c0365" + "eaaKit/v2": "b6128eff391a4be6" } }, { @@ -1138,7 +1138,7 @@ } ], "partialFingerprints": { - "eaaKit/v1": "f0e17d2582e9a5b3" + "eaaKit/v2": "f0e17d2582e9a5b3" } }, { @@ -1159,7 +1159,7 @@ } ], "partialFingerprints": { - "eaaKit/v1": "e444708c0ffb67e0" + "eaaKit/v2": "3943063f55791bff" } } ], diff --git a/src/audit/baseline.ts b/src/audit/baseline.ts index 7e7aa09..f208857 100644 --- a/src/audit/baseline.ts +++ b/src/audit/baseline.ts @@ -28,8 +28,18 @@ import { type Finding, type FindingNode, findingElements, type PageAudit } from * anything. Nothing here is permanent unless somebody keeps deciding it is. */ -/** Bumped only when an existing field is removed, renamed, or changes meaning. */ -export const BASELINE_SCHEMA_VERSION = 1 +/** + * Bumped only when an existing field is removed, renamed, or changes meaning. + * + * 2: `fingerprint` changed meaning. It used to hash the failing element's whole + * outerHTML, which for the document-level rules is the entire page — so every + * entry for `html-has-lang` or `document-title` stopped matching the moment + * anybody edited the page it was on, and the build went red on barriers that + * had been accepted. Entries written under 1 cannot be matched against 2, and + * silently accepting the file would suppress nothing while looking as though it + * had, so it is refused with the command that rewrites it. + */ +export const BASELINE_SCHEMA_VERSION = 2 /** Default filename, used by the CLI when no path is given. */ export const DEFAULT_BASELINE_FILE = 'eaa-baseline.json' @@ -261,7 +271,10 @@ export async function readBaseline(file: string, cwd = process.cwd()): Promise<B } if (result.data.schemaVersion !== BASELINE_SCHEMA_VERSION) { throw new BaselineError( - `${path.basename(target)} has schemaVersion ${result.data.schemaVersion}; this version of eaa-kit reads ${BASELINE_SCHEMA_VERSION}`, + `${path.basename(target)} has schemaVersion ${result.data.schemaVersion}; this version of eaa-kit reads ${BASELINE_SCHEMA_VERSION}.\n` + + ` Record it again on the current build: eaa-kit baseline\n` + + ' Read the new file before committing it: it lists what this run found, which is\n' + + ' not necessarily what the old one accepted.', ) } diff --git a/src/audit/collect.ts b/src/audit/collect.ts index 22dbaef..c62d9f8 100644 --- a/src/audit/collect.ts +++ b/src/audit/collect.ts @@ -95,6 +95,29 @@ export async function collectPages( return pages } +/** + * Whether the directory holds any HTML at all, ignoring include and exclude. + * + * Asked when a run collected nothing, to tell two different mistakes apart: a + * build directory with no pages in it, and a directory full of pages that the + * caller's own filters excluded. The advice for the first is wrong for the + * second — it names another directory to audit — so the question has to be + * answered before it is given. + * + * Globs rather than collecting: this only needs to know whether one file + * exists, and reading every page of a large build to answer that on a path that + * is about to print a warning and stop would be work for nothing. + */ +export async function holdsHtml(dir: string): Promise<boolean> { + const found = await glob([...DEFAULT_INCLUDE], { + cwd: path.resolve(dir), + ignore: [...DEFAULT_EXCLUDE], + onlyFiles: true, + dot: false, + }) + return found.length > 0 +} + async function assertDirectory(root: string, original: string): Promise<void> { let stats: Awaited<ReturnType<typeof stat>> try { diff --git a/src/audit/coverage.ts b/src/audit/coverage.ts index dfe58c8..f871d49 100644 --- a/src/audit/coverage.ts +++ b/src/audit/coverage.ts @@ -196,6 +196,16 @@ export function buildCoverage( const byCriterion = rulesByCriterion(tags) const decided = decidedRules(audits) const blinded = blindedRules(audits) + // ENGINE_BLIND_RULES is a fact about jsdom, not about auditing. Consulting it + // for a run that used real Chromium made the browser report contradict + // itself: colour contrast and target size came back as "this engine could not + // evaluate it", and the summary advised re-running with --browser — the flag + // that run was already using. A browser sees layout and CSS, so the only + // rules it reached no verdict on are the ones its own results said so about, + // which `blinded` already holds. + // An empty run keeps the table: there is no browser result to argue it away. + const jsdomBlindApplies = + audits.length === 0 || audits.some((audit) => audit.engine !== 'browser') const criteria: CriterionCoverage[] = WCAG22_AA_CRITERIA.map((criterion) => { const rules = byCriterion.get(criterion.number) ?? [] @@ -212,7 +222,7 @@ export function buildCoverage( // and matched nothing — different facts with different remedies, so they // are not folded together. const engineBlind = rules.filter( - (rule) => blinded.has(rule) || ENGINE_BLIND_RULES[rule] !== undefined, + (rule) => blinded.has(rule) || (jsdomBlindApplies && ENGINE_BLIND_RULES[rule] !== undefined), ) // One blind rule is enough. Requiring all of them was wrong on the one diff --git a/src/audit/diff.ts b/src/audit/diff.ts index 997e7c9..9e8ecfb 100644 --- a/src/audit/diff.ts +++ b/src/audit/diff.ts @@ -28,7 +28,7 @@ import { type ImpactLevel, impactRank, isImpactLevel } from './impact.ts' */ /** Reports older than this cannot be compared. */ -export const SUPPORTED_REPORT_SCHEMA = 1 +export const SUPPORTED_REPORT_SCHEMA = 2 export class DiffError extends Error { override readonly name = 'DiffError' @@ -268,7 +268,8 @@ export async function readReport(file: string, cwd = process.cwd()): Promise<Par if (result.data.schemaVersion !== SUPPORTED_REPORT_SCHEMA) { throw new DiffError( - `${file} is schemaVersion ${result.data.schemaVersion}; this version of eaa-kit reads ${SUPPORTED_REPORT_SCHEMA}`, + `${file} is schemaVersion ${result.data.schemaVersion}; this version of eaa-kit reads ${SUPPORTED_REPORT_SCHEMA}.\n` + + ' Write both reports with the same version of eaa-kit before comparing them.', ) } diff --git a/src/audit/fingerprint.ts b/src/audit/fingerprint.ts index 89d9790..4fb338d 100644 --- a/src/audit/fingerprint.ts +++ b/src/audit/fingerprint.ts @@ -3,7 +3,7 @@ import { createHash } from 'node:crypto' /** * A stable identity for one violating element. * - * Derived from the rule, the selector and the element's own markup, and + * Derived from the rule, the selector and the element's own opening tag, and * deliberately not from the file it was found in. Two consumers need this and * they need it to agree: SARIF, so that moving a page does not close one code * scanning alert and open an identical one, and the baseline, so that an @@ -14,5 +14,62 @@ import { createHash } from 'node:crypto' * large site unreadable. */ export function elementFingerprint(ruleId: string, selector: string, html: string): string { - return createHash('sha256').update(`${ruleId}\n${selector}\n${html}`).digest('hex').slice(0, 16) + return createHash('sha256') + .update(`${ruleId}\n${selector}\n${openingTag(html)}`) + .digest('hex') + .slice(0, 16) +} + +/** + * The element's own tag, without anything nested inside it. + * + * axe-core hands back the failing element's outerHTML, which for a leaf like + * `<img>` is the element and for a container is the element and every + * descendant it has. Hashing all of that made the identity of a container + * depend on its contents — and the container that matters here is `<html>`, + * which every document-level rule fails against: `html-has-lang`, + * `document-title`, `landmark-one-main`, `page-has-heading-one`. + * + * Their outerHTML is the whole page. So adding one paragraph anywhere changed + * the fingerprint of every document-level violation on that page, and all three + * consumers believed it: + * + * - the baseline stopped suppressing barriers it had accepted, and the build + * went red on a page whose only change was a typo fix; + * - `diff` reported the same untouched barrier as both new and **fixed**, which + * is the one thing it exists to refuse to do; + * - SARIF churned its partialFingerprints, so code scanning closed an alert and + * opened an identical one on every edit. + * + * The identity of an element is its own tag and attributes. Where two elements + * share those, axe-core's selector already tells them apart — it qualifies + * ambiguous matches with `:nth-child(…)` — so nothing that was distinguishable + * before stops being distinguishable now. + * + * Anything that is not an element — the empty string a rule with no attached + * node carries — is returned unchanged, so those keep the identity they had. + */ +export function openingTag(html: string): string { + const trimmed = html.trim() + if (!trimmed.startsWith('<')) return trimmed + + // Attribute values may contain `>`, so the scan tracks quoting rather than + // taking the first one it sees: `<a title="a > b">` is one tag, not two. + let quote: '"' | "'" | undefined + for (let i = 1; i < trimmed.length; i += 1) { + const char = trimmed[i] + if (quote !== undefined) { + if (char === quote) quote = undefined + continue + } + if (char === '"' || char === "'") { + quote = char + continue + } + if (char === '>') return trimmed.slice(0, i + 1) + } + + // No closing `>` at all: markup this malformed has no tag to take, so it is + // hashed as it came rather than silently becoming something else. + return trimmed } diff --git a/src/audit/report/console.ts b/src/audit/report/console.ts index e8317f0..5b001a1 100644 --- a/src/audit/report/console.ts +++ b/src/audit/report/console.ts @@ -313,23 +313,38 @@ function summary(audits: readonly PageAudit[], ctx: Context): string[] { ) const reviewCount = countRules(audits, 'needs-review') const pages = count(audits.length, 'page') + // A page that errored produced no findings because nothing read it, so it is + // not one of the pages a "no violations" sentence can be counted over. + const audited = audits.length - errored.length const lines = [line(ctx, 'Summary', ctx.c.bold), ...completenessLines(ctx)] - if (ruleCount === 0) { + if (ruleCount === 0 && audited === 0 && errored.length > 0) { + // Every page this run was given failed. "No violations" here would be a + // pass handed back for markup nothing ever opened, so the count is not + // printed as a verdict at all and the error line below carries the result. + // A run with no pages is a different thing and keeps its own wording: there + // was nothing to fail. + lines.push(line(ctx, ' Nothing was audited: no page could be read.', ctx.c.red)) + } else if (ruleCount === 0) { // Qualified rather than plain when the run did not see the whole site: "no // violations" over a fraction of the pages is not the sentence it looks // like, and the completeness lines above have just said which fraction. - const clean = - ctx.completeness && !ctx.completeness.complete - ? ` No violations across the ${pages} that were audited.` - : ` No violations across ${pages}.` + // The count is of pages actually audited, not of pages attempted: saying + // "no violations across 2 pages" when one of them errored claims a verdict + // on a page nothing looked at. + const incomplete = errored.length > 0 || (ctx.completeness && !ctx.completeness.complete) + const clean = incomplete + ? ` No violations across the ${count(audited, 'page')} that were audited.` + : ` No violations across ${pages}.` lines.push(line(ctx, clean, ctx.c.green)) } else { lines.push( render(ctx, [ { - text: ` ${count(ruleCount, 'violation')} on ${withViolations.length} of ${pages}`, + // Out of the pages audited, not the pages attempted: "on 1 of 2 + // pages" reads as one clean page when the other one errored. + text: ` ${count(ruleCount, 'violation')} on ${withViolations.length} of ${count(audited, 'page')}`, paint: ctx.c.red, }, { text: ` (${count(elementCount, 'element')})`, paint: ctx.c.dim }, @@ -441,10 +456,11 @@ function completenessLines(ctx: Context): string[] { if (completeness.unreachable.length > 0) { const noun = plural(completeness.unreachable.length, 'page') + const verb = completeness.unreachable.length === 1 ? 'was' : 'were' lines.push( line( ctx, - ` ${completeness.unreachable.length} ${noun} could not be reached, and were not audited`, + ` ${completeness.unreachable.length} ${noun} could not be reached, and ${verb} not audited`, ctx.c.yellow, ), ) diff --git a/src/audit/report/json.ts b/src/audit/report/json.ts index 8884f63..3f81ee5 100644 --- a/src/audit/report/json.ts +++ b/src/audit/report/json.ts @@ -11,8 +11,15 @@ import type { Finding, IncompleteFinding, PageAudit } from '../runners/jsdom.ts' * Bumped only when an existing field is removed, renamed, or changes meaning. * Adding a new field does not bump it, so consumers must ignore fields they do * not know. See the JSON report contract in the README. + * + * 2: `fingerprint` changed meaning. It used to hash the failing element's whole + * outerHTML, which for `<html>` is the entire page, so every document-level + * violation changed identity whenever anybody edited the page it was on. + * `diff` matches on this field, and comparing a report written under 1 against + * one written under 2 would report every such barrier as both new and fixed — + * the reading the version check exists to refuse. */ -export const SCHEMA_VERSION = 1 +export const SCHEMA_VERSION = 2 export type ReportEngine = 'jsdom' | 'browser' diff --git a/src/audit/report/sarif.ts b/src/audit/report/sarif.ts index b8f2fb8..73ae7dc 100644 --- a/src/audit/report/sarif.ts +++ b/src/audit/report/sarif.ts @@ -210,7 +210,11 @@ function toResults( * combines the fingerprint with the location itself. */ function fingerprint(ruleId: string, selector: string, html: string): Record<string, string> { - return { 'eaaKit/v1': elementFingerprint(ruleId, selector, html) } + // v2: the fingerprint stopped hashing the element's descendants, so a + // document-level alert survives an edit elsewhere on the page instead of + // being closed and reopened. The version in the key is what tells code + // scanning these are a different scheme rather than moved defects. + return { 'eaaKit/v2': elementFingerprint(ruleId, selector, html) } } /** Every rule the run knows about, so the catalogue is complete in GitHub. */ diff --git a/src/cli/audit.ts b/src/cli/audit.ts index 1f7401d..0a9d358 100644 --- a/src/cli/audit.ts +++ b/src/cli/audit.ts @@ -78,7 +78,7 @@ export async function runAuditCommand( ): Promise<AuditCommandResult> { const resolved = await resolvePages(dir, options) if (!resolved) return { audits: [], exitCode: 2 } - const { pages, origin, label, cleanup, completeness: collection } = resolved + const { pages, origin, label, cleanup, directory, completeness: collection } = resolved // try/finally rather than a call before each return: auto-detection may have // started the project's server, and leaving it running would hold the process // open after the report is written. @@ -97,9 +97,16 @@ export async function runAuditCommand( ...(options.timeoutMs === undefined ? {} : { timeoutMs: options.timeoutMs }), ...(options.browser ? { browser: true } : {}), ...(options.concurrency === undefined ? {} : { concurrency: options.concurrency }), - // No directory when the pages were crawled: they are audited at the URL + // The directory the pages were actually read from, not the one the + // caller typed: under auto-detection nobody typed one, and passing + // undefined told the browser runner these pages had been crawled. It + // then skipped the loopback server and navigated Chromium to a bare + // filesystem path, which is not a URL — so `eaa-kit audit --browser` + // with no directory argument failed every page it was given. + // + // Still undefined for a real crawl: those pages are audited at the URL // they came from, not served back out of a copy on disk. - ...(options.url === undefined && dir !== undefined ? { directory: dir } : {}), + ...(directory === undefined ? {} : { directory }), }) if (!audits) return { audits: [], exitCode: 2 } diff --git a/src/cli/baseline.ts b/src/cli/baseline.ts index a879641..dcb6288 100644 --- a/src/cli/baseline.ts +++ b/src/cli/baseline.ts @@ -51,7 +51,7 @@ export async function runBaselineCommand( const resolved = await resolvePages(path.resolve(cwd, dir), { ...options, label: dir }) if (!resolved) return { entries: 0, exitCode: 2 } - const { pages, origin, label } = resolved + const { pages, origin, label, directory } = resolved note(`Auditing ${count(pages.length, 'page')} in ${label}…`) @@ -62,7 +62,9 @@ export async function runBaselineCommand( ...(options.timeoutMs === undefined ? {} : { timeoutMs: options.timeoutMs }), ...(options.browser ? { browser: true } : {}), ...(options.concurrency === undefined ? {} : { concurrency: options.concurrency }), - ...(options.url === undefined ? { directory: path.resolve(cwd, dir) } : {}), + // Where the pages were read from, as the collection stage resolved it. + // Undefined for a crawl, whose pages are audited at their own URL. + ...(directory === undefined ? {} : { directory }), }) if (!audits) return { entries: 0, exitCode: 2 } diff --git a/src/cli/pages.ts b/src/cli/pages.ts index d29c831..92dc233 100644 --- a/src/cli/pages.ts +++ b/src/cli/pages.ts @@ -4,6 +4,7 @@ import { type CollectedPage, collectPages, emptyDirectoryHint, + holdsHtml, } from '../audit/collect.ts' import type { Collection, Unmeasured } from '../audit/completeness.ts' import { count } from '../text.ts' @@ -52,6 +53,18 @@ export interface ResolvePagesOptions extends CrawlCommandOptions { export interface ResolvedPages { pages: CollectedPage[] + /** + * Directory the pages were read off disk from, absolute or as the caller + * gave it. Undefined for a crawl, whose pages have a server of their own. + * + * Carried rather than left to the caller's own argument, because under + * auto-detection there is no argument: the caller passed undefined and this + * stage worked the directory out. A caller that reached for its own `dir` + * got undefined there and treated a build on disk as though it had been + * crawled — which is exactly what the browser runner uses to decide whether + * to serve the pages over loopback or navigate to them directly. + */ + directory?: string /** Stops anything auto-detection started, once the report is written. */ cleanup?: () => Promise<void> /** @@ -129,6 +142,21 @@ export async function resolvePages( } if (pages.length === 0) { + // A build directory full of HTML that the globs excluded is not the same + // mistake as one with no HTML in it, and the framework advice for the + // second is actively wrong for the first: it names another directory to + // audit when the directory was never the problem. So the filters are + // checked before that advice is offered. + const filtered = + (options.include !== undefined || options.exclude !== undefined) && + (await holdsHtml(directory as string)) + if (filtered) { + warn(`No page in ${shown} matched the filters, so nothing was audited.`) + if (options.include !== undefined) note(` --include ${options.include.join(' ')}`) + if (options.exclude !== undefined) note(` --exclude ${options.exclude.join(' ')}`) + note(' Patterns are relative to the audited directory, with POSIX separators.') + return undefined + } warn(await emptyDirectoryHint(shown, cwd)) return undefined } @@ -144,6 +172,7 @@ export async function resolvePages( return { pages, + directory: directory as string, label: shown, completeness: { discovery: 'directory', diff --git a/src/statement/findings.ts b/src/statement/findings.ts index fc1c2c3..0f159ec 100644 --- a/src/statement/findings.ts +++ b/src/statement/findings.ts @@ -10,7 +10,7 @@ import { StatementError } from './error.ts' * axe-core and the statement path deliberately does not. A test asserts the two * stay in step. */ -export const SUPPORTED_REPORT_SCHEMA = 1 +export const SUPPORTED_REPORT_SCHEMA = 2 /** * Only the fields the statement reads. Everything else in the report — node diff --git a/tests/astro/integration.test.ts b/tests/astro/integration.test.ts index e430afc..ceda2fb 100644 --- a/tests/astro/integration.test.ts +++ b/tests/astro/integration.test.ts @@ -9,6 +9,7 @@ import eaaKit, { type AstroIntegrationLike, type AstroLoggerLike, } from '../../src/astro/index.ts' +import { SCHEMA_VERSION } from '../../src/audit/report/json.ts' /** * The structural types in src/astro are a stand-in for Astro's own, so that the @@ -192,7 +193,7 @@ describe('auditing the build', () => { await run(eaaKit({ format: 'json', output }), dir, recorder().logger).catch(() => undefined) const doc = JSON.parse(await readFile(output, 'utf8')) - expect(doc.schemaVersion).toBe(1) + expect(doc.schemaVersion).toBe(SCHEMA_VERSION) expect(doc.summary.violations).toBeGreaterThan(0) }) diff --git a/tests/audit/baseline.test.ts b/tests/audit/baseline.test.ts index f4b5f03..04448b0 100644 --- a/tests/audit/baseline.test.ts +++ b/tests/audit/baseline.test.ts @@ -12,7 +12,6 @@ import { serialiseBaseline, writeBaseline, } from '../../src/audit/baseline.ts' -import { elementFingerprint } from '../../src/audit/fingerprint.ts' import type { Finding, PageAudit } from '../../src/audit/result.ts' const TODAY = new Date('2026-08-27T10:00:00.000Z') @@ -265,7 +264,7 @@ describe('applyBaseline', () => { }) it('accepts nothing from an empty baseline', () => { - const empty: Baseline = { schemaVersion: 1, createdOn: '', entries: [] } + const empty: Baseline = { schemaVersion: BASELINE_SCHEMA_VERSION, createdOn: '', entries: [] } const outcome = applyBaseline(audits, empty, { today: TODAY }) @@ -301,7 +300,9 @@ describe('the file', () => { it('declares its schema version', () => { expect(baselineFor([]).schemaVersion).toBe(BASELINE_SCHEMA_VERSION) - expect(serialiseBaseline(baselineFor([]))).toContain('"schemaVersion": 1') + expect(serialiseBaseline(baselineFor([]))).toContain( + `"schemaVersion": ${BASELINE_SCHEMA_VERSION}`, + ) }) it('says how to make one when it is not there', async () => { @@ -338,25 +339,3 @@ describe('the file', () => { await expect(readBaseline('b.json', dir)).rejects.toThrow(/schemaVersion 99/) }) }) - -describe('elementFingerprint', () => { - it('is stable for the same element', () => { - expect(elementFingerprint('image-alt', 'img', '<img>')).toBe( - elementFingerprint('image-alt', 'img', '<img>'), - ) - }) - - it('changes when the rule, the selector or the markup changes', () => { - const base = elementFingerprint('image-alt', 'img', '<img>') - - expect(elementFingerprint('link-name', 'img', '<img>')).not.toBe(base) - expect(elementFingerprint('image-alt', 'img.x', '<img>')).not.toBe(base) - expect(elementFingerprint('image-alt', 'img', '<img alt="">')).not.toBe(base) - }) - - it('does not depend on the page, so a moved page keeps its baseline', () => { - // The same promise SARIF relies on: moving a file must not close one alert - // and open an identical one. - expect(elementFingerprint('image-alt', 'img', '<img>')).toHaveLength(16) - }) -}) diff --git a/tests/audit/coverage.test.ts b/tests/audit/coverage.test.ts index 7a7f06b..282c0af 100644 --- a/tests/audit/coverage.test.ts +++ b/tests/audit/coverage.test.ts @@ -133,6 +133,73 @@ describe('buildCoverage', () => { expect(contradicted.map((criterion) => criterion.number)).toEqual([]) }) + it("does not hold jsdom's blind spots against a run that used a browser", () => { + // ENGINE_BLIND_RULES describes jsdom, which has no layout. Applying it to a + // browser run made the report contradict itself and then give advice it had + // already taken: colour contrast came back as "this engine could not + // evaluate it" from a run in real Chromium, under a summary telling the + // reader to re-run with --browser. + // What a browser run looks like: it can see layout, so it force-reports + // nothing as unevaluable and its incomplete list carries no + // engine-limitation at all. + const browserRun = audits.map((audit) => ({ + ...audit, + engine: 'browser' as const, + incomplete: audit.incomplete.filter((finding) => finding.reason !== 'engine-limitation'), + })) + + const browserCoverage = buildCoverage(browserRun) + const contrast = browserCoverage.criteria.find((criterion) => criterion.number === '1.4.3') + + expect(contrast?.status).not.toBe('not-evaluated') + expect(contrast?.browserWouldAnswer).toBe(false) + expect(browserCoverage.browserWouldAnswer).toBe(0) + }) + + it('still holds them against a run that used jsdom', () => { + // The other direction: the advice is worth giving to the engine it applies + // to, and switching it off everywhere would have been the easier fix and + // the wrong one. + expect(coverage.browserWouldAnswer).toBeGreaterThan(0) + }) + + it('believes a browser run that says a rule reached no verdict', () => { + // A browser can still fail to decide a rule — axe-core reports it as + // incomplete — and that is a fact about the run rather than about jsdom, so + // it survives. + const [first, ...rest] = audits + if (first === undefined) throw new Error('fixture produced no audits') + const browserRun = [ + { + ...first, + engine: 'browser' as const, + passes: first.passes.filter((outcome) => outcome.ruleId !== 'color-contrast'), + violations: first.violations.filter((finding) => finding.ruleId !== 'color-contrast'), + incomplete: [ + { + ruleId: 'color-contrast', + help: 'Elements must meet minimum colour contrast ratio thresholds', + helpUrl: '', + impact: 'serious' as const, + successCriteria: ['1.4.3'], + enClauses: [], + tags: ['wcag2aa', 'wcag143'], + reason: 'engine-limitation' as const, + reasonDetail: 'could not resolve a background colour', + nodes: [], + }, + ], + }, + ...rest.map((audit) => ({ ...audit, engine: 'browser' as const })), + ] + + const contrast = buildCoverage(browserRun).criteria.find( + (criterion) => criterion.number === '1.4.3', + ) + + expect(contrast?.status).toBe('not-evaluated') + }) + it('never claims a browser would answer a criterion no rule covers', () => { for (const criterion of coverage.criteria) { if (criterion.status === 'no-automated-rule') { diff --git a/tests/audit/diff.test.ts b/tests/audit/diff.test.ts index ac95375..b31015b 100644 --- a/tests/audit/diff.test.ts +++ b/tests/audit/diff.test.ts @@ -25,7 +25,7 @@ interface PageSpec { /** A JSON report with only the fields a diff reads. */ function report(pages: PageSpec[], overrides: Record<string, unknown> = {}): unknown { return { - schemaVersion: 1, + schemaVersion: SCHEMA_VERSION, generatedAt: '2026-08-20T18:00:00.000Z', target: { source: './dist' }, completeness: { complete: true, audited: pages.filter((p) => !p.error).length }, @@ -149,7 +149,7 @@ describe('diffReports', () => { // A rule can fail with nothing attached. Getting this wrong would make such // failures invisible to a diff while still visible to the baseline. const withNoNodes = { - schemaVersion: 1, + schemaVersion: SCHEMA_VERSION, generatedAt: '2026-08-20T18:00:00.000Z', pages: [{ path: 'index.html', violations: [{ ruleId: 'html-has-lang', nodes: [] }] }], } @@ -183,9 +183,9 @@ describe('diffReports', () => { describe('readReport', () => { it('refuses a schema version it was not written against', async () => { - await expect(parsed(report([{ path: 'index.html' }], { schemaVersion: 2 }))).rejects.toThrow( - DiffError, - ) + await expect( + parsed(report([{ path: 'index.html' }], { schemaVersion: SCHEMA_VERSION + 1 })), + ).rejects.toThrow(DiffError) }) it('refuses a file that is not a report', async () => { diff --git a/tests/audit/fingerprint.test.ts b/tests/audit/fingerprint.test.ts new file mode 100644 index 0000000..82092ca --- /dev/null +++ b/tests/audit/fingerprint.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest' +import { elementFingerprint, openingTag } from '../../src/audit/fingerprint.ts' + +/** + * The identity of a violating element, and the one property everything built on + * it needs: it must not move when the page around the element does. + * + * The baseline, `diff` and SARIF all key off this. When it was the element's + * whole outerHTML, the document-level rules — whose element is `<html>` and + * whose outerHTML is therefore the entire page — changed identity on every + * edit, and all three consumers reported nonsense as a result. + */ + +describe('openingTag', () => { + it('takes the element and leaves its descendants out', () => { + expect(openingTag('<a href="/x"><img src="1.png"></a>')).toBe('<a href="/x">') + }) + + it('keeps a leaf element whole, because it has no descendants to drop', () => { + expect(openingTag('<img src="/logo.svg">')).toBe('<img src="/logo.svg">') + }) + + it('does not stop at a > inside an attribute value', () => { + expect(openingTag(`<a title="a > b" href="/x">go</a>`)).toBe(`<a title="a > b" href="/x">`) + expect(openingTag(`<a title='a > b'>go</a>`)).toBe(`<a title='a > b'>`) + }) + + it('leaves markup with no closing bracket as it found it', () => { + // Nothing to take, and inventing a tag here would give two different + // fragments the same identity. + expect(openingTag('<img src="/a.png"')).toBe('<img src="/a.png"') + }) + + it('passes through what is not an element', () => { + // The empty string is the identity a rule that failed with no attached node + // carries, and the baseline writes an entry against exactly that. + expect(openingTag('')).toBe('') + expect(openingTag(' ')).toBe('') + expect(openingTag('bare text')).toBe('bare text') + }) +}) + +describe('elementFingerprint', () => { + it('is stable for the same element', () => { + expect(elementFingerprint('image-alt', 'img', '<img>')).toBe( + elementFingerprint('image-alt', 'img', '<img>'), + ) + }) + + it('changes when the rule, the selector or the element itself changes', () => { + const base = elementFingerprint('image-alt', 'img', '<img>') + + expect(elementFingerprint('link-name', 'img', '<img>')).not.toBe(base) + expect(elementFingerprint('image-alt', 'img.x', '<img>')).not.toBe(base) + expect(elementFingerprint('image-alt', 'img', '<img alt="">')).not.toBe(base) + }) + + it('does not move when the page inside a document-level element changes', () => { + // The regression this exists for. `html-has-lang` fails against `<html>`, + // so its outerHTML is the whole document: hashing that made adding one + // paragraph anywhere look like a different barrier, which the baseline + // stopped suppressing and `diff` reported as both new and fixed. + const before = elementFingerprint( + 'html-has-lang', + 'html', + '<html><head><title>

Kontakt

', + ) + const after = elementFingerprint( + 'html-has-lang', + 'html', + '

Kontakt

New.

', + ) + + expect(after).toBe(before) + }) + + it('still moves when the document element itself is fixed', () => { + // The other half: a barrier that was actually addressed has to change + // identity, or `diff` could never report anything as fixed. + const broken = elementFingerprint('html-has-lang', 'html', '') + const fixed = elementFingerprint( + 'html-has-lang', + 'html', + '', + ) + + expect(fixed).not.toBe(broken) + }) + + it('tells two containers apart by the selector axe-core gives them', () => { + // Dropping the descendants costs nothing here: axe-core qualifies an + // ambiguous match with :nth-child(…), so the two links stay distinct. + const first = elementFingerprint('link-name', 'a:nth-child(1)', '') + const second = elementFingerprint('link-name', 'a:nth-child(2)', '') + + expect(first).not.toBe(second) + }) +}) diff --git a/tests/audit/report/console.test.ts b/tests/audit/report/console.test.ts index 94bd639..1004332 100644 --- a/tests/audit/report/console.test.ts +++ b/tests/audit/report/console.test.ts @@ -300,6 +300,44 @@ describe('the completeness lines', () => { expect(report).toContain('2 pages could not be reached') }) + it('counts one unreachable page in the singular', () => { + const report = summaryOf({ unreachable: [{ location: '/a', reason: '404' }] }) + + expect(report).toContain('1 page could not be reached, and was not audited') + }) + + it('does not count an errored page among the pages it found nothing on', () => { + // "No violations across 2 pages" over a run where one page could not be + // read hands back a verdict on markup nothing opened. The count is of what + // was audited; the error line beneath it says what was not. + const pages = [clean(), { ...clean(), relativePath: 'broken.html', error: 'boom' }] + + const report = formatConsoleReport(pages, { + color: false, + width: 80, + completeness: runCompleteness(pages, completeCollection('directory', 2)), + }) + + expect(report).toContain('No violations across the 1 page that were audited.') + expect(report).not.toContain('No violations across 2 pages') + }) + + it('refuses to call a run clean when every page failed', () => { + // The shape `audit --browser` produced when it navigated to a filesystem + // path instead of a URL: every page errored, nothing was read, and the + // summary opened with "No violations" in green. + const pages = [ + { ...clean(), relativePath: 'a.html', error: 'boom' }, + { ...clean(), relativePath: 'b.html', error: 'boom' }, + ] + + const report = formatConsoleReport(pages, { color: false, width: 80 }) + + expect(report).toContain('Nothing was audited: no page could be read.') + expect(report).not.toContain('No violations') + expect(report).toContain('2 pages could not be audited') + }) + it('leaves the errored count to the summary rather than double-reporting it', () => { const pages = [clean(), { ...clean(), relativePath: 'broken.html', error: 'boom' }] const report = formatConsoleReport(pages, { diff --git a/tests/audit/report/json.test.ts b/tests/audit/report/json.test.ts index 164b2da..12c1976 100644 --- a/tests/audit/report/json.test.ts +++ b/tests/audit/report/json.test.ts @@ -48,7 +48,6 @@ beforeAll(async () => { describe('document envelope', () => { it('declares a schema version', () => { expect(report.schemaVersion).toBe(SCHEMA_VERSION) - expect(report.schemaVersion).toBe(1) }) it('names the tool, its version and the axe-core it wrapped', () => { @@ -260,7 +259,7 @@ describe('stability of the contract', () => { const serialised = serialiseJsonReport(report) expect(serialised.endsWith('}\n')).toBe(true) - expect(serialised).toContain('\n "schemaVersion": 1') + expect(serialised).toContain(`\n "schemaVersion": ${SCHEMA_VERSION}`) expect(JSON.parse(serialised)).toEqual(report) }) @@ -305,7 +304,7 @@ describe('the target block', () => { expect(report.target).toMatchObject({ source: 'http://localhost:3000', kind: 'url' }) }) - it('keeps schemaVersion 1, because no v1 report could have looked different', () => { + it('does not move the version for a nullable directory, an added-field change', () => { // Adding fields does not move the version. `directory` becoming nullable // only shows up for crawls, which did not exist under v1 at all, so every // report shape a v1 consumer could already receive is unchanged. @@ -317,7 +316,7 @@ describe('the target block', () => { now: NOW, }) - expect(report.schemaVersion).toBe(1) + expect(report.schemaVersion).toBe(SCHEMA_VERSION) }) }) @@ -361,6 +360,5 @@ describe('the completeness block', () => { it('does not move the schema version, being an added field', () => { expect(report.schemaVersion).toBe(SCHEMA_VERSION) - expect(SCHEMA_VERSION).toBe(1) }) }) diff --git a/tests/audit/report/sarif.test.ts b/tests/audit/report/sarif.test.ts index e97f94c..e70f9b7 100644 --- a/tests/audit/report/sarif.test.ts +++ b/tests/audit/report/sarif.test.ts @@ -199,7 +199,7 @@ describe('results', () => { it('fingerprints results so alerts survive a file move', () => { for (const result of log.runs[0]?.results ?? []) { - expect(result.partialFingerprints['eaaKit/v1']).toMatch(/^[0-9a-f]{16}$/) + expect(result.partialFingerprints['eaaKit/v2']).toMatch(/^[0-9a-f]{16}$/) } }) diff --git a/tests/audit/runners/playwright.test.ts b/tests/audit/runners/playwright.test.ts index b4fdec1..e95773c 100644 --- a/tests/audit/runners/playwright.test.ts +++ b/tests/audit/runners/playwright.test.ts @@ -142,3 +142,44 @@ describe('runBrowserAudit', () => { expect(audit?.violations).toEqual([]) }, 120_000) }) + +describe('audit --browser, end to end', () => { + /** + * The headline invocation: no directory argument, so auto-detection finds the + * build. It used to hand the browser runner no directory at all, which is the + * runner's signal that these pages came off a running site and already have + * somewhere to be fetched from — so it skipped the loopback server and + * navigated Chromium to `/tmp/…/index.html`, a filesystem path and not a URL. + * Every page errored, and the summary opened with "No violations". + */ + it('audits the build it found rather than failing every page', async () => { + const { runAuditCommand } = await import('../../../src/cli/audit.ts') + const project = await mkdtemp(path.join(tmpdir(), 'eaa-kit-browser-cli-')) + await writeFile(path.join(project, 'package.json'), '{"name":"site","private":true}', 'utf8') + await mkdir(path.join(project, 'dist'), { recursive: true }) + await mkdir(path.join(project, 'dist', 'assets'), { recursive: true }) + await writeFile(path.join(project, 'dist', 'assets', 'site.css'), STYLES, 'utf8') + await writeFile(path.join(project, 'dist', 'index.html'), PAGE, 'utf8') + + const writes = { stdout: process.stdout.write, stderr: process.stderr.write } + process.stdout.write = (() => true) as typeof process.stdout.write + process.stderr.write = (() => true) as typeof process.stderr.write + try { + const { audits } = await runAuditCommand(undefined, { + cwd: project, + browser: true, + noBuild: true, + }) + + expect(audits).toHaveLength(1) + expect(audits[0]?.error).toBeUndefined() + // Served over loopback, so the stylesheet loaded and the rule that needs + // it reached a verdict — the whole point of passing the directory along. + expect(audits[0]?.violations.map((finding) => finding.ruleId)).toContain('color-contrast') + } finally { + process.stdout.write = writes.stdout + process.stderr.write = writes.stderr + await rm(project, { recursive: true, force: true }) + } + }, 180_000) +}) diff --git a/tests/cli/audit.test.ts b/tests/cli/audit.test.ts index 4dc3031..e9253c1 100644 --- a/tests/cli/audit.test.ts +++ b/tests/cli/audit.test.ts @@ -6,6 +6,7 @@ import path from 'node:path' import { fileURLToPath } from 'node:url' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { IMPACT_LEVELS, type ImpactLevel } from '../../src/audit/impact.ts' +import { SCHEMA_VERSION } from '../../src/audit/report/json.ts' import { runAuditCommand } from '../../src/cli/audit.ts' const SITE = fileURLToPath(new URL('../fixtures/site', import.meta.url)) @@ -99,7 +100,7 @@ describe('--format and --output', () => { await runAuditCommand(IMPACTS, { include: ['critical.html'], format: 'json' }) const document = JSON.parse(stdout.join('')) - expect(document.schemaVersion).toBe(1) + expect(document.schemaVersion).toBe(SCHEMA_VERSION) expect(document.pages[0].path).toBe('critical.html') expect(stdout.join('')).not.toContain('eaa-kit audit') }, 60_000) @@ -115,7 +116,7 @@ describe('--format and --output', () => { }) const written = JSON.parse(await readFile(target, 'utf8')) - expect(written.schemaVersion).toBe(1) + expect(written.schemaVersion).toBe(SCHEMA_VERSION) expect(written.summary.violations).toBe(1) expect(stdout.join('')).toBe('') expect(stderr.join('')).toContain('Report written to') @@ -229,6 +230,19 @@ describe('runAuditCommand', () => { expect(stderr.join('')).toContain('could not be audited') }, 60_000) + it('says the filters excluded everything, not that the build is empty', async () => { + // The directory is full of HTML; --include is what left nothing. Saying it + // "holds no HTML files" is false, and the advice that follows that + // sentence names another directory to audit — sending somebody to fix a + // path that was never wrong. + const { exitCode } = await runAuditCommand(SITE, { include: ['nope/**'] }) + + expect(exitCode).toBe(2) + expect(stderr.join('')).toContain('matched the filters') + expect(stderr.join('')).toContain('--include nope/**') + expect(stderr.join('')).not.toContain('holds no HTML') + }) + it('exits 2 when the directory holds no HTML', async () => { const empty = await mkdtemp(path.join(tmpdir(), 'eaa-kit-cli-')) await writeFile(path.join(empty, 'app.js'), 'export {}', 'utf8') diff --git a/tests/cli/baseline.test.ts b/tests/cli/baseline.test.ts index d319cf2..434d51f 100644 --- a/tests/cli/baseline.test.ts +++ b/tests/cli/baseline.test.ts @@ -3,7 +3,8 @@ import { tmpdir } from 'node:os' import path from 'node:path' import { fileURLToPath } from 'node:url' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { readBaseline } from '../../src/audit/baseline.ts' +import { BASELINE_SCHEMA_VERSION, readBaseline } from '../../src/audit/baseline.ts' +import { SCHEMA_VERSION } from '../../src/audit/report/json.ts' import { runAuditCommand } from '../../src/cli/audit.ts' import { runBaselineCommand } from '../../src/cli/baseline.ts' @@ -56,7 +57,9 @@ describe('runBaselineCommand', () => { expect(exitCode).toBe(0) expect(entries).toBe(3) - expect(await readBaseline('eaa-baseline.json', dir)).toMatchObject({ schemaVersion: 1 }) + expect(await readBaseline('eaa-baseline.json', dir)).toMatchObject({ + schemaVersion: BASELINE_SCHEMA_VERSION, + }) }) it('says the file is a list of barriers, not a way to switch the tool off', async () => { @@ -106,13 +109,27 @@ describe('runBaselineCommand', () => { expect(stderr.join('')).toContain('Build directory not found') }) - it('exits 2 when there is no HTML to audit', async () => { + it('exits 2 when the filters leave nothing to audit, and says so', async () => { + // The build is full of HTML; --include is what left none of it. Reporting + // that the directory "holds no HTML" would be false, and the framework + // advice that follows it names a different directory to try — a fix for a + // path that was never wrong. const dir = await site() const { exitCode } = await runBaselineCommand('dist', { cwd: dir, include: ['**/*.xhtml'] }) expect(exitCode).toBe(2) - expect(stderr.join('')).toContain('holds no HTML') + expect(stderr.join('')).toContain('matched the filters') + expect(stderr.join('')).not.toContain('holds no HTML') + }) + + it('exits 2 when the directory really does hold no HTML', async () => { + const dir = await site() + await writeFile(path.join(dir, 'empty.txt'), '', 'utf8') + + const { exitCode } = await runBaselineCommand('.', { cwd: dir, include: ['nothing/**'] }) + + expect(exitCode).toBe(2) }) }) @@ -227,7 +244,7 @@ describe('audit --baseline', () => { }) expect(JSON.parse(await readFile(path.join(dir, 'reports/a11y.json'), 'utf8'))).toMatchObject({ - schemaVersion: 1, + schemaVersion: SCHEMA_VERSION, }) }) diff --git a/tests/cli/diff.test.ts b/tests/cli/diff.test.ts index 55cdbd7..1bb6612 100644 --- a/tests/cli/diff.test.ts +++ b/tests/cli/diff.test.ts @@ -2,6 +2,7 @@ import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { SCHEMA_VERSION } from '../../src/audit/report/json.ts' import { runDiffCommand } from '../../src/cli/diff.ts' const dirs: string[] = [] @@ -42,7 +43,7 @@ async function reports( dirs.push(dir) const document = (violations: Violation[], pages: string[]): unknown => ({ - schemaVersion: 1, + schemaVersion: SCHEMA_VERSION, generatedAt: '2026-08-20T18:00:00.000Z', target: { source: './dist' }, completeness: { complete: true, audited: pages.length }, diff --git a/tests/cli/pages.test.ts b/tests/cli/pages.test.ts new file mode 100644 index 0000000..0f4ada6 --- /dev/null +++ b/tests/cli/pages.test.ts @@ -0,0 +1,80 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { resolvePages } from '../../src/cli/pages.ts' + +/** + * Where the pages come from, and what the caller is told about it. + * + * The directory this stage settles on is not always the one the caller passed: + * `eaa-kit audit` with no argument passes nothing and lets auto-detection work + * it out. A caller that reached for its own argument instead got undefined, and + * the browser runner reads exactly that to decide whether these pages live on + * disk and need serving over loopback or came off a site of their own. + */ + +let project: string + +beforeEach(async () => { + project = await mkdtemp(path.join(tmpdir(), 'eaa-kit-pages-')) + await writeFile(path.join(project, 'package.json'), '{"name":"site","private":true}', 'utf8') + await mkdir(path.join(project, 'dist'), { recursive: true }) + await writeFile( + path.join(project, 'dist', 'index.html'), + 'Home

Home

', + 'utf8', + ) + // Progress goes to stderr and is not what these cases are about. + vi.spyOn(process.stderr, 'write').mockImplementation(() => true) +}) + +afterEach(async () => { + vi.restoreAllMocks() + await rm(project, { recursive: true, force: true }) +}) + +describe('resolvePages', () => { + it('reports the directory it worked out when nobody named one', async () => { + // The regression: `audit --browser` with no directory argument navigated + // Chromium to a bare filesystem path — which is not a URL — because this + // was the only thing that knew where the build was, and it did not say. + // Every page came back errored, under a summary reading "No violations". + const resolved = await resolvePages(undefined, { cwd: project, noBuild: true }) + + expect(resolved?.pages).toHaveLength(1) + expect(resolved?.directory).toBe(path.join(project, 'dist')) + }) + + it('reports the directory it was given', async () => { + const dir = path.join(project, 'dist') + + const resolved = await resolvePages(dir, { cwd: project }) + + expect(resolved?.directory).toBe(dir) + }) + + it('reports no directory for a crawl, whose pages are not read off disk', async () => { + // Serving a crawled page back out of a copy on disk would audit the markup + // with the server that produced it cut out of the picture, so the browser + // runner has to be able to tell the two apart. + const { createServer } = await import('node:http') + const server = createServer((_request, response) => { + response.writeHead(200, { 'content-type': 'text/html' }) + response.end( + 'Aa', + ) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const { port } = server.address() as { port: number } + + try { + const resolved = await resolvePages(undefined, { url: `http://127.0.0.1:${port}/` }) + + expect(resolved?.pages.length).toBeGreaterThan(0) + expect(resolved?.directory).toBeUndefined() + } finally { + await new Promise((resolve) => server.close(() => resolve())) + } + }) +}) diff --git a/tests/cli/statement.test.ts b/tests/cli/statement.test.ts index 39bffc1..92a0074 100644 --- a/tests/cli/statement.test.ts +++ b/tests/cli/statement.test.ts @@ -4,6 +4,7 @@ import path from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { runStatementCommand } from '../../src/cli/statement.ts' import type { Country } from '../../src/config/define.ts' +import { SUPPORTED_REPORT_SCHEMA } from '../../src/statement/findings.ts' /** * Stands in for a country added to COUNTRIES before its template is written. @@ -179,7 +180,7 @@ describe('runStatementCommand --audit', () => { await writeFile( path.join(dir, 'clean.json'), JSON.stringify({ - schemaVersion: 1, + schemaVersion: SUPPORTED_REPORT_SCHEMA, generatedAt: '2026-08-21T09:30:00.000Z', summary: { pages: 1, needsReview: 0, notEvaluated: 0 }, rules: {}, diff --git a/tests/fixtures/statement/audit.json b/tests/fixtures/statement/audit.json index 0aea18c..b327dea 100644 --- a/tests/fixtures/statement/audit.json +++ b/tests/fixtures/statement/audit.json @@ -1,5 +1,5 @@ { - "schemaVersion": 1, + "schemaVersion": 2, "tool": { "name": "eaa-kit", "version": "0.1.0", diff --git a/tests/statement/findings.test.ts b/tests/statement/findings.test.ts index 9976741..0be8cd5 100644 --- a/tests/statement/findings.test.ts +++ b/tests/statement/findings.test.ts @@ -18,7 +18,7 @@ async function fixture(): Promise> { /** The smallest document the reader accepts, to build invalid ones from. */ function report(overrides: Record = {}): Record { return { - schemaVersion: 1, + schemaVersion: SUPPORTED_REPORT_SCHEMA, generatedAt: '2026-08-21T09:30:00.000Z', summary: { pages: 1, needsReview: 0, notEvaluated: 0 }, rules: { @@ -192,8 +192,12 @@ describe('summariseAuditReport', () => { }) it('rejects a schema version it does not know how to read', () => { - expect(() => summariseAuditReport(report({ schemaVersion: 2 }))).toThrow( - /schemaVersion 2; this version of eaa-kit reads 1/, + expect(() => + summariseAuditReport(report({ schemaVersion: SUPPORTED_REPORT_SCHEMA + 1 })), + ).toThrow( + new RegExp( + `schemaVersion ${SUPPORTED_REPORT_SCHEMA + 1}; this version of eaa-kit reads ${SUPPORTED_REPORT_SCHEMA}`, + ), ) })