Axe API integration with Jest and Puppeteer by gziolo · Pull Request #13241 · WordPress/gutenberg

gziolo · 2019-01-08T16:04:19Z

Description

Implementation based on #12743.

Hopefully this is th final try to include axe integration with out e2e tests setup. This PR introduces new Jest matcher which takes a Puppeteer page instance and check if it is accesible as shown below:

await expect( page ).toPassAxeTests( {
	include: '.block-editor',
	exclude: '#metaboxes, #post-title-0, .editor-default-block-appender__content',
} );

How has this been tested?

npm run test-e2e with the following patch applied in addition to those changes:

diff --git a/packages/tests-e2e/support/setup-test-framework.js b/packages/tests-e2e/support/setup-test-framework.js
index cc8168644da..89cfc37b127 100644
--- a/packages/tests-e2e/support/setup-test-framework.js
+++ b/packages/tests-e2e/support/setup-test-framework.js
@@ -162,6 +162,7 @@ beforeAll( async () => {
 } );
 
 afterEach( async () => {
+	await expect( page ).toPassAxeTests();
 	await setupBrowser();
 } );

Types of changes

New feature for e2e testing.

Known issues

There is a bug in Jest with async matcher which makes them print differently than regular matchers. See jestjs/jest#7066 for more details. They are working on a fix and it looks like it might land in the next major release of Jest.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

gziolo · 2019-01-11T17:28:15Z

Logs with accessibility violations (closer to the end of file) for jobs without popular plugins and author role:

https://api.travis-ci.org/v3/job/478442476/log.txt (part 1)
https://api.travis-ci.org/v3/job/478442477/log.txt (part 2)

To test it locally try the following diff:

diff --git a/packages/tests-e2e/support/setup-test-framework.js b/packages/tests-e2e/support/setup-test-framework.js
index cc8168644da..89cfc37b127 100644
--- a/packages/tests-e2e/support/setup-test-framework.js
+++ b/packages/tests-e2e/support/setup-test-framework.js
@@ -162,6 +162,7 @@ beforeAll( async () => {
 } );
 
 afterEach( async () => {
+	await expect( page ).toPassAxeTests();
 	await setupBrowser();
 } );

gziolo · 2019-01-11T19:19:22Z

This is ready for review. This PR in the final version adds only new package which exposes the async matcher to validate whether a given page or its part is accessible according to Axe tool analysis.

@stephenmathieson and @dylanb, I would appreciate your feedback on this one as it's based on axe-puppeteer and your great work started at WC US (#12743) :)

greatislander

I've proposed some changes based on concerns that the function name and documentation could lead users to think that running Axe alone can accurately determine whether or not a page is accessible. Any automated accessibility tool can only catch a certain subset of potential accessibility issues, and must be combined with manual evaluation. It is probably better not to use language like:

expect( page ).toBeAccessible();

But rather to use documentation and function names which indicates that the evaluation is based on criteria of this specific tool, e.g.:

expect( page ).toPassAxeTests();

Open to and grateful for other ideas of how to frame this.

packages/jest-puppeteer-axe/README.md

packages/jest-puppeteer-axe/src/index.js

gziolo · 2019-01-14T20:13:31Z

I've proposed some changes based on concerns that the function name and documentation could lead users to think that running Axe alone can accurately determine whether or not a page is accessible. Any automated accessibility tool can only catch a certain subset of potential accessibility issues, and must be combined with manual evaluation.

Totally agree with your reasoning 👍 I didn't think about it at all. I was too focused on the technical aspects. Thanks for your review. I applied all the changes you proposed as they better explain the intents of this new Jest matcher introduced. I'm not 100% sure if tests in expect( page ).toPassAxeTests(); is the best fit but I'm having hard time finding out a better alternative. Some ideas I had are:

expect( page ).toPassAxeChecks();
expect( page ).toPassAxeAnalysis();
expect( page ).toPassAxeVerification();

We can totally proceed as is if there isn't a better proposal.

greatislander · 2019-01-14T20:17:09Z

@gziolo thanks for accepting the suggestions! I hear the concern around using Tests in the function name. Of your suggestions I think I'd lean towards:

expect( page ).toPassAxeChecks();

