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

Automated axe testing #2569

Merged
merged 28 commits into from
Mar 5, 2020
Merged

Automated axe testing #2569

merged 28 commits into from
Mar 5, 2020

Conversation

myasonik
Copy link
Contributor

@myasonik myasonik commented Nov 27, 2019

Summary

Closes #2231 and introduces automated axe testing!

Currently only testing the guidelines so the capability can be merged. Adding more pages over time can become a ticket an a few pages can be added at a time when people have spare cycles. (For reference, there are currently 84 errors found when running it across all pages. This does not count color contrast rules which had to be disabled due to their quantity...)

Run npm run start-test-server-and-a11y-test to run the tests locally.

TODO

Checklist

- [ ] Checked in dark mode
- [ ] Checked in mobile
- [ ] Checked in IE11 and Firefox
- [ ] Props have proper autodocs
- [ ] Added documentation examples
- [ ] Added or updated jest tests
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Checked for accessibility including keyboard-only and screenreader modes
- [ ] A changelog entry exists and is marked appropriately

@thompsongl
Copy link
Contributor

On the Colors Guidelines there's a duplicate ID error in the range slider that I can't track down...

I'll take a look today

@thompsongl
Copy link
Contributor

^ #2588 has what I think is the right way to fix the duplicate id error, but check my math there.

@myasonik myasonik marked this pull request as ready for review December 5, 2019 19:20
@chandlerprall
Copy link
Contributor

Overall, these changes look good. I haven't reviewed a11y-testing.js or the readme changes in depth yet, but everything looks good functionally. A blocker for this moving forward is CI's failure when running

20:21:56 > @elastic/eui@17.0.0 test-a11y /app
20:21:56 > node ./scripts/a11y-testing
20:21:56 
20:21:57 (node:559) UnhandledPromiseRejectionWarning: Error: Failed to launch chrome!
20:21:57 /app/node_modules/puppeteer/.local-chromium/linux-706915/chrome-linux/chrome: error while loading shared libraries: libX11-xcb.so.1: cannot open shared object file: No such file or directory
20:21:57 
20:21:57 
20:21:57 TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md
20:21:57 
20:21:57     at onClose (/app/node_modules/puppeteer/lib/Launcher.js:348:14)
20:21:57     at Interface.helper.addEventListener (/app/node_modules/puppeteer/lib/Launcher.js:337:50)
20:21:57     at Interface.emit (events.js:203:15)
20:21:57     at Interface.close (readline.js:397:8)
20:21:57     at Socket.onend (readline.js:173:10)
20:21:57     at Socket.emit (events.js:203:15)
20:21:57     at endReadableNT (_stream_readable.js:1143:12)
20:21:57     at process._tickCallback (internal/process/next_tick.js:63:19)
20:21:57 (node:559) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
20:21:57 (node:559) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

which demonstrates both a failure to run puppeteer and that the error didn't fail the CI job, instead it crashed "silently".

@thompsongl
Copy link
Contributor

Just an update: I've been working on some Docker and script changes to get the necessary deps into EUI CI runs. Learning some things and getting closer to understanding what the limitations are. I'll continue to with infra to get this going.

@thompsongl thompsongl mentioned this pull request Jan 27, 2020
14 tasks
@thompsongl
Copy link
Contributor

jenkins test this

^ Last commit works in Docker locally, so I suspect some user/permission thing with the Jenkins setup

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

Going trigger a few more builds to see about stability.

The recent change is to run Chromium without a sandbox, which is generally only acceptable in cases like this where we fully own the content being crawled.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

I'm going to call this "stable enough" and do some clean up 🎉

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl
Copy link
Contributor

Ready for your review, @chandlerprall

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGreatTM, pulled & tested locally in both a passing & error states, with both the automated test server on port 9999 & against the local development server.

@thompsongl
Copy link
Contributor

thompsongl commented Mar 5, 2020

LGreatTM

😅

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2569/

@thompsongl thompsongl merged commit 68299f4 into elastic:master Mar 5, 2020
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.

Automated a11y testing
4 participants