From 9badaf2c897a74e32dc54ee0897a12c844a39bab Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 7 Jun 2026 08:07:52 -0700 Subject: [PATCH 1/3] fix: wait for SPA rendering before running axe scan The scanner ran axe immediately after page.goto, so single-page apps that render after load were scanned against a near-empty DOM and reported no violations. Wait for networkidle (30s cap) before scanning; if a page never reaches idle, warn and proceed so long-polling/websocket sites still scan. Closes #201 Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/actions/find/src/findForUrl.ts | 5 ++ .github/actions/find/tests/findForUrl.test.ts | 85 +++++++++++++++---- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index d9f1ea87..fc13b771 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -27,6 +27,11 @@ export async function findForUrl( const context = await browser.newContext(contextOptions) const page = await context.newPage() await page.goto(url) + try { + await page.waitForLoadState('networkidle', {timeout: 30000}) + } catch (e) { + core.warning(`Unable to wait for ${url} to reach network idle before scanning: ${e}`) + } const findings: Finding[] = [] const addFinding = async (findingData: Finding) => { diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts index 85299c5c..70861546 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -7,21 +7,43 @@ import * as pluginManager from '../src/pluginManager/index.js' import type {Plugin} from '../src/pluginManager/types.js' import {clearCache} from '../src/scansContextProvider.js' +const playwrightMocks = vi.hoisted(() => { + const pageGoto = vi.fn() + const pageWaitForLoadState = vi.fn() + const pageUrl = vi.fn() + const contextClose = vi.fn() + const browserClose = vi.fn() + const contextNewPage = vi.fn(() => ({ + goto: pageGoto, + waitForLoadState: pageWaitForLoadState, + url: pageUrl, + })) + const browserNewContext = vi.fn(() => ({ + newPage: contextNewPage, + close: contextClose, + })) + const browserLaunch = vi.fn(() => ({ + newContext: browserNewContext, + close: browserClose, + })) + + return { + browserLaunch, + browserNewContext, + contextNewPage, + pageGoto, + pageWaitForLoadState, + pageUrl, + contextClose, + browserClose, + } +}) + vi.mock('@actions/core', {spy: true}) vi.mock('playwright', () => ({ default: { chromium: { - launch: () => ({ - newContext: () => ({ - newPage: () => ({ - pageUrl: '', - goto: () => {}, - url: () => {}, - }), - close: () => {}, - }), - close: () => {}, - }), + launch: playwrightMocks.browserLaunch, }, }, })) @@ -39,6 +61,9 @@ let loadedPlugins: Plugin[] = [] function clearAll() { clearCache() vi.clearAllMocks() + playwrightMocks.pageGoto.mockResolvedValue(undefined) + playwrightMocks.pageWaitForLoadState.mockResolvedValue(undefined) + playwrightMocks.pageUrl.mockReturnValue('test.com') } describe('findForUrl', () => { @@ -49,12 +74,42 @@ describe('findForUrl', () => { async function axeOnlyTest() { clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(0) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(0) } + describe('page load handling', () => { + it('waits for network idle after navigation before scanning', async () => { + actionInput = '' + clearAll() + + await findForUrl({url: 'test.com'}) + + expect(playwrightMocks.pageGoto).toHaveBeenCalledWith('test.com') + expect(playwrightMocks.pageWaitForLoadState).toHaveBeenCalledWith('networkidle', {timeout: 30000}) + expect(playwrightMocks.pageGoto.mock.invocationCallOrder[0]).toBeLessThan( + playwrightMocks.pageWaitForLoadState.mock.invocationCallOrder[0], + ) + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + }) + + it('logs a warning and proceeds with scanning when network idle times out', async () => { + const timeoutError = new Error('Timeout 30000ms exceeded') + actionInput = '' + clearAll() + playwrightMocks.pageWaitForLoadState.mockRejectedValueOnce(timeoutError) + + await findForUrl({url: 'test.com'}) + + expect(core.warning).toHaveBeenCalledWith( + `Unable to wait for test.com to reach network idle before scanning: ${timeoutError}`, + ) + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + }) + }) + describe('when no scans list is provided', () => { it('defaults to running only axe scan', async () => { actionInput = '' @@ -80,7 +135,7 @@ describe('findForUrl', () => { actionInput = JSON.stringify(['axe', 'custom-scan-1']) clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(1) @@ -97,7 +152,7 @@ describe('findForUrl', () => { actionInput = JSON.stringify(['custom-scan-1', 'custom-scan-2']) clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(2) @@ -112,7 +167,7 @@ describe('findForUrl', () => { actionInput = JSON.stringify(['custom-scan-1']) clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(loadedPlugins[0].default).toHaveBeenCalledTimes(1) expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0) }) From 450fddb738c1c99350a1c0ff7748460802318406 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 4 Jul 2026 09:21:28 -0700 Subject: [PATCH 2/3] fix: use web-assertion readiness instead of networkidle before axe scan Per review: Playwright discourages networkidle and recommends web assertions for readiness. Replaces the networkidle wait with a locator assertion on rendered visible content under body, with a warn-and-proceed fallback so minimal static pages still scan. The timeout is exposed as a configurable action input. --- .github/actions/find/README.md | 4 ++ .github/actions/find/action.yml | 4 ++ .github/actions/find/src/findForUrl.ts | 17 ++++-- .github/actions/find/src/index.ts | 22 +++++++- .github/actions/find/tests/findForUrl.test.ts | 52 ++++++++++++++++--- README.md | 2 + action.yml | 5 ++ 7 files changed, 93 insertions(+), 13 deletions(-) diff --git a/.github/actions/find/README.md b/.github/actions/find/README.md index b8bca81c..ab147f64 100644 --- a/.github/actions/find/README.md +++ b/.github/actions/find/README.md @@ -31,6 +31,10 @@ configuration option. [`colorScheme`](https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme) configuration option. +#### `rendered_content_timeout` + +**Optional** Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues. Default: `30000`. + #### `include_screenshots` **Optional** Bool - whether to capture screenshots of scanned pages and include links to them in the issue diff --git a/.github/actions/find/action.yml b/.github/actions/find/action.yml index fb53d901..6bbdb156 100644 --- a/.github/actions/find/action.yml +++ b/.github/actions/find/action.yml @@ -25,6 +25,10 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false + rendered_content_timeout: + description: 'Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues.' + required: false + default: '30000' outputs: findings_file: diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index fc13b771..68cd0b30 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -7,12 +7,15 @@ import {loadPlugins, invokePlugin} from './pluginManager/index.js' import {getScansContext} from './scansContextProvider.js' import * as core from '@actions/core' +const DEFAULT_RENDERED_CONTENT_TIMEOUT = 30000 + export async function findForUrl( urlConfig: UrlConfig, authContext?: AuthContext, includeScreenshots: boolean = false, reducedMotion?: ReducedMotionPreference, colorScheme?: ColorSchemePreference, + renderedContentTimeout: number = DEFAULT_RENDERED_CONTENT_TIMEOUT, ): Promise { const {url, excludeSelectors} = urlConfig const browser = await playwright.chromium.launch({ @@ -27,11 +30,7 @@ export async function findForUrl( const context = await browser.newContext(contextOptions) const page = await context.newPage() await page.goto(url) - try { - await page.waitForLoadState('networkidle', {timeout: 30000}) - } catch (e) { - core.warning(`Unable to wait for ${url} to reach network idle before scanning: ${e}`) - } + await waitForRenderedContent({page, url, timeout: renderedContentTimeout}) const findings: Finding[] = [] const addFinding = async (findingData: Finding) => { @@ -72,6 +71,14 @@ export async function findForUrl( return findings } +async function waitForRenderedContent({page, url, timeout}: {page: playwright.Page; url: string; timeout: number}) { + try { + await page.locator('body *:visible').first().waitFor({state: 'visible', timeout}) + } catch (e) { + core.warning(`Unable to confirm rendered content for ${url} before scanning: ${e}`) + } +} + async function runAxeScan({ page, addFinding, diff --git a/.github/actions/find/src/index.ts b/.github/actions/find/src/index.ts index 675dff6f..bbf979fe 100644 --- a/.github/actions/find/src/index.ts +++ b/.github/actions/find/src/index.ts @@ -11,6 +11,7 @@ export default async function () { const urls = loadUrls({urlConfigs}) const reducedMotion = loadReducedMotion() const colorScheme = loadColorScheme() + const renderedContentTimeout = loadRenderedContentTimeout() const actualUrls = urlConfigs || urls || [] @@ -22,7 +23,14 @@ export default async function () { for (const urlConfig of actualUrls) { const {url} = urlConfig core.info(`Preparing to scan ${url}`) - const findingsForUrl = await findForUrl(urlConfig, authContext, includeScreenshots, reducedMotion, colorScheme) + const findingsForUrl = await findForUrl( + urlConfig, + authContext, + includeScreenshots, + reducedMotion, + colorScheme, + renderedContentTimeout, + ) if (findingsForUrl.length === 0) { core.info(`No accessibility gaps were found on ${url}`) continue @@ -101,3 +109,15 @@ function loadColorScheme() { return colorSchemeInput as ColorSchemePreference } + +function loadRenderedContentTimeout() { + const renderedContentTimeoutInput = core.getInput('rendered_content_timeout', {required: false}) + if (!renderedContentTimeoutInput) return + + const renderedContentTimeout = Number(renderedContentTimeoutInput) + if (!Number.isSafeInteger(renderedContentTimeout) || renderedContentTimeout <= 0) { + throw new Error("Input 'rendered_content_timeout' must be a positive integer number of milliseconds.") + } + + return renderedContentTimeout +} diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts index 70861546..b9798c4d 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -10,12 +10,20 @@ import {clearCache} from '../src/scansContextProvider.js' const playwrightMocks = vi.hoisted(() => { const pageGoto = vi.fn() const pageWaitForLoadState = vi.fn() + const locatorWaitFor = vi.fn() + const locatorFirst = vi.fn(() => ({ + waitFor: locatorWaitFor, + })) + const pageLocator = vi.fn(() => ({ + first: locatorFirst, + })) const pageUrl = vi.fn() const contextClose = vi.fn() const browserClose = vi.fn() const contextNewPage = vi.fn(() => ({ goto: pageGoto, waitForLoadState: pageWaitForLoadState, + locator: pageLocator, url: pageUrl, })) const browserNewContext = vi.fn(() => ({ @@ -33,6 +41,9 @@ const playwrightMocks = vi.hoisted(() => { contextNewPage, pageGoto, pageWaitForLoadState, + pageLocator, + locatorFirst, + locatorWaitFor, pageUrl, contextClose, browserClose, @@ -63,6 +74,7 @@ function clearAll() { vi.clearAllMocks() playwrightMocks.pageGoto.mockResolvedValue(undefined) playwrightMocks.pageWaitForLoadState.mockResolvedValue(undefined) + playwrightMocks.locatorWaitFor.mockResolvedValue(undefined) playwrightMocks.pageUrl.mockReturnValue('test.com') } @@ -81,33 +93,59 @@ describe('findForUrl', () => { } describe('page load handling', () => { - it('waits for network idle after navigation before scanning', async () => { + it('waits for late-rendered SPA content after navigation before scanning', async () => { actionInput = '' clearAll() + let resolveRenderedContent!: () => void + const renderedContent = new Promise(resolve => { + resolveRenderedContent = resolve + }) + playwrightMocks.locatorWaitFor.mockReturnValueOnce(renderedContent) - await findForUrl({url: 'test.com'}) + const scan = findForUrl({url: 'test.com'}) + await new Promise(resolve => setTimeout(resolve, 0)) expect(playwrightMocks.pageGoto).toHaveBeenCalledWith('test.com') - expect(playwrightMocks.pageWaitForLoadState).toHaveBeenCalledWith('networkidle', {timeout: 30000}) + expect(playwrightMocks.pageWaitForLoadState).not.toHaveBeenCalled() + expect(playwrightMocks.pageLocator).toHaveBeenCalledWith('body *:visible') + expect(playwrightMocks.locatorFirst).toHaveBeenCalledTimes(1) + expect(playwrightMocks.locatorWaitFor).toHaveBeenCalledWith({state: 'visible', timeout: 30000}) expect(playwrightMocks.pageGoto.mock.invocationCallOrder[0]).toBeLessThan( - playwrightMocks.pageWaitForLoadState.mock.invocationCallOrder[0], + playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0], + ) + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0) + + resolveRenderedContent() + await scan + + expect(playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0]).toBeLessThan( + playwrightMocks.pageUrl.mock.invocationCallOrder[0], ) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) }) - it('logs a warning and proceeds with scanning when network idle times out', async () => { + it('logs a warning and proceeds with scanning when minimal static pages do not render visible content', async () => { const timeoutError = new Error('Timeout 30000ms exceeded') actionInput = '' clearAll() - playwrightMocks.pageWaitForLoadState.mockRejectedValueOnce(timeoutError) + playwrightMocks.locatorWaitFor.mockRejectedValueOnce(timeoutError) await findForUrl({url: 'test.com'}) expect(core.warning).toHaveBeenCalledWith( - `Unable to wait for test.com to reach network idle before scanning: ${timeoutError}`, + `Unable to confirm rendered content for test.com before scanning: ${timeoutError}`, ) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) }) + + it('uses the configured rendered content timeout', async () => { + actionInput = '' + clearAll() + + await findForUrl({url: 'test.com'}, undefined, false, undefined, undefined, 1000) + + expect(playwrightMocks.locatorWaitFor).toHaveBeenCalledWith({state: 'visible', timeout: 1000}) + }) }) describe('when no scans list is provided', () => { diff --git a/README.md b/README.md index 88b68c11..43215681 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ jobs: # open_grouped_issues: false # Optional: Set to true to open an issue grouping individual issues per violation # reduced_motion: no-preference # Optional: Playwright reduced motion configuration option # color_scheme: light # Optional: Playwright color scheme configuration option + # rendered_content_timeout: 30000 # Optional: Milliseconds to wait for visible rendered content before scanning # scans: '["axe","reflow-scan"]' # Optional: An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. # url_configs: '[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]' # Optional: Per-URL config with CSS selectors to exclude from the Axe scan. When provided, takes precedence over 'urls'. ``` @@ -130,6 +131,7 @@ Trigger the workflow manually or automatically based on your configuration. The | `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | | `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | +| `rendered_content_timeout` | No | Milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues. Default: `30000` | `30000` | | `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | | `url_configs` | No | A stringified JSON array of URL config objects. Each object must have a `url` field and may have an optional `excludeSelectors` field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the `urls` input. | `'[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]'` | diff --git a/action.yml b/action.yml index b86a45e5..a48c44d6 100644 --- a/action.yml +++ b/action.yml @@ -51,6 +51,10 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false + rendered_content_timeout: + description: 'Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues.' + required: false + default: '30000' scans: description: 'Stringified JSON array of scans to perform. If not provided, only Axe will be performed' required: false @@ -117,6 +121,7 @@ runs: include_screenshots: ${{ inputs.include_screenshots }} reduced_motion: ${{ inputs.reduced_motion }} color_scheme: ${{ inputs.color_scheme }} + rendered_content_timeout: ${{ inputs.rendered_content_timeout }} scans: ${{ inputs.scans }} - name: File id: file From 59f8e50f0374a630ff11e79ee704c52e188e46f5 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 10 Jul 2026 11:26:22 -0700 Subject: [PATCH 3/3] fix: wait on per-url waitForSelectors instead of a rendered-content heuristic Replace the generic visible-content wait (and its rendered_content_timeout input) with an optional waitForSelectors field on url_configs entries, per review. Each listed selector must become visible before the scan runs. --- .github/actions/find/README.md | 8 +- .github/actions/find/action.yml | 7 +- .github/actions/find/src/findForUrl.ts | 19 ++--- .github/actions/find/src/index.ts | 29 ++------ .github/actions/find/src/types.d.ts | 1 + .github/actions/find/tests/findForUrl.test.ts | 74 ++++++++----------- .github/actions/find/tests/index.test.ts | 57 ++++++++++++++ README.md | 6 +- action.yml | 7 +- 9 files changed, 111 insertions(+), 97 deletions(-) create mode 100644 .github/actions/find/tests/index.test.ts diff --git a/.github/actions/find/README.md b/.github/actions/find/README.md index ab147f64..f2893a31 100644 --- a/.github/actions/find/README.md +++ b/.github/actions/find/README.md @@ -15,6 +15,10 @@ https://primer.style https://primer.style/octicons/ ``` +#### `url_configs` + +**Optional** Stringified JSON array of per-URL configuration objects. Each object must include a `url` and may include `excludeSelectors` (selectors to exclude from Axe) and `waitForSelectors` (selectors that must become visible within 30 seconds before scanning). When provided, this input takes precedence over `urls`. + #### `auth_context` **Optional** Stringified JSON object containing `username`, `password`, `cookies`, and/or `localStorage` from an authenticated session. For example: `{"username":"some-user","password":"correct-horse-battery-staple","cookies":[{"name":"theme-preference","value":"light","domain":"primer.style","path":"/"}],"localStorage":{"https://primer.style":{"theme-preference":"light"}}}` @@ -31,10 +35,6 @@ configuration option. [`colorScheme`](https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme) configuration option. -#### `rendered_content_timeout` - -**Optional** Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues. Default: `30000`. - #### `include_screenshots` **Optional** Bool - whether to capture screenshots of scanned pages and include links to them in the issue diff --git a/.github/actions/find/action.yml b/.github/actions/find/action.yml index 6bbdb156..32d331e8 100644 --- a/.github/actions/find/action.yml +++ b/.github/actions/find/action.yml @@ -7,7 +7,7 @@ inputs: required: false multiline: true url_configs: - description: "Stringified JSON array of URL config objects, each with a 'url' field and an optional 'excludeSelectors' field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the 'urls' input." + description: "Stringified JSON array of URL config objects, each with a 'url' field and optional 'excludeSelectors' (selectors to exclude from Axe) and 'waitForSelectors' (selectors that must be visible before scanning) fields. When provided, takes precedence over the 'urls' input." required: false auth_context: description: "Stringified JSON object containing 'username', 'password', 'cookies', and/or 'localStorage' from an authenticated session" @@ -25,11 +25,6 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false - rendered_content_timeout: - description: 'Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues.' - required: false - default: '30000' - outputs: findings_file: description: 'Path to a JSON file containing the list of potential accessibility gaps' diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index 68cd0b30..eefc577c 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -7,7 +7,7 @@ import {loadPlugins, invokePlugin} from './pluginManager/index.js' import {getScansContext} from './scansContextProvider.js' import * as core from '@actions/core' -const DEFAULT_RENDERED_CONTENT_TIMEOUT = 30000 +const SELECTOR_WAIT_TIMEOUT = 30000 export async function findForUrl( urlConfig: UrlConfig, @@ -15,9 +15,8 @@ export async function findForUrl( includeScreenshots: boolean = false, reducedMotion?: ReducedMotionPreference, colorScheme?: ColorSchemePreference, - renderedContentTimeout: number = DEFAULT_RENDERED_CONTENT_TIMEOUT, ): Promise { - const {url, excludeSelectors} = urlConfig + const {url, excludeSelectors, waitForSelectors} = urlConfig const browser = await playwright.chromium.launch({ headless: true, executablePath: process.env.CI ? '/usr/bin/google-chrome' : undefined, @@ -30,7 +29,11 @@ export async function findForUrl( const context = await browser.newContext(contextOptions) const page = await context.newPage() await page.goto(url) - await waitForRenderedContent({page, url, timeout: renderedContentTimeout}) + await Promise.all( + (waitForSelectors ?? []).map(selector => + page.locator(selector).waitFor({state: 'visible', timeout: SELECTOR_WAIT_TIMEOUT}), + ), + ) const findings: Finding[] = [] const addFinding = async (findingData: Finding) => { @@ -71,14 +74,6 @@ export async function findForUrl( return findings } -async function waitForRenderedContent({page, url, timeout}: {page: playwright.Page; url: string; timeout: number}) { - try { - await page.locator('body *:visible').first().waitFor({state: 'visible', timeout}) - } catch (e) { - core.warning(`Unable to confirm rendered content for ${url} before scanning: ${e}`) - } -} - async function runAxeScan({ page, addFinding, diff --git a/.github/actions/find/src/index.ts b/.github/actions/find/src/index.ts index bbf979fe..138307c0 100644 --- a/.github/actions/find/src/index.ts +++ b/.github/actions/find/src/index.ts @@ -11,7 +11,6 @@ export default async function () { const urls = loadUrls({urlConfigs}) const reducedMotion = loadReducedMotion() const colorScheme = loadColorScheme() - const renderedContentTimeout = loadRenderedContentTimeout() const actualUrls = urlConfigs || urls || [] @@ -23,14 +22,7 @@ export default async function () { for (const urlConfig of actualUrls) { const {url} = urlConfig core.info(`Preparing to scan ${url}`) - const findingsForUrl = await findForUrl( - urlConfig, - authContext, - includeScreenshots, - reducedMotion, - colorScheme, - renderedContentTimeout, - ) + const findingsForUrl = await findForUrl(urlConfig, authContext, includeScreenshots, reducedMotion, colorScheme) if (findingsForUrl.length === 0) { core.info(`No accessibility gaps were found on ${url}`) continue @@ -63,6 +55,13 @@ function loadUrlConfigs() { if (typeof item !== 'object' || item === null || typeof item.url !== 'string') { throw new Error("Each entry in 'url_configs' must be an object with a 'url' string field.") } + if ( + item.waitForSelectors !== undefined && + (!Array.isArray(item.waitForSelectors) || + item.waitForSelectors.some((selector: unknown) => typeof selector !== 'string')) + ) { + throw new Error("Each 'waitForSelectors' field in 'url_configs' must be an array of CSS selector strings.") + } } return parsed as UrlConfig[] @@ -109,15 +108,3 @@ function loadColorScheme() { return colorSchemeInput as ColorSchemePreference } - -function loadRenderedContentTimeout() { - const renderedContentTimeoutInput = core.getInput('rendered_content_timeout', {required: false}) - if (!renderedContentTimeoutInput) return - - const renderedContentTimeout = Number(renderedContentTimeoutInput) - if (!Number.isSafeInteger(renderedContentTimeout) || renderedContentTimeout <= 0) { - throw new Error("Input 'rendered_content_timeout' must be a positive integer number of milliseconds.") - } - - return renderedContentTimeout -} diff --git a/.github/actions/find/src/types.d.ts b/.github/actions/find/src/types.d.ts index dcbc8600..bb5335cd 100644 --- a/.github/actions/find/src/types.d.ts +++ b/.github/actions/find/src/types.d.ts @@ -41,4 +41,5 @@ export type ColorSchemePreference = 'light' | 'dark' | 'no-preference' | null export type UrlConfig = { url: string excludeSelectors?: string[] + waitForSelectors?: string[] } diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts index b9798c4d..3c1b66a5 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -9,20 +9,15 @@ import {clearCache} from '../src/scansContextProvider.js' const playwrightMocks = vi.hoisted(() => { const pageGoto = vi.fn() - const pageWaitForLoadState = vi.fn() const locatorWaitFor = vi.fn() - const locatorFirst = vi.fn(() => ({ - waitFor: locatorWaitFor, - })) const pageLocator = vi.fn(() => ({ - first: locatorFirst, + waitFor: locatorWaitFor, })) const pageUrl = vi.fn() const contextClose = vi.fn() const browserClose = vi.fn() const contextNewPage = vi.fn(() => ({ goto: pageGoto, - waitForLoadState: pageWaitForLoadState, locator: pageLocator, url: pageUrl, })) @@ -40,9 +35,7 @@ const playwrightMocks = vi.hoisted(() => { browserNewContext, contextNewPage, pageGoto, - pageWaitForLoadState, pageLocator, - locatorFirst, locatorWaitFor, pageUrl, contextClose, @@ -50,6 +43,11 @@ const playwrightMocks = vi.hoisted(() => { } }) +const pluginMocks = vi.hoisted(() => ({ + loadPlugins: vi.fn(), + invokePlugin: vi.fn(), +})) + vi.mock('@actions/core', {spy: true}) vi.mock('playwright', () => ({ default: { @@ -65,6 +63,7 @@ vi.mock('@axe-core/playwright', () => { AxeBuilderMock.prototype.analyze = vi.fn(() => Promise.resolve(rawFinding)) return {AxeBuilder: AxeBuilderMock} }) +vi.mock('../src/pluginManager/index.js', () => pluginMocks) let actionInput: string = '' let loadedPlugins: Plugin[] = [] @@ -73,15 +72,16 @@ function clearAll() { clearCache() vi.clearAllMocks() playwrightMocks.pageGoto.mockResolvedValue(undefined) - playwrightMocks.pageWaitForLoadState.mockResolvedValue(undefined) playwrightMocks.locatorWaitFor.mockResolvedValue(undefined) playwrightMocks.pageUrl.mockReturnValue('test.com') } describe('findForUrl', () => { vi.spyOn(core, 'getInput').mockImplementation(() => actionInput) - vi.spyOn(pluginManager, 'loadPlugins').mockImplementation(() => Promise.resolve(loadedPlugins)) - vi.spyOn(pluginManager, 'invokePlugin') + vi.mocked(pluginManager.loadPlugins).mockImplementation(() => Promise.resolve(loadedPlugins)) + vi.mocked(pluginManager.invokePlugin).mockImplementation(({plugin, page, addFinding}) => + plugin.default({page, addFinding}), + ) async function axeOnlyTest() { clearAll() @@ -93,58 +93,44 @@ describe('findForUrl', () => { } describe('page load handling', () => { - it('waits for late-rendered SPA content after navigation before scanning', async () => { + it('uses the default navigation readiness when no selectors are configured', async () => { actionInput = '' clearAll() - let resolveRenderedContent!: () => void - const renderedContent = new Promise(resolve => { - resolveRenderedContent = resolve - }) - playwrightMocks.locatorWaitFor.mockReturnValueOnce(renderedContent) - const scan = findForUrl({url: 'test.com'}) - await new Promise(resolve => setTimeout(resolve, 0)) + await findForUrl({url: 'test.com'}) expect(playwrightMocks.pageGoto).toHaveBeenCalledWith('test.com') - expect(playwrightMocks.pageWaitForLoadState).not.toHaveBeenCalled() - expect(playwrightMocks.pageLocator).toHaveBeenCalledWith('body *:visible') - expect(playwrightMocks.locatorFirst).toHaveBeenCalledTimes(1) - expect(playwrightMocks.locatorWaitFor).toHaveBeenCalledWith({state: 'visible', timeout: 30000}) - expect(playwrightMocks.pageGoto.mock.invocationCallOrder[0]).toBeLessThan( - playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0], - ) - expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0) - - resolveRenderedContent() - await scan - - expect(playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0]).toBeLessThan( - playwrightMocks.pageUrl.mock.invocationCallOrder[0], - ) + expect(playwrightMocks.pageLocator).not.toHaveBeenCalled() expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) }) - it('logs a warning and proceeds with scanning when minimal static pages do not render visible content', async () => { - const timeoutError = new Error('Timeout 30000ms exceeded') + it('waits for each configured selector after navigation and before scanning', async () => { actionInput = '' clearAll() - playwrightMocks.locatorWaitFor.mockRejectedValueOnce(timeoutError) - await findForUrl({url: 'test.com'}) + await findForUrl({url: 'test.com', waitForSelectors: ['#app', '[data-ready]']}) - expect(core.warning).toHaveBeenCalledWith( - `Unable to confirm rendered content for test.com before scanning: ${timeoutError}`, + expect(playwrightMocks.pageLocator).toHaveBeenNthCalledWith(1, '#app') + expect(playwrightMocks.pageLocator).toHaveBeenNthCalledWith(2, '[data-ready]') + expect(playwrightMocks.locatorWaitFor).toHaveBeenNthCalledWith(1, {state: 'visible', timeout: 30000}) + expect(playwrightMocks.locatorWaitFor).toHaveBeenNthCalledWith(2, {state: 'visible', timeout: 30000}) + expect(playwrightMocks.pageGoto.mock.invocationCallOrder[0]).toBeLessThan( + playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0], + ) + expect(playwrightMocks.locatorWaitFor.mock.invocationCallOrder[1]).toBeLessThan( + AxeBuilder.prototype.analyze.mock.invocationCallOrder[0], ) - expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) }) - it('uses the configured rendered content timeout', async () => { + it('does not scan when a configured selector times out', async () => { + const timeoutError = new Error('Timeout 30000ms exceeded') actionInput = '' clearAll() + playwrightMocks.locatorWaitFor.mockRejectedValueOnce(timeoutError) - await findForUrl({url: 'test.com'}, undefined, false, undefined, undefined, 1000) + await expect(findForUrl({url: 'test.com', waitForSelectors: ['#app']})).rejects.toThrow(timeoutError) - expect(playwrightMocks.locatorWaitFor).toHaveBeenCalledWith({state: 'visible', timeout: 1000}) + expect(AxeBuilder.prototype.analyze).not.toHaveBeenCalled() }) }) diff --git a/.github/actions/find/tests/index.test.ts b/.github/actions/find/tests/index.test.ts new file mode 100644 index 00000000..f5d760fc --- /dev/null +++ b/.github/actions/find/tests/index.test.ts @@ -0,0 +1,57 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest' +import find from '../src/index.js' + +const mocks = vi.hoisted(() => ({ + inputs: {} as Record, + findForUrl: vi.fn(), + writeFileSync: vi.fn(), +})) + +vi.mock('@actions/core', () => ({ + getInput: vi.fn((name: string) => mocks.inputs[name] ?? ''), + getMultilineInput: vi.fn(() => []), + debug: vi.fn(), + info: vi.fn(), + setOutput: vi.fn(), +})) + +vi.mock('node:fs', () => ({ + default: { + writeFileSync: mocks.writeFileSync, + }, +})) + +vi.mock('../src/findForUrl.js', () => ({ + findForUrl: mocks.findForUrl, +})) + +describe('url_configs', () => { + beforeEach(() => { + vi.clearAllMocks() + for (const name of Object.keys(mocks.inputs)) delete mocks.inputs[name] + mocks.findForUrl.mockResolvedValue([]) + }) + + it('passes waitForSelectors through to the URL scan', async () => { + const urlConfig = { + url: 'https://example.com', + excludeSelectors: ['iframe'], + waitForSelectors: ['#app', '[data-ready]'], + } + mocks.inputs.url_configs = JSON.stringify([urlConfig]) + mocks.inputs.include_screenshots = 'false' + + await find() + + expect(mocks.findForUrl).toHaveBeenCalledWith(urlConfig, expect.anything(), false, undefined, undefined) + }) + + it('rejects waitForSelectors values that are not arrays of strings', async () => { + mocks.inputs.url_configs = JSON.stringify([{url: 'https://example.com', waitForSelectors: ['#app', 42]}]) + + await expect(find()).rejects.toThrow( + "Invalid 'url_configs' input: Each 'waitForSelectors' field in 'url_configs' must be an array of CSS selector strings.", + ) + expect(mocks.findForUrl).not.toHaveBeenCalled() + }) +}) diff --git a/README.md b/README.md index 43215681..7b9e457a 100644 --- a/README.md +++ b/README.md @@ -57,9 +57,8 @@ jobs: # open_grouped_issues: false # Optional: Set to true to open an issue grouping individual issues per violation # reduced_motion: no-preference # Optional: Playwright reduced motion configuration option # color_scheme: light # Optional: Playwright color scheme configuration option - # rendered_content_timeout: 30000 # Optional: Milliseconds to wait for visible rendered content before scanning # scans: '["axe","reflow-scan"]' # Optional: An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. - # url_configs: '[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]' # Optional: Per-URL config with CSS selectors to exclude from the Axe scan. When provided, takes precedence over 'urls'. + # url_configs: '[{"url":"https://example.com","excludeSelectors":["iframe","#widget"],"waitForSelectors":["#app","[data-ready]"]}]' # Optional: Per-URL selectors to exclude from Axe or wait for before scanning. When provided, takes precedence over 'urls'. ``` > 👉 Update all `REPLACE_THIS` placeholders with your actual values. See [Action Inputs](#action-inputs) for details. @@ -131,9 +130,8 @@ Trigger the workflow manually or automatically based on your configuration. The | `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | | `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | -| `rendered_content_timeout` | No | Milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues. Default: `30000` | `30000` | | `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | -| `url_configs` | No | A stringified JSON array of URL config objects. Each object must have a `url` field and may have an optional `excludeSelectors` field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the `urls` input. | `'[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]'` | +| `url_configs` | No | A stringified JSON array of URL config objects. Each object must have a `url` field and may have `excludeSelectors` (selectors to exclude from Axe) and `waitForSelectors` (selectors that must become visible within 30 seconds before scanning). When provided, takes precedence over `urls`. | `'[{"url":"https://example.com","waitForSelectors":["#app"]}]'` | --- diff --git a/action.yml b/action.yml index a48c44d6..197f8c6f 100644 --- a/action.yml +++ b/action.yml @@ -7,7 +7,7 @@ inputs: required: false multiline: true url_configs: - description: "Stringified JSON array of URL config objects, each with a 'url' field and an optional 'excludeSelectors' field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the 'urls' input." + description: "Stringified JSON array of URL config objects, each with a 'url' field and optional 'excludeSelectors' (selectors to exclude from Axe) and 'waitForSelectors' (selectors that must be visible before scanning) fields. When provided, takes precedence over the 'urls' input." required: false repository: description: 'Repository (with owner) to file issues in' @@ -51,10 +51,6 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false - rendered_content_timeout: - description: 'Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues.' - required: false - default: '30000' scans: description: 'Stringified JSON array of scans to perform. If not provided, only Axe will be performed' required: false @@ -121,7 +117,6 @@ runs: include_screenshots: ${{ inputs.include_screenshots }} reduced_motion: ${{ inputs.reduced_motion }} color_scheme: ${{ inputs.color_scheme }} - rendered_content_timeout: ${{ inputs.rendered_content_timeout }} scans: ${{ inputs.scans }} - name: File id: file