I also thought of:

expect( page ).toPassAxeAudit();

Open to other suggestions too.

aduth

The messaging of a failing test is non-conventional, with only an "Error" text being highlighted. In direct conversation, it was mentioned this is a bug present in Jest for the moment. Pending @gziolo to share.

In the meantime, this appears good as it stands. 👍

packages/jest-console/package.json

packages/jest-puppeteer-axe/package.json

packages/tests-e2e/support/setup-test-framework.js

Co-Authored-By: gziolo <grzegorz@gziolo.pl>

dylanb · 2019-01-26T15:46:51Z

I am a little late on this but this looks like great progress

dylanb · 2019-01-26T15:49:53Z

@gziolo seems like a good improvement would be to publish a axe-jest-puppeteer module that implements the matcher and use that - WDYT?

cc/ @WilcoFiers @stephenmathieson

gziolo · 2019-01-28T11:05:27Z

This is going to be published to npm as @wordpress/jest-puppeteer-axe closer to the end of February, is it what you mean?

* Axe API integration with Jest and Puppeteer * Implement expect matcher for accessibility checks * Docs: Add documentation for jest-puppeteer-axe package * Apply suggestions from code review Co-Authored-By: gziolo <grzegorz@gziolo.pl>

gziolo self-assigned this Jan 8, 2019

gziolo force-pushed the add/jest-puppeteer-axe branch from 73baa4d to 5645f37 Compare January 8, 2019 16:07

gziolo removed their assignment Jan 8, 2019

gziolo force-pushed the add/jest-puppeteer-axe branch from f66adb2 to e052132 Compare January 11, 2019 17:02

gziolo force-pushed the add/jest-puppeteer-axe branch from e85c171 to d9699bb Compare January 11, 2019 19:12

gziolo requested review from ntwb and tofumatt January 11, 2019 19:15

gziolo added npm Packages Related to npm packages and removed [Status] In Progress Tracking issues with work in progress labels Jan 11, 2019

gziolo requested review from joemcgill and jorgefilipecosta January 11, 2019 19:19

greatislander suggested changes Jan 14, 2019

View reviewed changes

aduth approved these changes Jan 15, 2019

View reviewed changes

packages/jest-console/package.json Outdated Show resolved Hide resolved

packages/jest-puppeteer-axe/package.json Outdated Show resolved Hide resolved

packages/tests-e2e/support/setup-test-framework.js Outdated Show resolved Hide resolved

gziolo added 2 commits January 15, 2019 17:35

Axe API integration with Jest and Puppeteer

e10c4be

Implement expect matcher for accessibility checks

192c3ff

gziolo force-pushed the add/jest-puppeteer-axe branch from f96302a to d8c0c28 Compare January 15, 2019 17:44

Docs: Add documentation for jest-puppeteer-axe package

81ea2c5

gziolo force-pushed the add/jest-puppeteer-axe branch from d8c0c28 to 81ea2c5 Compare January 15, 2019 18:01

Apply suggestions from code review

ae95337

Co-Authored-By: gziolo <grzegorz@gziolo.pl>

gziolo merged commit 2368662 into master Jan 15, 2019

gziolo deleted the add/jest-puppeteer-axe branch January 15, 2019 18:30

gziolo added this to the 4.9 (Gutenberg) milestone Jan 15, 2019

gziolo mentioned this pull request Jan 29, 2019

test: add a11y reporting using axe-puppeteer #12743

Closed

4 tasks

gziolo mentioned this pull request Apr 18, 2019

Testing: Add partial support for Axe verification in e2e tests #15018

Merged

Conversation

gziolo commented Jan 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Types of changes

Known issues

Checklist:

Uh oh!

gziolo commented Jan 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gziolo commented Jan 11, 2019

Uh oh!

greatislander left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gziolo commented Jan 14, 2019

Uh oh!

greatislander commented Jan 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dylanb commented Jan 26, 2019

Uh oh!

dylanb commented Jan 26, 2019

Uh oh!

gziolo commented Jan 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gziolo commented Jan 8, 2019 •

edited

Loading

gziolo commented Jan 11, 2019 •

edited

Loading

greatislander commented Jan 14, 2019 •

edited

Loading