-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(jest): add custom results processor for a11y failures #62
Changes from 1 commit
77b7277
5d17571
76aa29f
a6d04be
443b4e1
228b384
483cdf7
aed493c
43f1f5c
ca71b30
84c8546
e183191
be9661f
8cf387f
7f0bbda
dba9a81
fc68b02
b0f480d
0695605
57fa2be
097b2a1
0457cd2
97b8cf0
6f19075
54bf098
dfd4cfb
5179445
dd0a65f
fcaaa3b
1e07f4d
878e9d5
6728614
bcbb2db
4e0ed0b
8be300c
6686a0c
b9ba221
15b0d4b
067a4d6
817985b
f224b55
ce51ed0
be1de3d
52b7e05
0e05588
0a60d6f
fe47b54
c6ef300
8838979
b2482e0
71eea8a
cce6621
5a4fc3f
124256f
73004ac
a8f2d0d
a24ed27
174c47f
467cb5b
b4bfda6
fc355d5
b29acf5
9ef8ac5
131eb9f
34dd7e4
7470dab
aedf3c1
15decaa
fcc2646
88c30a6
acb0ab1
7717c3d
6582930
39c80ef
b424e86
6e0fc7e
3052554
d24cea3
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 |
---|---|---|
|
@@ -12,7 +12,9 @@ Accessibility matcher for [Jest](https://jestjs.io) | |
- [Test module level](#test-module-level) | ||
- [Automatic checks](#automatic-checks) | ||
- [Using environment variables](#using-environment-variables) | ||
- [Workflow](#workflow) | ||
- [Sa11y results processor](#sa11y-results-processor) | ||
- [JSON result transformation](#json-result-transformation) | ||
- [Limitations](#limitations) | ||
- [Caution](#caution) | ||
- [Usage](#usage) | ||
|
||
|
@@ -92,6 +94,8 @@ beforeAll(() => { | |
|
||
The sa11y API can be setup to be automatically invoked at the end of each test as an alternative to adding the `toBeAccessible` API at the end of each test. | ||
|
||
- When automatic checks are enabled each child element in the DOM body will be checked for a11y and failures reported as part of the test. | ||
|
||
```javascript | ||
setup({ autoCheckOpts: { runAfterEach: true } }); | ||
|
||
|
@@ -110,18 +114,95 @@ SA11Y_AUTO=1 SA11Y_CLEANUP=1 jest | |
- Invoking `jest` with environment variables as above will enable automatic checks with no changes required to `setup()` | ||
- The environment variables can be used to set up parallel builds e.g., in a CI environment without code changes to `setup()` to opt-in to automatic checks | ||
|
||
#### Workflow | ||
#### Sa11y results processor | ||
|
||
When automatic checks are enabled | ||
The sa11y custom test results processor can be enabled using e.g., - `jest --json --outputFile results.json --testResultsProcessor node_modules/@sa11y/jest/dist/resultsProcessor.js` | ||
|
||
- Each child element in the DOM body will be checked for a11y, results consolidated and failures reported as part of the test | ||
- sa11y results processor affects only the JSON result output | ||
- It does not affect the default console reporter or output of any other reporter (e.g., HTML reporter) | ||
- a11y errors within a single test file will be de-duped by rule ID and CSS selectors | ||
- a11y errors can be transformed into their own test failures using the sa11y custom test result processor | ||
- `jest --json --outputFile results.json --testResultsProcessor node_modules/@sa11y/jest/dist/resultsProcessor.js` | ||
- a11y errors will be transformed into their own test failures | ||
- This would extract the a11y errors from the original tests and create additional test failures with the WCAG version, level, rule ID, CSS selectors as key | ||
- bringing a11y metadata to forefront instead of being part of stacktrace | ||
- The JSON output can be transformed into JUnit XML format e.g., using [jest-junit](https://github.com/jest-community/jest-junit) | ||
|
||
##### JSON result transformation | ||
|
||
With default results processor - a11y error is embedded within the test failure: | ||
|
||
```json | ||
"assertionResults": [ | ||
{ | ||
"ancestorTitles": [ | ||
"integration test @sa11y/jest" | ||
], | ||
"failureMessages": [ | ||
"A11yError: 1 Accessibility issues found\n * (link-name) Links must have discernible text: a\n\t- Help URL: https://dequeuniversity.com/rules/axe/4.1/link-name\n at Function.checkAndThrow (packages/format/src/format.ts:67:19)\n at automaticCheck (packages/jest/src/automatic.ts:54:19)\n at Object.<anonymous> (packages/jest/src/automatic.ts:69:13)" | ||
], | ||
"fullName": "integration test @sa11y/jest should throw error for inaccessible dom", | ||
"location": null, | ||
"status": "failed", | ||
"title": "should throw error for inaccessible dom" | ||
} | ||
] | ||
``` | ||
|
||
With sa11y results processor: | ||
|
||
- Original JSON test result (failure with embedded a11y error) is disabled | ||
|
||
```json | ||
"assertionResults": [ | ||
{ | ||
"ancestorTitles": [ | ||
"integration test @sa11y/jest" | ||
], | ||
"failureMessages": [ | ||
"A11yError: 1 Accessibility issues found\n * (link-name) Links must have discernible text: a\n\t- Help URL: https://dequeuniversity.com/rules/axe/4.1/link-name\n at Function.checkAndThrow (packages/format/src/format.ts:67:19)\n at automaticCheck (packages/jest/src/automatic.ts:54:19)\n at Object.<anonymous> (packages/jest/src/automatic.ts:69:13)" | ||
], | ||
"fullName": "integration test @sa11y/jest should throw error for inaccessible dom", | ||
"location": null, | ||
"status": "disabled", | ||
"title": "should throw error for inaccessible dom" | ||
}, | ||
] | ||
``` | ||
|
||
- Each unique a11y failure in a test module is extracted as a new test failure and added to a new test suite using a11y metadata as key. This could result in increase of total test count and suite count in the results JSON. | ||
|
||
```json | ||
"assertionResults": [ | ||
{ | ||
"ancestorTitles": [ | ||
"integration test @sa11y/jest", | ||
"integration test @sa11y/jest should throw error for inaccessible dom" | ||
], | ||
"failureMessages": [ | ||
"Accessibility issues found: Links must have discernible text\nCSS Selectors: a\nHTML element: <a href=\"#\"></a>\nHelp: https://dequeuniversity.com/rules/axe/4.1/link-name\nTests: \"integration test @sa11y/jest should throw error for inaccessible dom\"\nSummary: Fix all of the following:\n Element is in tab order and does not have accessible text\n\nFix any of the following:\n Element does not have text that is visible to screen readers\n aria-label attribute does not exist or is empty\n aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty\n Element has no title attribute" | ||
], | ||
"fullName": "Links must have discernible text: a", | ||
"location": null, | ||
"status": "failed", | ||
"title": "should throw error for inaccessible dom" | ||
} | ||
], | ||
``` | ||
|
||
#### Limitations | ||
|
||
Automatic checks currently has the following limitations. | ||
|
||
- Automatic check is triggered regardless of the test status which would result in the original test failure if any getting overwritten by a11y failures if any from automatic checks ([#66](https://github.com/salesforce/sa11y/issues/66)) | ||
- Tests using the sa11y jest api would get tested twice with automatic checks - once as part of the sa11y API in the test and again as part of the automatic check | ||
- a11y issues from automatic checks would overwrite the a11y issues found by the API | ||
- If the sa11y API has been added to the test to check specific intermediate states of the DOM, enabling automatic checks could result in missed a11y issues | ||
- Automatic checks check the DOM state as it is at the end of the test. DOM states before the end of the test are not checked which could result in missed a11y issues. | ||
- If the test cleans up the DOM after execution, as part of teardown e.g., the sa11y automatic check executed at the end of the test would not be able to check the DOM | ||
- Workaround: Remove the DOM cleanup code from the test and opt-in to using sa11y to clean-up the DOM using the options as described above (`cleanupAfterEach: true` or `SA11Y_CLEANUP=1`) | ||
- With the sa11y results processor | ||
- Though the originating test from which the a11y failures are extracted is disabled, and test counts adjusted accordingly - the original test suite failure message still contains the a11y failures. | ||
- The test suite failure message is typically not displayed or used in testing workflows. But if your testing workflow uses the test suite failure message, this might cause confusion. | ||
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. The double indentation here feels a little confusing to me. Can we combine these 3 bullets into a single bullet and a couple sentences maybe? 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. Agree, modified - please take a look. |
||
|
||
## Caution | ||
|
||
- **async**: `toBeAccessible` **must** be invoked with `async/wait` or `Promise` or the equivalent supported asynchronous method in your environment | ||
|
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.
I think this point is also covered by the next one below.
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.
Yes it is related, but there is also a fine difference - Enabling automatic checks without using the API vs while also using the API. Please check if the rewording now makes sense.
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.
I guess to me since you say the automatic check is only at the end of the test it was clear to me that checking intermediate states of the DOM, whether you called the API or not, would not be caught. I'm fine to keep it worded like this though if you think it's more clear.
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.
Thanks Trevor. I will try to revisit and reword/remove it in a subsequent PR.