Skip to content
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

[Accessibility] test(react): add toHaveNoViolations matcher #3927

Merged
merged 12 commits into from
Sep 9, 2019

Conversation

joshblack
Copy link
Contributor

I've been wanting to explore how to add axe-core as a Jest matcher and ended up throwing this exploratory PR together. Specifically, this PR adds the following matcher:

expect(...).toHaveNoViolations();

Under the hood, this matcher uses axe-core to run tests on the given HTML Element. This PR also shows how it could be used in our React codebase under Accordion-test.js

Changelog

New

  • setupAfterEnv.js test config for setting up custom jest matchers

Changed

  • Accordion-test.js now includes an it('should have no AVT1 violations') block

Removed

@joshblack joshblack requested review from dakahn and a team September 6, 2019 14:22
@ghost ghost requested review from jnm2377 and removed request for a team September 6, 2019 14:22
@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for the-carbon-components ready!

Built with commit dd458ff

https://deploy-preview-3927--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-elements ready!

Built with commit dd458ff

https://deploy-preview-3927--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-components-react failed.

Built with commit dd458ff

https://app.netlify.com/sites/carbon-components-react/deploys/5d726bb04aa2c400077aa37a

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for the-carbon-components ready!

Built with commit 73cf6f3

https://deploy-preview-3927--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-elements ready!

Built with commit 73cf6f3

https://deploy-preview-3927--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-components-react ready!

Built with commit 73cf6f3

https://deploy-preview-3927--carbon-components-react.netlify.com

Copy link
Contributor

@jnm2377 jnm2377 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really awesome! 🎉

@dakahn
Copy link
Contributor

dakahn commented Sep 6, 2019

Great! Thank you so much for taking the initiative here. 🎉

});
}

function format(violations) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be down to call this like formatOutput?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure, feel free to use a commit suggestion block in the comment and I'll apply it for sure 👍

* LICENSE file in the root directory of this source tree.
*/

const toHaveNoViolations = require('./matchers/toHaveNoViolations');
Copy link
Contributor

@dakahn dakahn Sep 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what this is doing, but I don't know why we need to do this 😸. Totally down to my lack of knowledge here, but maybe a quick comment block talking about what we're doing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What specifically? In this case, we need to extend expect so that anywhere in our tests we can call expect(...).toHaveNoViolations similar to expect(...).toEqual(...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha! Exactly what I was asking. Thanks 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the file name "setup.." I just wanting to make sure I knew what was happening here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally fair, let me add a block 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

joshblack and others added 4 commits September 6, 2019 11:37
Co-Authored-By: DAK <40970507+dakahn@users.noreply.github.com>
Co-Authored-By: DAK <40970507+dakahn@users.noreply.github.com>
);

await expect(document).toHaveNoViolations();
mountNode.parentNode.removeChild(mountNode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this code run when the assertion fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! Let me pull it out into afterEach 👍

@joshblack joshblack merged commit 4c06756 into carbon-design-system:master Sep 9, 2019
@joshblack joshblack deleted the test/add-axe-testing branch September 9, 2019 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants