Skip to content
Merged
8 changes: 8 additions & 0 deletions .github/actions/file/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ inputs:
description: "When true, log the issues that would be filed without opening, closing, or reopening any issues."
required: false
default: "false"
file_best_practice_issues:
description: "File issues for best-practice findings (accessibility recommendations that are not hard WCAG failures). Disabling only suppresses new issues; existing ones are left untouched."
required: false
default: "true"
file_experimental_issues:
description: "File issues for experimental findings (checks that are not yet stable). Disabling only suppresses new issues; existing ones are left untouched."
required: false
default: "true"

outputs:
filings_file:
Expand Down
16 changes: 14 additions & 2 deletions .github/actions/file/src/generateIssueBody.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,25 @@ export function generateIssueBody(finding: Finding, screenshotRepo: string): str
`
}

const categoryNotice =
Comment thread
kzhou314 marked this conversation as resolved.
finding.category && finding.category !== 'wcag'
? `> [!NOTE]\n> This is ${
finding.category === 'experimental' ? 'an experimental check' : 'a best-practice recommendation'
}, not a definite WCAG failure.\n\n`
: ''

const standardsLine =
finding.category && finding.category !== 'wcag'
? '- [ ] The fix MUST meet the accessibility standards specified by the repository or organization (WCAG 2.2 if applicable).'
: '- [ ] The fix MUST meet WCAG 2.2 guidelines OR the accessibility standards specified by the repository or organization.'

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

const body = `## What
const body = `${categoryNotice}## What
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}.

${screenshotSection ?? ''}
Expand Down
31 changes: 30 additions & 1 deletion .github/actions/file/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import {updateFilingsWithNewFindings} from './updateFilingsWithNewFindings.js'
import {OctokitResponse} from '@octokit/types'
const OctokitWithThrottling = Octokit.plugin(throttling)

// core.getBooleanInput throws on unset inputs, so apply the default first.
function getBooleanInputWithDefault(name: string, defaultValue: boolean): boolean {
if (!core.getInput(name)) return defaultValue
return core.getBooleanInput(name)
}

export default async function () {
core.info("Started 'file' action")
const findingsFile = core.getInput('findings_file', {required: true})
Expand All @@ -31,13 +37,17 @@ export default async function () {
: []
const shouldOpenGroupedIssues = core.getBooleanInput('open_grouped_issues')
const dryRun = core.getBooleanInput('dry_run')
const fileBestPracticeIssues = getBooleanInputWithDefault('file_best_practice_issues', true)
const fileExperimentalIssues = getBooleanInputWithDefault('file_experimental_issues', true)
core.debug(`Input: 'findings_file: ${findingsFile}'`)
core.debug(`Input: 'repository: ${repoWithOwner}'`)
core.debug(`Input: 'base_url: ${baseUrl ?? '(default)'}'`)
core.debug(`Input: 'screenshot_repository: ${screenshotRepo}'`)
core.debug(`Input: 'cached_filings_file: ${cachedFilingsFile}'`)
core.debug(`Input: 'open_grouped_issues: ${shouldOpenGroupedIssues}'`)
core.debug(`Input: 'dry_run: ${dryRun}'`)
core.debug(`Input: 'file_best_practice_issues: ${fileBestPracticeIssues}'`)
core.debug(`Input: 'file_experimental_issues: ${fileExperimentalIssues}'`)

const octokit = new OctokitWithThrottling({
auth: token,
Expand All @@ -61,6 +71,9 @@ export default async function () {
})
const filings = updateFilingsWithNewFindings(cachedFilings, findings)

// Suppressed new filings are kept out of the cache
const suppressedFilings = new Set<Filing>()

// Fetch closed wontfix issues once up front; a failed fetch reopens as usual
let wontfixIssueNumbers = new Set<number>()
if (!dryRun) {
Expand All @@ -80,6 +93,21 @@ export default async function () {
for (const filing of filings) {
let response: OctokitResponse<IssueResponse> | undefined
try {
// Category switches gate only new issues
if (isNewFiling(filing)) {
const category = filing.findings[0].category ?? 'wcag'
if (
(category === 'best-practice' && !fileBestPracticeIssues) ||
(category === 'experimental' && !fileExperimentalIssues)
) {
core.info(
`Skipping new ${category} issue (filing disabled for this category): ${filing.findings[0].problemShort}`,
)
suppressedFilings.add(filing)
continue
}
}

if (dryRun) {
if (isResolvedFiling(filing)) {
dryRunCounts.close++
Expand Down Expand Up @@ -182,7 +210,8 @@ export default async function () {
}

const filingsPath = path.join(process.env.RUNNER_TEMP || '/tmp', `filings-${crypto.randomUUID()}.json`)
fs.writeFileSync(filingsPath, JSON.stringify(filings))
const outputFilings = suppressedFilings.size > 0 ? filings.filter(f => !suppressedFilings.has(f)) : filings
fs.writeFileSync(filingsPath, JSON.stringify(outputFilings))
core.setOutput('filings_file', filingsPath)

core.debug(`Output: 'filings_file: ${filingsPath}'`)
Expand Down
4 changes: 4 additions & 0 deletions .github/actions/file/src/openIssue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export async function openIssue(octokit: Octokit, repoWithOwner: string, finding
if (finding.ruleId) {
labels.push(`${finding.scannerType} rule: ${finding.ruleId}`)
}
// Flag non-WCAG findings so they can be filtered or triaged separately
if (finding.category && finding.category !== 'wcag') {
labels.push(finding.category)
}

const title = truncateWithEllipsis(
`Accessibility issue: ${finding.problemShort[0].toUpperCase() + finding.problemShort.slice(1)} on ${new URL(finding.url).pathname}`,
Expand Down
3 changes: 3 additions & 0 deletions .github/actions/file/src/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
export type FindingCategory = 'wcag' | 'best-practice' | 'experimental'

export type Finding = {
scannerType: string
category?: FindingCategory
ruleId?: string
url: string
html?: string
Expand Down
28 changes: 28 additions & 0 deletions .github/actions/file/tests/generateIssueBody.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('generateIssueBody', () => {
expect(body).toContain('## What')
expect(body).toContain('## Acceptance Criteria')
expect(body).toContain('The specific violation reported in this issue is no longer reproducible.')
expect(body).toContain('The fix MUST meet WCAG 2.2 guidelines OR')
expect(body).not.toContain('Specifically:')
})

Expand Down Expand Up @@ -76,4 +77,31 @@ describe('generateIssueBody', () => {
expect(body).toContain(`found an issue on ${findingWithEmptyOptionalFields.url}`)
expect(body).not.toContain('flagged the element')
})

it('omits the category notice for WCAG findings', () => {
expect(generateIssueBody(baseFinding, 'github/accessibility-scanner')).not.toContain('> [!NOTE]')
expect(generateIssueBody({...baseFinding, category: 'wcag'}, 'github/accessibility-scanner')).not.toContain(
'> [!NOTE]',
)
})

it('includes a best-practice notice for best-practice findings', () => {
const body = generateIssueBody({...baseFinding, category: 'best-practice'}, 'github/accessibility-scanner')

expect(body).toContain('> [!NOTE]')
expect(body).toContain('best-practice recommendation')
expect(body).toContain('not a definite WCAG failure')
expect(body).toContain('WCAG 2.2 if applicable')
expect(body).not.toContain('The fix MUST meet WCAG 2.2 guidelines OR')
})

it('includes an experimental notice for experimental findings', () => {
const body = generateIssueBody({...baseFinding, category: 'experimental'}, 'github/accessibility-scanner')

expect(body).toContain('> [!NOTE]')
expect(body).toContain('an experimental check')
expect(body).toContain('not a definite WCAG failure')
expect(body).toContain('WCAG 2.2 if applicable')
expect(body).not.toContain('The fix MUST meet WCAG 2.2 guidelines OR')
})
})
22 changes: 22 additions & 0 deletions .github/actions/file/tests/openIssue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,28 @@ describe('openIssue', () => {
)
})

it('adds a category label for non-WCAG findings', async () => {
const octokit = mockOctokit()
await openIssue(octokit, 'org/repo', {...baseFinding, category: 'best-practice'})

expect(octokit.request).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
labels: ['axe-scanning-issue', 'axe rule: color-contrast', 'best-practice'],
}),
)
})

it('does not add a category label for WCAG findings', async () => {
const octokit = mockOctokit()
await openIssue(octokit, 'org/repo', {...baseFinding, category: 'wcag'})

const labels = octokit.request.mock.calls[0][1].labels
expect(labels).not.toContain('wcag')
expect(labels).not.toContain('best-practice')
expect(labels).not.toContain('experimental')
})

it('truncates long titles with ellipsis', async () => {
const octokit = mockOctokit()
const longFinding = {
Expand Down
11 changes: 10 additions & 1 deletion .github/actions/find/src/findForUrl.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type {ColorSchemePreference, Finding, ReducedMotionPreference, UrlConfig} from './types.d.js'
import type {ColorSchemePreference, Finding, FindingCategory, ReducedMotionPreference, UrlConfig} from './types.d.js'
import {AxeBuilder} from '@axe-core/playwright'
import playwright from 'playwright'
import {AuthContext} from './AuthContext.js'
Expand Down Expand Up @@ -87,6 +87,7 @@ async function runAxeScan({
for (const violation of rawFindings.violations) {
await addFinding({
scannerType: 'axe',
category: categorizeAxeViolation(violation.tags),
url,
html: violation.nodes[0].html.replace(/'/g, '&apos;'),
problemShort: violation.help.toLowerCase().replace(/'/g, '&apos;'),
Expand All @@ -98,3 +99,11 @@ async function runAxeScan({
}
}
}

// Maps an Axe violation's tags to a conformance tier. Experimental is checked
// first because some experimental rules also carry a wcag* tag.
function categorizeAxeViolation(tags: string[]): FindingCategory {
if (tags.includes('experimental')) return 'experimental'
if (tags.includes('best-practice')) return 'best-practice'
return 'wcag'
}
3 changes: 3 additions & 0 deletions .github/actions/find/src/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
export type FindingCategory = 'wcag' | 'best-practice' | 'experimental'

export type Finding = {
scannerType: string
category?: FindingCategory
url: string
html?: string
problemShort: string
Expand Down
36 changes: 36 additions & 0 deletions .github/actions/find/tests/findForUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,40 @@ describe('findForUrl', () => {
expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0)
})
})

describe('axe finding categorization', () => {
function axeViolation(tags: string[]) {
return {
id: 'some-rule',
help: 'Help',
helpUrl: 'https://example.com',
description: 'Description',
tags,
nodes: [{html: '<div></div>', failureSummary: 'summary'}],
}
}

async function categoryFor(tags: string[]) {
clearAll()
actionInput = JSON.stringify(['axe'])
vi.mocked(AxeBuilder.prototype.analyze).mockResolvedValueOnce({
violations: [axeViolation(tags)],
} as unknown as axe.AxeResults)

const findings = await findForUrl('test.com')
return findings[0].category
}

it('categorizes a violation with only wcag tags as wcag', async () => {
expect(await categoryFor(['wcag2a', 'wcag111'])).toBe('wcag')
})

it('categorizes a violation with a best-practice tag as best-practice', async () => {
expect(await categoryFor(['cat.semantics', 'best-practice'])).toBe('best-practice')
})

it('categorizes a violation with an experimental tag as experimental, even alongside wcag tags', async () => {
expect(await categoryFor(['wcag2a', 'experimental'])).toBe('experimental')
})
})
})
Loading