Skip to content

Commit 7866232

Browse files
authored
v2.12.0 testing (#175)
Release testing & prep for v2.12.0.
2 parents 08a31b1 + 5bd8d26 commit 7866232

File tree

19 files changed

+155
-74
lines changed

19 files changed

+155
-74
lines changed

.github/actions/file/src/generateIssueBody.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,20 @@ export function generateIssueBody(finding: Finding, screenshotRepo: string): str
1919
}
2020

2121
const acceptanceCriteria = `## Acceptance Criteria
22-
- [ ] The specific axe violation reported in this issue is no longer reproducible.
23-
- [ ] The fix MUST meet WCAG 2.1 guidelines OR the accessibility standards specified by the repository or organization.
24-
- [ ] A test SHOULD be added to ensure this specific axe violation does not regress.
25-
- [ ] This PR MUST NOT introduce any new accessibility issues or regressions.
26-
`
22+
- [ ] The specific violation reported in this issue is no longer reproducible.
23+
- [ ] The fix MUST meet WCAG 2.1 guidelines OR the accessibility standards specified by the repository or organization.
24+
- [ ] A test SHOULD be added to ensure this specific violation does not regress.
25+
- [ ] This PR MUST NOT introduce any new accessibility issues or regressions.`
2726

2827
const body = `## What
29-
An accessibility scan flagged the element \`${finding.html}\` on ${finding.url} because ${finding.problemShort}. Learn more about why this was flagged by visiting ${finding.problemUrl}.
28+
An accessibility scan ${finding.html ? `flagged the element \`${finding.html}\`` : `found an issue on ${finding.url}`} because ${finding.problemShort}. Learn more about why this was flagged by visiting ${finding.problemUrl}.
3029
31-
${screenshotSection ?? ''}
32-
To fix this, ${finding.solutionShort}.
33-
${solutionLong ? `\nSpecifically:\n\n${solutionLong}` : ''}
30+
${screenshotSection ?? ''}
31+
To fix this, ${finding.solutionShort}.
32+
${solutionLong ? `\nSpecifically:\n\n${solutionLong}` : ''}
3433
35-
${acceptanceCriteria}
36-
`
34+
${acceptanceCriteria}
35+
`
3736

3837
return body
3938
}

.github/actions/file/src/openIssue.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ export async function openIssue(octokit: Octokit, repoWithOwner: string, finding
2121
const owner = repoWithOwner.split('/')[0]
2222
const repo = repoWithOwner.split('/')[1]
2323

24-
const labels = [`${finding.scannerType} rule: ${finding.ruleId}`, `${finding.scannerType}-scanning-issue`]
24+
const labels = [`${finding.scannerType}-scanning-issue`]
25+
// Only include a ruleId label when it's defined
26+
if (finding.ruleId) {
27+
labels.push(`${finding.scannerType} rule: ${finding.ruleId}`)
28+
}
29+
2530
const title = truncateWithEllipsis(
2631
`Accessibility issue: ${finding.problemShort[0].toUpperCase() + finding.problemShort.slice(1)} on ${new URL(finding.url).pathname}`,
2732
GITHUB_ISSUE_TITLE_MAX_LENGTH,

.github/actions/file/src/types.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
export type Finding = {
22
scannerType: string
3-
ruleId: string
3+
ruleId?: string
44
url: string
5-
html: string
5+
html?: string
66
problemShort: string
77
problemUrl: string
88
solutionShort: string

.github/actions/file/src/updateFilingsWithNewFindings.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ function getFilingKey(filing: ResolvedFiling | RepeatedFiling): string {
55
}
66

77
function getFindingKey(finding: Finding): string {
8-
return `${finding.url};${finding.ruleId};${finding.html}`
8+
if (finding.ruleId && finding.html) {
9+
return `${finding.url};${finding.ruleId};${finding.html}`
10+
}
11+
return `${finding.url};${finding.scannerType};${finding.problemUrl}`
912
}
1013

1114
export function updateFilingsWithNewFindings(

.github/actions/file/tests/generateIssueBody.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,21 @@ const baseFinding = {
1111
solutionShort: 'ensure the contrast between foreground and background colors meets WCAG thresholds',
1212
}
1313

14+
const findingWithEmptyOptionalFields = {
15+
scannerType: 'reflow',
16+
url: 'https://example.com/404',
17+
problemShort: 'page requires horizontal scrolling at 320x256 viewport',
18+
problemUrl: 'https://www.w3.org/WAI/WCAG21/Understanding/reflow.html',
19+
solutionShort: 'ensure content is responsive and does not require horizontal scrolling at small viewport sizes',
20+
}
21+
1422
describe('generateIssueBody', () => {
1523
it('includes acceptance criteria and omits the Specifically section when solutionLong is missing', () => {
1624
const body = generateIssueBody(baseFinding, 'github/accessibility-scanner')
1725

1826
expect(body).toContain('## What')
1927
expect(body).toContain('## Acceptance Criteria')
20-
expect(body).toContain('The specific axe violation reported in this issue is no longer reproducible.')
28+
expect(body).toContain('The specific violation reported in this issue is no longer reproducible.')
2129
expect(body).not.toContain('Specifically:')
2230
})
2331

@@ -61,4 +69,11 @@ describe('generateIssueBody', () => {
6169
expect(body).not.toContain('View screenshot')
6270
expect(body).not.toContain('.screenshots')
6371
})
72+
73+
it('uses url fallback when html is not present', () => {
74+
const body = generateIssueBody(findingWithEmptyOptionalFields, 'github/accessibility-scanner')
75+
76+
expect(body).toContain(`found an issue on ${findingWithEmptyOptionalFields.url}`)
77+
expect(body).not.toContain('flagged the element')
78+
})
6479
})

.github/actions/file/tests/openIssue.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ describe('openIssue', () => {
6060
expect(octokit.request).toHaveBeenCalledWith(
6161
expect.any(String),
6262
expect.objectContaining({
63-
labels: ['axe rule: color-contrast', 'axe-scanning-issue'],
63+
labels: ['axe-scanning-issue', 'axe rule: color-contrast'],
6464
}),
6565
)
6666
})

.github/actions/find/src/pluginManager.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,18 @@ const __dirname = path.dirname(__filename)
1212

1313
type PluginDefaultParams = {
1414
page: playwright.Page
15-
addFinding: (findingData: Finding) => void
15+
addFinding: (findingData: Finding) => Promise<void>
1616
}
1717

1818
type Plugin = {
1919
name: string
2020
default: (options: PluginDefaultParams) => Promise<void>
2121
}
2222

23+
// Built-in plugin names shipped with the scanner.
24+
// Used to skip duplicates when loading custom plugins.
25+
const BUILT_IN_PLUGINS = ['reflow-scan']
26+
2327
const plugins: Plugin[] = []
2428
let pluginsLoaded = false
2529

@@ -65,7 +69,7 @@ export async function loadCustomPlugins() {
6569

6670
// - currently, the plugin manager will abort loading
6771
// all plugins if there's an error
68-
// - the problem with this is that if a scanner user doesnt
72+
// - the problem with this is that if a scanner user doesn't
6973
// have custom plugins, they won't have a 'scanner-plugins' folder
7074
// which will cause an error and abort loading all plugins, including built-in ones
7175
// - so for custom plugins, if the path doesn't exist, we can return early
@@ -75,19 +79,32 @@ export async function loadCustomPlugins() {
7579
return
7680
}
7781

78-
await loadPluginsFromPath({pluginsPath})
82+
await loadPluginsFromPath({pluginsPath, skipBuiltInPlugins: BUILT_IN_PLUGINS})
7983
}
8084

8185
// exported for mocking/testing. not for actual use
82-
export async function loadPluginsFromPath({pluginsPath}: {pluginsPath: string}) {
86+
export async function loadPluginsFromPath({
87+
pluginsPath,
88+
skipBuiltInPlugins,
89+
}: {
90+
pluginsPath: string
91+
skipBuiltInPlugins?: string[]
92+
}) {
8393
try {
8494
const res = fs.readdirSync(pluginsPath)
8595
for (const pluginFolder of res) {
8696
const pluginFolderPath = path.join(pluginsPath, pluginFolder)
8797

8898
if (fs.existsSync(pluginFolderPath) && fs.lstatSync(pluginFolderPath).isDirectory()) {
89-
core.info(`Found plugin: ${pluginFolder}`)
90-
plugins.push(await dynamicImport(path.join(pluginsPath, pluginFolder, 'index.js')))
99+
const plugin = await dynamicImport(path.join(pluginsPath, pluginFolder, 'index.js'))
100+
101+
if (skipBuiltInPlugins?.includes(plugin.name)) {
102+
core.info(`Skipping built-in plugin: ${plugin.name}`)
103+
continue
104+
}
105+
106+
core.info(`Found plugin: ${plugin.name}`)
107+
plugins.push(plugin)
91108
}
92109
}
93110
} catch (e) {

.github/actions/find/src/types.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
export type Finding = {
22
scannerType: string
33
url: string
4-
html: string
4+
html?: string
55
problemShort: string
66
problemUrl: string
77
solutionShort: string

.github/actions/find/tests/pluginManager.test.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,17 @@ vi.mock('../src/pluginManager.js', {spy: true})
1212
vi.mock('@actions/core', {spy: true})
1313

1414
describe('loadPlugins', () => {
15-
vi.spyOn(dynamicImportModule, 'dynamicImport').mockImplementation(path => Promise.resolve(path))
15+
let dynamicImportCallCount = 0
16+
vi.spyOn(dynamicImportModule, 'dynamicImport').mockImplementation(() => {
17+
dynamicImportCallCount++
18+
return Promise.resolve({name: `plugin-${dynamicImportCallCount}`, default: vi.fn()})
19+
})
1620
beforeEach(() => {
21+
dynamicImportCallCount = 0
1722
// @ts-expect-error - we don't need the full fs readdirsync
1823
// method signature here
19-
vi.spyOn(fs, 'readdirSync').mockImplementation(readPath => {
20-
return [readPath + '/plugin-1', readPath + '/plugin-2']
24+
vi.spyOn(fs, 'readdirSync').mockImplementation(() => {
25+
return ['folder-a', 'folder-b']
2126
})
2227
vi.spyOn(fs, 'lstatSync').mockImplementation(() => {
2328
return {
@@ -61,4 +66,27 @@ describe('loadPlugins', () => {
6166
expect(logSpy).toHaveBeenCalledWith(pluginManager.abortError)
6267
})
6368
})
69+
70+
describe('when a custom plugin folder matches a built-in plugin name', () => {
71+
beforeEach(() => {
72+
// @ts-expect-error - we don't need the full fs readdirsync
73+
// method signature here
74+
vi.spyOn(fs, 'readdirSync').mockImplementation(() => {
75+
return ['reflow-scan']
76+
})
77+
vi.spyOn(dynamicImportModule, 'dynamicImport').mockImplementation(() => {
78+
return Promise.resolve({name: 'reflow-scan', default: vi.fn()})
79+
})
80+
})
81+
82+
it('skips the built-in name in custom plugins and only loads it once', async () => {
83+
pluginManager.clearCache()
84+
const infoSpy = vi.spyOn(core, 'info').mockImplementation(() => {})
85+
const plugins = await pluginManager.loadPlugins()
86+
// Built-in loads it, custom skips the folder by name
87+
expect(plugins.length).toBe(1)
88+
expect(plugins[0].name).toBe('reflow-scan')
89+
expect(infoSpy).toHaveBeenCalledWith('Skipping built-in plugin: reflow-scan')
90+
})
91+
})
6492
})
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
export default async function reflowScan({page, addFinding} = {}) {
2+
const originalViewport = page.viewportSize()
3+
const url = page.url()
4+
// Check for horizontal scrolling at 320x256 viewport
5+
try {
6+
await page.setViewportSize({width: 320, height: 256})
7+
const scrollWidth = await page.evaluate(() => document.documentElement.scrollWidth)
8+
const clientWidth = await page.evaluate(() => document.documentElement.clientWidth)
9+
10+
// If horizontal scroll is required (with 1px tolerance for rounding)
11+
if (scrollWidth > clientWidth + 1) {
12+
await addFinding({
13+
scannerType: 'reflow-scan',
14+
url,
15+
problemShort: 'page requires horizontal scrolling at 320x256 viewport',
16+
problemUrl: 'https://www.w3.org/WAI/WCAG21/Understanding/reflow.html',
17+
solutionShort: 'ensure content is responsive and does not require horizontal scrolling at small viewport sizes',
18+
solutionLong: `The page has a scroll width of ${scrollWidth}px but a client width of only ${clientWidth}px at a 320x256 viewport, requiring horizontal scrolling. This violates WCAG 2.1 Level AA Success Criterion 1.4.10 (Reflow).`,
19+
})
20+
}
21+
} catch (e) {
22+
console.error('Error checking horizontal scroll:', e)
23+
} finally {
24+
// Restore original viewport so subsequent scans (e.g. Axe) aren't affected
25+
if (originalViewport) {
26+
await page.setViewportSize(originalViewport)
27+
}
28+
}
29+
}
30+
31+
export const name = 'reflow-scan'

0 commit comments

Comments
 (0)