diff --git a/.github/workflows/test-e2e-reusable.yml b/.github/workflows/test-e2e-reusable.yml index 904e8aff1c8..b2f17bcfb14 100644 --- a/.github/workflows/test-e2e-reusable.yml +++ b/.github/workflows/test-e2e-reusable.yml @@ -178,6 +178,9 @@ jobs: N8N_SANDBOX_SERVICE_URL: ${{ secrets.N8N_SANDBOX_SERVICE_URL }} N8N_SANDBOX_SERVICE_API_KEY: ${{ secrets.N8N_SANDBOX_SERVICE_API_KEY }} N8N_TEST_ENV: ${{ inputs.n8n-env }} + # Opt in to the a11y reporter here rather than in playwright.config.ts, + # so a local run stays quiet unless it is asked for. + PLAYWRIGHT_A11Y_REPORT: '1' - name: Upload Shard Artifacts if: always() @@ -195,6 +198,19 @@ jobs: retention-days: 1 if-no-files-found: ignore + # Only on a red run: the per-test axe scans are already attached to their + # test in Currents, and a green run still gets the violation table in the + # job summary, so uploading the full report every time would be a third + # copy of the same data per shard. + - name: Upload Accessibility Report + if: failure() + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: ${{ inputs.artifact-prefix }}-a11y-report-shard-${{ matrix.shard }} + path: packages/testing/playwright/a11y-report/ + retention-days: 7 + if-no-files-found: ignore + - name: Cancel Currents run if workflow is cancelled if: ${{ cancelled() }} env: diff --git a/.gitignore b/.gitignore index 22c2b0ede4c..9ad13b95f43 100644 --- a/.gitignore +++ b/.gitignore @@ -46,6 +46,7 @@ test-results.json *.0x packages/testing/playwright/playwright-report packages/testing/playwright/eval-report +packages/testing/playwright/a11y-report packages/testing/playwright/test-results packages/testing/playwright/results.xml packages/testing/playwright/eval-results.json diff --git a/packages/testing/playwright/AGENTS.md b/packages/testing/playwright/AGENTS.md index 18bf68c17ad..5c7f625b2ad 100644 --- a/packages/testing/playwright/AGENTS.md +++ b/packages/testing/playwright/AGENTS.md @@ -197,6 +197,37 @@ Buckets: `page` (whole document), `canvas`, `ndv`, `node-creator`, `sidebar`, `modal`. Defined in `fixtures/a11y.ts` (`A11Y_BUCKETS`). Scans run with WCAG 2.1 A + AA rules; override per call with `a11y.check('modal', { tags, disableRules })`. +When a journey acts as another user, `n8n.start.withUser()` returns a new +`n8nPage`. Point the checker at it with `a11y.for(otherN8n)` - the derived +checker reports into the same scan list. + +### Report and failure budget + +Every scan is attached to its test, so the raw axe results are always available +on that test (including in Currents). Set `PLAYWRIGHT_A11Y_REPORT` to also +register `reporters/a11y-reporter.ts`, which aggregates a run's scans into one +axe HTML report at `a11y-report/index.html` plus a table in the GitHub job +summary. Under sharding each shard reports the specs it ran. CI sets the +variable for the e2e workflow, which uploads the report as its own artifact on a +failing run; locally the reporter stays off unless asked for: + +```bash +PLAYWRIGHT_A11Y_REPORT=1 pnpm --filter=n8n-playwright test:local +``` + +Violations are **reporting-only by default**. Set +`PLAYWRIGHT_A11Y_MAX_VIOLATIONS` to the number of violations a single test may +report before it fails: + +```bash +# Fail any test reporting more than 5 violations +PLAYWRIGHT_A11Y_MAX_VIOLATIONS=5 pnpm --filter=n8n-playwright test:local +``` + +Unset (the default), empty or malformed all mean "no budget", so the violations +that already exist can't turn CI red. The budget is not applied to a test that +already failed for another reason. + ## Test Isolation Tests run in parallel. Design tests to be fully isolated so they don't interfere with each other. diff --git a/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts b/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts index e28bc3a87c4..5a50cc49070 100644 --- a/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts +++ b/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts @@ -3,6 +3,7 @@ import { expect } from '@playwright/test'; import type { IWorkflowBase } from 'n8n-workflow'; import { nanoid } from 'nanoid'; +import type { A11yChecker } from '../../fixtures/a11y'; import type { n8nPage } from '../../pages/n8nPage'; import type { ApiHelpers } from '../../services/api-helper'; import type { TestUser } from '../../services/user-api-helper'; @@ -99,11 +100,26 @@ export async function viewExecutionsListAsAdmin( }); } +/** + * The journey as a test drives it: seed, sign in as the project admin, open the + * executions list. + * + * Pass `a11y` to scan the screens the journey lands on. The scans happen after the + * list has rendered - scanning mid-navigation reports the loading state, not the + * screen the admin ends up looking at. They never fail the journey on their own; + * `PLAYWRIGHT_A11Y_MAX_VIOLATIONS` decides that, and is unset by default. + */ export async function adminViewsExecutionsList(deps: { n8n: n8nPage; api: ApiHelpers; + a11y?: A11yChecker; }): Promise { const ctx = await setupAdminViewsExecutionsList(deps.api); const adminN8n = await deps.n8n.start.withUser(ctx.admin); await viewExecutionsListAsAdmin(adminN8n, ctx); + + // `withUser` opens its own page, so point the checker at the admin's. + const a11y = deps.a11y?.for(adminN8n); + await a11y?.check('page'); + await a11y?.check('sidebar'); } diff --git a/packages/testing/playwright/fixtures/a11y.test.ts b/packages/testing/playwright/fixtures/a11y.test.ts index e59cd878245..b057646957d 100644 --- a/packages/testing/playwright/fixtures/a11y.test.ts +++ b/packages/testing/playwright/fixtures/a11y.test.ts @@ -3,13 +3,20 @@ import { describe, expect, test, vi } from 'vitest'; import { A11Y_BUCKETS, + A11Y_MAX_VIOLATIONS_ENV, A11yChecker, DEFAULT_A11Y_TAGS, + a11yBudgetError, + countA11yViolations, + getA11yViolationBudget, type A11yAnalyzer, type A11yViolation, } from './a11y'; +import type { n8nPage } from '../pages/n8nPage'; -const page = {} as Page; +const PAGE_URL = 'http://localhost:5678/home/workflows'; + +const page = { url: () => PAGE_URL } as Page; function violation(id: string): A11yViolation { return { id, impact: 'serious', tags: [], description: '', help: '', helpUrl: '', nodes: [] }; @@ -70,4 +77,93 @@ describe('A11yChecker', () => { expect(warn).toHaveBeenCalledWith(expect.stringContaining('canvas')); warn.mockRestore(); }); + + test('records each scan with its bucket and url', async () => { + const analyze = vi.fn().mockResolvedValue({ violations: [violation('label')] }); + const checker = new A11yChecker(page, analyze); + + await checker.check('canvas'); + await checker.check('sidebar'); + + expect(checker.scans).toEqual([ + { bucket: 'canvas', url: PAGE_URL, violations: [violation('label')] }, + { bucket: 'sidebar', url: PAGE_URL, violations: [violation('label')] }, + ]); + }); + + test('does not record a scan that could not run', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + const analyze = vi.fn().mockRejectedValue(new Error('axe failed')); + const checker = new A11yChecker(page, analyze); + + await checker.check('canvas'); + + expect(checker.scans).toEqual([]); + vi.restoreAllMocks(); + }); + + test('scans another user page while reporting into the same scan list', async () => { + const otherUrl = 'http://localhost:5678/projects/1/executions'; + const otherPage = { url: () => otherUrl } as Page; + const analyze = vi.fn().mockResolvedValue({ violations: [] }); + const checker = new A11yChecker(page, analyze); + + await checker.for({ page: otherPage } as n8nPage).check('page'); + + expect(analyze).toHaveBeenCalledWith(expect.objectContaining({ page: otherPage })); + expect(checker.scans).toEqual([{ bucket: 'page', url: otherUrl, violations: [] }]); + }); +}); + +describe('getA11yViolationBudget', () => { + test('is undefined when the env var is unset, so checks stay non-blocking', () => { + expect(getA11yViolationBudget({})).toBeUndefined(); + expect(getA11yViolationBudget({ [A11Y_MAX_VIOLATIONS_ENV]: ' ' })).toBeUndefined(); + }); + + test('reads a non-negative integer budget', () => { + expect(getA11yViolationBudget({ [A11Y_MAX_VIOLATIONS_ENV]: '0' })).toBe(0); + expect(getA11yViolationBudget({ [A11Y_MAX_VIOLATIONS_ENV]: '12' })).toBe(12); + }); + + test('falls back to non-blocking on a malformed value', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + expect(getA11yViolationBudget({ [A11Y_MAX_VIOLATIONS_ENV]: 'strict' })).toBeUndefined(); + expect(getA11yViolationBudget({ [A11Y_MAX_VIOLATIONS_ENV]: '-1' })).toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(2); + + warn.mockRestore(); + }); +}); + +describe('a11yBudgetError', () => { + const scans = [ + { + bucket: 'page' as const, + url: PAGE_URL, + violations: [violation('label'), violation('region')], + }, + { bucket: 'sidebar' as const, url: PAGE_URL, violations: [] }, + ]; + + test('counts violations across every scan', () => { + expect(countA11yViolations(scans)).toBe(2); + }); + + test('is undefined without a budget, however many violations there are', () => { + expect(a11yBudgetError(scans, undefined)).toBeUndefined(); + }); + + test('is undefined while the violations stay within budget', () => { + expect(a11yBudgetError(scans, 2)).toBeUndefined(); + }); + + test('names the offending rules once the budget is exceeded', () => { + const error = a11yBudgetError(scans, 1); + + expect(error?.message).toContain('2 accessibility violations'); + expect(error?.message).toContain('allows 1'); + expect(error?.message).toContain('page: label, region'); + }); }); diff --git a/packages/testing/playwright/fixtures/a11y.ts b/packages/testing/playwright/fixtures/a11y.ts index 534c9137a52..8f037a97dc3 100644 --- a/packages/testing/playwright/fixtures/a11y.ts +++ b/packages/testing/playwright/fixtures/a11y.ts @@ -1,5 +1,5 @@ import { AxeBuilder } from '@axe-core/playwright'; -import type { Fixtures, Page } from '@playwright/test'; +import type { Fixtures, Page, TestInfo } from '@playwright/test'; import type { Result, TagValue } from 'axe-core'; import type { n8nPage } from '../pages/n8nPage'; @@ -36,6 +36,24 @@ export type A11yCheckOptions = { disableRules?: string[]; }; +/** One completed scan, kept so the run can be reported on after the fact. */ +export type A11yScan = { + bucket: A11yBucket; + /** Where the scan happened, so a report can point back at the screen. */ + url: string; + violations: A11yViolation[]; +}; + +/** Attachment the a11y reporter reads a test's scans back out of. */ +export const A11Y_ATTACHMENT_NAME = 'a11y-scans'; + +/** + * Turns violations into failures once a run reports more than this many in a + * single test. Unset - the default - keeps the checks reporting-only, so the + * violations that already exist can't turn CI red. + */ +export const A11Y_MAX_VIOLATIONS_ENV = 'PLAYWRIGHT_A11Y_MAX_VIOLATIONS'; + type AnalyzeParams = { page: Page; include?: string; @@ -65,8 +83,19 @@ export class A11yChecker { constructor( private readonly page: Page, private readonly analyze: A11yAnalyzer = analyzeWithAxe, + /** Shared with every checker {@link for} derives, so one test reports one list. */ + readonly scans: A11yScan[] = [], ) {} + /** + * Rebinds the checker to another page while keeping the same scan list. + * Journeys that act as a second user get a fresh `n8nPage` from + * `n8n.start.withUser()`, and that page - not the fixture's - is the one to scan. + */ + for(target: n8nPage): A11yChecker { + return new A11yChecker(target.page, this.analyze, this.scans); + } + async check(bucket: A11yBucket, options: A11yCheckOptions = {}): Promise { const include = A11Y_BUCKETS[bucket]; @@ -77,6 +106,7 @@ export class A11yChecker { tags: options.tags ?? DEFAULT_A11Y_TAGS, disableRules: options.disableRules ?? [], }); + this.scans.push({ bucket, url: this.page.url(), violations }); return violations; } catch (error) { const reason = error instanceof Error ? error.message : String(error); @@ -86,6 +116,46 @@ export class A11yChecker { } } +/** + * How many violations a single test may report before it fails. Unset, empty or + * malformed all mean "no budget", which is what keeps the default non-blocking. + */ +export function getA11yViolationBudget(env: NodeJS.ProcessEnv = process.env): number | undefined { + const raw = env[A11Y_MAX_VIOLATIONS_ENV]?.trim(); + if (!raw) return undefined; + + const budget = Number(raw); + if (!Number.isInteger(budget) || budget < 0) { + console.warn( + `[a11y] Ignoring ${A11Y_MAX_VIOLATIONS_ENV}="${raw}", expected a non-negative integer. Checks stay non-blocking.`, + ); + return undefined; + } + return budget; +} + +export function countA11yViolations(scans: A11yScan[]): number { + return scans.reduce((total, scan) => total + scan.violations.length, 0); +} + +/** The error a test fails with when it blows its budget, or `undefined` if it didn't. */ +export function a11yBudgetError(scans: A11yScan[], budget: number | undefined): Error | undefined { + if (budget === undefined) return undefined; + + const total = countA11yViolations(scans); + if (total <= budget) return undefined; + + const perBucket = scans + .filter((scan) => scan.violations.length > 0) + .map((scan) => ` ${scan.bucket}: ${scan.violations.map((v) => v.id).join(', ')}`) + .join('\n'); + + return new Error( + `${total} accessibility violations, ${A11Y_MAX_VIOLATIONS_ENV} allows ${budget}.\n${perBucket}\n` + + 'See the a11y-report artifact for the full report.', + ); +} + export type A11yTestFixtures = { a11y: A11yChecker; }; @@ -94,11 +164,31 @@ type A11yFixtureDeps = { n8n: n8nPage; }; +/** + * Hands the scans to the reporter and applies the budget. Enforcement is skipped + * for a test that already failed - a budget error there would bury the real one. + */ +async function reportA11yScans(scans: A11yScan[], testInfo: TestInfo): Promise { + if (scans.length === 0) return; + + await testInfo.attach(A11Y_ATTACHMENT_NAME, { + body: JSON.stringify(scans), + contentType: 'application/json', + }); + + if (testInfo.errors.length > 0) return; + + const error = a11yBudgetError(scans, getA11yViolationBudget()); + if (error) throw error; +} + /** * Accessibility fixture. Spread into `test.extend()` to expose `a11y.check(bucket)`. */ export const a11yFixtures: Fixtures = { - a11y: async ({ n8n }, use) => { - await use(new A11yChecker(n8n.page)); + a11y: async ({ n8n }, use, testInfo) => { + const checker = new A11yChecker(n8n.page); + await use(checker); + await reportA11yScans(checker.scans, testInfo); }, }; diff --git a/packages/testing/playwright/playwright.config.ts b/packages/testing/playwright/playwright.config.ts index e8bdbc28b60..451fb18a06e 100644 --- a/packages/testing/playwright/playwright.config.ts +++ b/packages/testing/playwright/playwright.config.ts @@ -143,12 +143,18 @@ export default defineConfig({ ...(process.env.CURRENTS_RECORD_KEY ? [currentsReporter(currentsConfig)] : []), ['./reporters/metrics-reporter.ts'], ['./reporters/benchmark-summary-reporter.ts'], + ...(process.env.PLAYWRIGHT_A11Y_REPORT + ? ([['./reporters/a11y-reporter.ts']] as const) + : []), ...(process.env.LANGSMITH_API_KEY ? ([['./reporters/langsmith-eval.ts']] as const) : []), ] : [ ['html'], ['./reporters/metrics-reporter.ts'], ['./reporters/benchmark-summary-reporter.ts'], + ...(process.env.PLAYWRIGHT_A11Y_REPORT + ? ([['./reporters/a11y-reporter.ts']] as const) + : []), ['list'], ...(process.env.LANGSMITH_API_KEY ? ([['./reporters/langsmith-eval.ts']] as const) : []), ], diff --git a/packages/testing/playwright/reporters/a11y-reporter.test.ts b/packages/testing/playwright/reporters/a11y-reporter.test.ts new file mode 100644 index 00000000000..c4fd515b7db --- /dev/null +++ b/packages/testing/playwright/reporters/a11y-reporter.test.ts @@ -0,0 +1,139 @@ +import type { TestCase, TestResult } from '@playwright/test/reporter'; +import { describe, expect, test } from 'vitest'; + +import { A11yReporter, mergeA11yScans, parseA11yAttachment } from './a11y-reporter'; +import { A11Y_ATTACHMENT_NAME, type A11yScan, type A11yViolation } from '../fixtures/a11y'; + +type NodeResult = A11yViolation['nodes'][number]; + +// Only the fields the merge keys on; the check arrays axe fills in are noise here. +function node(target: string, html = `<${target} />`): NodeResult { + return { target: [target], html } as NodeResult; +} + +function violation( + id: string, + nodes: NodeResult[], + impact: A11yViolation['impact'] = 'serious', +): A11yViolation { + return { id, impact, tags: [], description: '', help: '', helpUrl: '', nodes }; +} + +function scan(bucket: A11yScan['bucket'], violations: A11yViolation[]): A11yScan { + return { bucket, url: 'http://localhost:5678/home/workflows', violations }; +} + +describe('mergeA11yScans', () => { + test('merges the nodes a rule tripped on across scans', () => { + const merged = mergeA11yScans([ + scan('page', [violation('label', [node('input')])]), + scan('sidebar', [violation('label', [node('select')])]), + ]); + + expect(merged).toHaveLength(1); + expect(merged[0].nodes.map((n) => n.html)).toEqual(['', '