-
Notifications
You must be signed in to change notification settings - Fork 38
[terra-functional-testing] Add Accessibility Reporter #456
Changes from 2 commits
ac782e8
8d040a4
716acc4
763c33d
908139c
e94194e
51b3981
5f58ab1
35b9085
40c0860
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,13 @@ const { runAxe } = require('../axe'); | |
*/ | ||
function toBeAccessible(_, options = {}) { | ||
const { result } = runAxe(options); | ||
const { violations } = result; | ||
const { incomplete, violations } = result; | ||
|
||
// Rules that fail but are marked for review are returned in the incomplete array. | ||
// https://github.com/dequelabs/axe-core/blob/develop/doc/API.md#results-object | ||
if (incomplete && incomplete.length > 0) { | ||
process.emit('terra:report:accessibility', { incomplete }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also add violations to the report's output? I could see two ways:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Violations are printed to the console when the expect fails; adding to the reporter also would print them again. We'd want to change how violations are printed if we wanted to pursue this, keeping in mind the expect would fail the test and print a message and then the reporter would print a message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to add violations to the report. The failed test run should be indication enough. I see the report as valuable on non-failed test runs. |
||
} | ||
|
||
return { | ||
pass: violations.length === 0, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
const chalk = require('chalk'); | ||
const WDIOReporter = require('@wdio/reporter').default; | ||
|
||
class AccessibilityReporter extends WDIOReporter { | ||
/** | ||
* Indents a string. | ||
* @param {string} string - The string to indent. | ||
* @param {number} indent - The number of spaces to indent the string. | ||
*/ | ||
static indent(string, indent) { | ||
if (indent > 0) { | ||
return string.replace(/^(?!\s*$)/gm, Array(indent + 1).join(' ')); | ||
} | ||
|
||
return string; | ||
} | ||
|
||
constructor(options) { | ||
super(options); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we defined the outputDir and logFile option to pass to super ? It seems there is only one option for output files? maybe, which option 2 from this comment: https://github.com/cerner/terra-toolkit/pull/456/files#r513483163 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the discussion in our WDIO 6 sync I'd say this PR is not going to write out to a file, but it is something we can discuss and circle back on down the road if we think it would be beneficial |
||
|
||
this.currentTest = null; | ||
this.accessibilityResults = {}; | ||
|
||
process.on('terra:report:accessibility', this.onReportAccessibility.bind(this)); | ||
} | ||
|
||
/** | ||
* Hook invoked at the start of the test. | ||
* @param {TestStats} test - The current test object. | ||
*/ | ||
onTestStart(test) { | ||
this.currentTest = test.uid; | ||
} | ||
|
||
/** | ||
* Hook invoked when accessibility results are reported. | ||
* @param {Object} results - Accessibility results. | ||
* @param {array} results.incomplete - Incomplete accessibility results. | ||
*/ | ||
onReportAccessibility(results) { | ||
this.accessibilityResults[this.currentTest] = results; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to know the suite info here instead of recursively building it out at the end? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably, but I personally think traveling the tree is actually easier than tracking additional test data manually. A suite can be nested infinitely inside of other suites |
||
} | ||
|
||
/** | ||
* Hook invoked when the runner ends. | ||
* @param RunnerStats runner - The test runner stats object. | ||
*/ | ||
onRunnerEnd(runner) { | ||
this.printReport(runner); | ||
} | ||
|
||
/** | ||
* Print the accessibility report. | ||
* @param {RunnerStats} runner - The test runner. | ||
*/ | ||
printReport(runner) { | ||
const warnings = Object.keys(this.accessibilityResults); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any way to write to the console that an aggregated report can be found in the results dir There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can write to the console, but we don't currently write any files to the results dir |
||
|
||
if (warnings.length === 0) { | ||
return; | ||
} | ||
|
||
const rootSuite = this.currentSuites.find((suite) => suite.uid === '(root)'); | ||
const output = this.travelSuite(rootSuite); | ||
const spec = runner.specs[0]; | ||
|
||
this.write(`Spec: ${spec}\n${output}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we output this as a md/json report as well that can be pulled in as an build artifact and as a release site artifact? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll probably want something like this, but I think that work will likely happen inside of the WDIO 6 reporter that generates the reports (which may or may not be the same reporter) |
||
} | ||
|
||
/** | ||
* Travels the suite tree to generate the accessibility report. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by travel? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Travels though each node in the tree |
||
* @param {SuiteStats} currentSuite - The current suite. | ||
* @param {number} depth - The current nesting depth of the suite tree. | ||
* @returns {string} - An accessibility report. | ||
*/ | ||
travelSuite(currentSuite, depth = 0) { | ||
const { suites, tests, title } = currentSuite; | ||
|
||
const warnings = []; | ||
|
||
suites.forEach((suite) => { | ||
const output = this.travelSuite(suite, depth + 1); | ||
|
||
if (output.length > 0) { | ||
warnings.push(output); | ||
} | ||
}); | ||
|
||
tests.forEach((test) => { | ||
const { uid } = test; | ||
|
||
if (this.accessibilityResults[uid]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to ensure that this result specifically has the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's guaranteed, with the current implementation the results are only stored if they contain an incomplete array |
||
const testTitle = this.formatTestWarning(uid, (depth * 2) + 2); | ||
|
||
warnings.push(testTitle); | ||
} | ||
}); | ||
|
||
if (warnings.length > 0) { | ||
const suiteTitle = title === '(root)' ? '' : title; | ||
|
||
return `${AccessibilityReporter.indent(suiteTitle, (depth * 2))}\n${warnings.join('\n')}\n`; | ||
} | ||
|
||
return ''; | ||
} | ||
|
||
/** | ||
* Formats an accessibility warning summary for a test. | ||
* @param {string} uid - The unique identifier of the test. | ||
* @param {number} indent - The indentation the report should have, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you intend to have more comment at the end or should it be a period? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a period, updated: |
||
*/ | ||
formatTestWarning(uid, indent) { | ||
const { title } = this.tests[uid]; | ||
const { incomplete } = this.accessibilityResults[uid]; | ||
|
||
const string = `${chalk.yellow('warning')} ${title}\n\n ${chalk.yellow(`${JSON.stringify(incomplete, null, 2)}`)}`; | ||
|
||
return AccessibilityReporter.indent(string, indent); | ||
} | ||
} | ||
|
||
module.exports = AccessibilityReporter; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
{ | ||
"title": "(root)", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be the spec file that was ran? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this was generated directly by the test runner and copy pasted into this file. The root suite is always assigned '(root)': |
||
"suites": [ | ||
{ | ||
"type": "suite:start", | ||
"start": "2020-10-27T01:14:33.430Z", | ||
"_duration": 214, | ||
"uid": "suite-0-0", | ||
"cid": "0-0", | ||
"title": "Example Describe 1", | ||
"fullTitle": "Example Describe 1", | ||
"tests": [], | ||
"hooks": [], | ||
"suites": [ | ||
{ | ||
"type": "suite:start", | ||
"start": "2020-10-27T01:14:33.430Z", | ||
"_duration": 213, | ||
"uid": "suite-1-0", | ||
"cid": "0-0", | ||
"title": "Example Describe 2", | ||
"fullTitle": "Example Describe 1 Example Describe 2", | ||
"tests": [ | ||
{ | ||
"type": "test", | ||
"start": "2020-10-27T01:14:33.431Z", | ||
"_duration": 101, | ||
"uid": "test-10-0", | ||
"cid": "0-0", | ||
"title": "example it 1", | ||
"fullTitle": "Example Describe 1 Example Describe 2 example it 1", | ||
"output": [], | ||
"retries": 0, | ||
"state": "passed", | ||
"end": "2020-10-27T01:14:33.532Z" | ||
}, | ||
{ | ||
"type": "test", | ||
"start": "2020-10-27T01:14:33.532Z", | ||
"_duration": 53, | ||
"uid": "test-10-1", | ||
"cid": "0-0", | ||
"title": "example it 2", | ||
"fullTitle": "Example Describe 1 Example Describe 2 example it 2", | ||
"output": [], | ||
"retries": 0, | ||
"state": "passed", | ||
"end": "2020-10-27T01:14:33.585Z" | ||
}, | ||
{ | ||
"type": "test", | ||
"start": "2020-10-27T01:14:33.586Z", | ||
"_duration": 57, | ||
"uid": "test-10-2", | ||
"cid": "0-0", | ||
"title": "example it 3", | ||
"fullTitle": "Example Describe 1 Example Describe 2 example it 3", | ||
"output": [], | ||
"retries": 0, | ||
"state": "passed", | ||
"end": "2020-10-27T01:14:33.643Z" | ||
} | ||
], | ||
"hooks": [], | ||
"suites": [], | ||
"hooksAndTests": [], | ||
"end": "2020-10-27T01:14:33.643Z" | ||
} | ||
], | ||
"hooksAndTests": [], | ||
"end": "2020-10-27T01:14:33.644Z" | ||
} | ||
], | ||
"tests": [] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restoreScroll
was removed: dequelabs/axe-core#2388There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know the scrolling that it used to do was only for color-contrast.