[INFRA] Add Cypress a11y tests to Buildkite PR test job #7354
Conversation
* Fixed keyboard a11y for Card, Header, KeyPadMenu. * Fixed keyboard a11y for BasicTable and InMemoryTable. * Fixed keyboard a11y for Expression. * Fixed keyboard a11y for Image. * Fixed keyboard a11y for NotificationEvent, ResizableContainer, SearchBar. * Fixed keyboard a11y for ErrorBoundary, FocusTrap, Range.
|
@tkajtoch do you mind reviewing this PR? 🙏 thank you! |
|
Preview staging links for this PR:
|
💚 Build Succeeded
History
cc @1Copenut |
| && NODE_OPTIONS="--max-old-space-size=2048" npm run test-cypress-a11y\'', | ||
| && yarn run test-cypress-a11y \ | ||
| --node-options=--max_old_space_size=2048 \ | ||
| --skip-css '", // Skipping CSS because compiling has a tendency to hang on Apple Silicon |
There was a problem hiding this comment.
(not a blocking comment, just thinking out loud) Interesting - a11y tests shouldn't need CSS, but the other Cypress tests definitely do. Why would these sets of Cypress tests have issues but the others don't? 🤔
There was a problem hiding this comment.
This is a great question and I wish I had a better answer. I had a lot of trouble getting these to run in a local Docker container without ignoring CSS. This started after I moved to an Apple Silicon machine, so my best guess is the compile step is either very slow or running out of memory on Rosetta. Was not an issue on an older Intel box.
I'm happy to remove the --skip-css declaration if that feels warranted. It was a coin-flip call. I came down on the side of better to be able to run the tests consistently in a Docker environment that matches our CI more closely than runs on MacOS.
There was a problem hiding this comment.
Nah it totally makes sense to do if nothing else because the a11y tests don't need them. Plus eventually when we're fully on Emotion, the command goes away anyway.
There was a problem hiding this comment.
LGTM if CI is passing! Nice work on this Trevor.
(below is not a change request, just a note to keep in mind for the future)
22 minutes is on the higher end of our CI runs - this isn't an issue now, but may be in the future as we scale/add more a11y tests. We may want to reconsider/rethink our approach to these and make sure we have specific goals in place for our a11y tests, or potentially run these on a cron instead of per PR 🤷
Much agreed. There's Buildkite documentation about running jobs on a schedule, so it should be an easy port if/when the tests start taking too much time. |
Summary
PR adds our Cypress a11y test suite to the Buildkite
testjob now that tests are running in parallel. This took additional work because something changed in Chrome after release 118. The change caused a number of tests using Cypress Real Events to start failing. This was not because of the library, but most likely how I was using the customcy.realMount()and creating a single pixel to "click into" the Cypress testing iframe. Tests were refactored to usecy.get('ELEM').focus()then proceeded to use CRE methods. All tests were verified locally in Docker using EUI's current container.PR closes #7306.
QA
Remove or strikethrough items that do not apply to your PR.
General checklist
Local test run