Skip to content

Commit 23fb006

Browse files
committed
fix: distinguish WCAG violations from best practices in issue output
Addresses issue #34 - The scanner previously conflated best-practice recommendations with actual WCAG conformance failures in auto-generated issue titles and bodies. This led to Copilot filing PRs claiming WCAG violations when only best-practice rules were triggered. Changes: - Add `ruleType` field to Finding type (wcag | best-practice | experimental) - Extract rule type from axe-core violation tags in findForUrl - Update issue body generator with clear type badge and description - Update issue labels: add wcag-violation / best-practice / experimental labels - Update tests with new label expectations - Fix ESLint error in test plugin (unused vars) - Add missing esbuild devDependency - Include .js files in eslint config
1 parent 9d7a8fe commit 23fb006

11 files changed

Lines changed: 587 additions & 9 deletions

File tree

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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,34 @@
11
import type {Finding} from './types.d.js'
22

3+
/**
4+
* Determines if a finding is a WCAG violation or a best practice recommendation.
5+
*/
6+
function getRuleTypeLabel(ruleType?: string): { heading: string; badge: string; isWcag: boolean } {
7+
if (ruleType === 'best-practice') {
8+
return {
9+
heading: 'Best Practice Recommendation',
10+
badge: '\U0001F4A1 Best Practice',
11+
isWcag: false,
12+
}
13+
}
14+
if (ruleType === 'experimental') {
15+
return {
16+
heading: 'Experimental Rule',
17+
badge: '\U0001F9EA Experimental',
18+
isWcag: false,
19+
}
20+
}
21+
// Default to WCAG violation
22+
return {
23+
heading: 'WCAG Violation',
24+
badge: '\U0001F6A8 WCAG Violation',
25+
isWcag: true,
26+
}
27+
}
28+
329
export function generateIssueBody(finding: Finding, screenshotRepo: string): string {
30+
const ruleTypeLabel = getRuleTypeLabel(finding.ruleType)
31+
432
const solutionLong = finding.solutionLong
533
?.split('\n')
634
.map((line: string) =>
@@ -18,15 +46,27 @@ export function generateIssueBody(finding: Finding, screenshotRepo: string): str
1846
`
1947
}
2048

49+
const ruleTypeSection = `> **Type:** ${ruleTypeLabel.badge}
50+
>
51+
> ${ruleTypeLabel.isWcag
52+
? 'This is a **WCAG conformance failure**. Fixing this issue helps meet WCAG 2.1 accessibility requirements.'
53+
: 'This is a **best practice recommendation**, not a WCAG conformance failure. Fixing it improves accessibility but is not required for WCAG compliance.'
54+
}`
55+
2156
const acceptanceCriteria = `## Acceptance Criteria
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.
57+
- [ ] The specific issue reported in this issue is no longer reproducible.
58+
${ruleTypeLabel.isWcag
59+
? '- [ ] The fix MUST meet WCAG 2.1 guidelines OR the accessibility standards specified by the repository or organization.'
60+
: '- [ ] The fix SHOULD follow recognized accessibility best practices to improve the user experience.'
61+
}
62+
- [ ] A test SHOULD be added to ensure this specific issue does not regress.
2563
- [ ] This PR MUST NOT introduce any new accessibility issues or regressions.`
2664

27-
const body = `## What
65+
const body = `## ${ruleTypeLabel.heading}
2866
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}.
2967
68+
${ruleTypeSection}
69+
3070
${screenshotSection ?? ''}
3171
To fix this, ${finding.solutionShort}.
3272
${solutionLong ? `\nSpecifically:\n\n${solutionLong}` : ''}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@ export async function openIssue(octokit: Octokit, repoWithOwner: string, finding
2222
const repo = repoWithOwner.split('/')[1]
2323

2424
const labels = [`${finding.scannerType}-scanning-issue`]
25+
26+
// Add rule type label to distinguish WCAG violations from best practices
27+
if (finding.ruleType === 'best-practice') {
28+
labels.push('best-practice')
29+
} else if (finding.ruleType === 'experimental') {
30+
labels.push('experimental')
31+
} else {
32+
// Default to wcag for any WCAG-tagged rule
33+
labels.push('wcag-violation')
34+
}
35+
2536
// Only include a ruleId label when it's defined
2637
if (finding.ruleId) {
2738
labels.push(`${finding.scannerType} rule: ${finding.ruleId}`)

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
export type Finding = {
22
scannerType: string
33
ruleId?: string
4+
/** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */
5+
ruleType?: string
46
url: string
57
html?: string
68
problemShort: string

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@ describe('generateIssueBody', () => {
2323
it('includes acceptance criteria and omits the Specifically section when solutionLong is missing', () => {
2424
const body = generateIssueBody(baseFinding, 'github/accessibility-scanner')
2525

26-
expect(body).toContain('## What')
26+
expect(body).toContain('## WCAG Violation')
27+
expect(body).toContain('\U0001F6A8 WCAG Violation')
2728
expect(body).toContain('## Acceptance Criteria')
28-
expect(body).toContain('The specific violation reported in this issue is no longer reproducible.')
29+
expect(body).toContain('The specific issue reported in this issue is no longer reproducible.')
2930
expect(body).not.toContain('Specifically:')
3031
})
3132

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,31 @@ describe('openIssue', () => {
6060
expect(octokit.request).toHaveBeenCalledWith(
6161
expect.any(String),
6262
expect.objectContaining({
63-
labels: ['axe-scanning-issue', 'axe rule: color-contrast'],
63+
labels: ['axe-scanning-issue', 'wcag-violation', 'axe rule: color-contrast'],
64+
}),
65+
)
66+
})
67+
68+
it('labels best-practice findings correctly', async () => {
69+
const octokit = mockOctokit()
70+
await openIssue(octokit, 'org/repo', {...baseFinding, ruleType: 'best-practice'})
71+
72+
expect(octokit.request).toHaveBeenCalledWith(
73+
expect.any(String),
74+
expect.objectContaining({
75+
labels: ['axe-scanning-issue', 'best-practice', 'axe rule: color-contrast'],
76+
}),
77+
)
78+
})
79+
80+
it('labels experimental findings correctly', async () => {
81+
const octokit = mockOctokit()
82+
await openIssue(octokit, 'org/repo', {...baseFinding, ruleType: 'experimental'})
83+
84+
expect(octokit.request).toHaveBeenCalledWith(
85+
expect.any(String),
86+
expect.objectContaining({
87+
labels: ['axe-scanning-issue', 'experimental', 'axe rule: color-contrast'],
6488
}),
6589
)
6690
})

.github/actions/find/src/findForUrl.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,25 @@ async function runAxeScan({
7979

8080
if (rawFindings) {
8181
for (const violation of rawFindings.violations) {
82+
// Determine rule type from axe-core tags
83+
const tags = violation.tags ?? []
84+
let ruleType: string | undefined
85+
if (tags.some((tag: string) => tag.startsWith('wcag'))) {
86+
ruleType = 'wcag'
87+
} else if (tags.includes('best-practice')) {
88+
ruleType = 'best-practice'
89+
} else if (tags.includes('experimental')) {
90+
ruleType = 'experimental'
91+
}
92+
8293
await addFinding({
8394
scannerType: 'axe',
8495
url,
8596
html: violation.nodes[0].html.replace(/'/g, '''),
8697
problemShort: violation.help.toLowerCase().replace(/'/g, '''),
8798
problemUrl: violation.helpUrl.replace(/'/g, '''),
8899
ruleId: violation.id,
100+
ruleType,
89101
solutionShort: violation.description.toLowerCase().replace(/'/g, '''),
90102
solutionLong: violation.nodes[0].failureSummary?.replace(/'/g, '''),
91103
})

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ export type Finding = {
88
solutionLong?: string
99
screenshotId?: string
1010
ruleId?: string
11+
/** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */
12+
ruleType?: string
1113
}
1214

1315
export type Cookie = {

.github/scanner-plugins/test-js-file-plugin-loading/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// - this exist as a test to verify that loading plugins
22
// via js files still works and there are no regressions
33

4-
export default async function TestJsFilePluginLoad({ page, addFinding } = {}) {
4+
export default async function TestJsFilePluginLoad() {
55
console.log('testing plugin load using js file')
66
}
77

eslint.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import tseslint from "typescript-eslint";
22
import prettierConfig from "eslint-config-prettier";
33

44
export default tseslint.config(...tseslint.configs.recommended, prettierConfig, {
5-
files: ["**/*.ts"],
5+
files: ["**/*.ts", "**/*.js"],
66
rules: {
77
"@typescript-eslint/no-unused-vars": [
88
"error",

0 commit comments

Comments
 (0)