diff --git a/.github/actions/find/README.md b/.github/actions/find/README.md index a950a2d..0a0be1f 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"}}}` diff --git a/.github/actions/find/action.yml b/.github/actions/find/action.yml index 0d3907e..6662d86 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,7 +25,6 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false - 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 32296d7..2d5046e 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -8,6 +8,8 @@ import {loadPlugins, invokePlugin} from './pluginManager/index.js' import {getScansContext} from './scansContextProvider.js' import * as core from '@actions/core' +const SELECTOR_WAIT_TIMEOUT = 30000 + export async function findForUrl( urlConfig: UrlConfig, authContext?: AuthContext, @@ -15,7 +17,7 @@ export async function findForUrl( reducedMotion?: ReducedMotionPreference, colorScheme?: ColorSchemePreference, ): 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, @@ -28,6 +30,11 @@ export async function findForUrl( const context = await browser.newContext(contextOptions) const page = await context.newPage() await page.goto(url) + await Promise.all( + (waitForSelectors ?? []).map(selector => + page.locator(selector).waitFor({state: 'visible', timeout: SELECTOR_WAIT_TIMEOUT}), + ), + ) const findings: Finding[] = [] const addFinding = async (findingData: Finding) => { diff --git a/.github/actions/find/src/index.ts b/.github/actions/find/src/index.ts index 675dff6..138307c 100644 --- a/.github/actions/find/src/index.ts +++ b/.github/actions/find/src/index.ts @@ -55,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[] diff --git a/.github/actions/find/src/types.d.ts b/.github/actions/find/src/types.d.ts index 4102c32..678474d 100644 --- a/.github/actions/find/src/types.d.ts +++ b/.github/actions/find/src/types.d.ts @@ -50,4 +50,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 23ee53d..af73ed5 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -8,21 +8,52 @@ 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 locatorWaitFor = vi.fn() + const pageLocator = vi.fn(() => ({ + waitFor: locatorWaitFor, + })) + const pageUrl = vi.fn() + const contextClose = vi.fn() + const browserClose = vi.fn() + const contextNewPage = vi.fn(() => ({ + goto: pageGoto, + locator: pageLocator, + url: pageUrl, + })) + const browserNewContext = vi.fn(() => ({ + newPage: contextNewPage, + close: contextClose, + })) + const browserLaunch = vi.fn(() => ({ + newContext: browserNewContext, + close: browserClose, + })) + + return { + browserLaunch, + browserNewContext, + contextNewPage, + pageGoto, + pageLocator, + locatorWaitFor, + pageUrl, + contextClose, + browserClose, + } +}) + +const pluginMocks = vi.hoisted(() => ({ + loadPlugins: vi.fn(), + invokePlugin: vi.fn(), +})) + vi.mock('@actions/core', {spy: true}) vi.mock('playwright', () => ({ default: { chromium: { - launch: () => ({ - newContext: () => ({ - newPage: () => ({ - pageUrl: '', - goto: () => {}, - url: () => {}, - }), - close: () => {}, - }), - close: () => {}, - }), + launch: playwrightMocks.browserLaunch, }, }, })) @@ -33,6 +64,7 @@ vi.mock('@axe-core/playwright', () => { AxeBuilderMock.prototype.analyze = vi.fn(() => Promise.resolve(rawFinding)) return {AxeBuilder: AxeBuilderMock} }) +vi.mock('../src/pluginManager/index.js', () => pluginMocks) vi.mock('@accesslint/playwright', () => ({ accesslintAudit: vi.fn(() => Promise.resolve({violations: []})), @@ -44,23 +76,70 @@ let loadedPlugins: Plugin[] = [] function clearAll() { clearCache() vi.clearAllMocks() + playwrightMocks.pageGoto.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() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) expect(accesslintAudit).toHaveBeenCalledTimes(0) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(0) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(0) } + describe('page load handling', () => { + it('uses the default navigation readiness when no selectors are configured', async () => { + actionInput = '' + clearAll() + + await findForUrl({url: 'test.com'}) + + expect(playwrightMocks.pageGoto).toHaveBeenCalledWith('test.com') + expect(playwrightMocks.pageLocator).not.toHaveBeenCalled() + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + }) + + it('waits for each configured selector after navigation and before scanning', async () => { + actionInput = '' + clearAll() + + await findForUrl({url: 'test.com', waitForSelectors: ['#app', '[data-ready]']}) + + 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], + ) + }) + + 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 expect(findForUrl({url: 'test.com', waitForSelectors: ['#app']})).rejects.toThrow(timeoutError) + + expect(AxeBuilder.prototype.analyze).not.toHaveBeenCalled() + }) + }) + describe('when no scans list is provided', () => { it('defaults to running only axe scan', async () => { actionInput = '' @@ -86,7 +165,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) @@ -103,7 +182,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) @@ -156,7 +235,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) }) diff --git a/.github/actions/find/tests/index.test.ts b/.github/actions/find/tests/index.test.ts new file mode 100644 index 0000000..f5d760f --- /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 718a223..4ddfee7 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ jobs: # reduced_motion: no-preference # Optional: Playwright reduced motion configuration option # color_scheme: light # Optional: Playwright color scheme configuration option # scans: '["axe","accesslint","reflow-scan"]' # Optional: An array of scans (or plugins) to be performed. Built-in engines are 'axe' and 'accesslint'; any other entry is a plugin name. 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 config with CSS 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. @@ -139,7 +139,7 @@ Trigger the workflow manually or automatically based on your configuration. The | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | | `scans` | No | An array of scans (or plugins) to be performed. Built-in engines are `axe` and `accesslint`; any other entry is treated as a plugin name. If not provided, only Axe will be performed. | `'["axe", "accesslint", ...other plugins]'` | | `dry_run` | No | When `true`, scan and log the issues that _would_ be filed without opening, closing, reopening, or assigning any issues — and without writing to the `gh-cache` branch. Useful for safely previewing results. Default: `false` | `true` | -| `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 the `urls` input. | `'[{"url":"https://example.com","excludeSelectors":["iframe","#widget"],"waitForSelectors":["#app"]}]'` | --- diff --git a/action.yml b/action.yml index 7de2f5d..56b4160 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'