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.

Sign up for GitHub

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

Jump to bottom

test(a11y): automates generation of component list for a11y testing #43

Closed
wants to merge 2 commits into from
Closed

test(a11y): automates generation of component list for a11y testing #43

wants to merge 2 commits into from

Conversation

calebtr-metro
Copy link
Collaborator

Summary

Implements automated accessibility testing from emulsify-drupal project in compound

This PR fixes/implements the following bugs/features

Explain the motivation for making this change. What existing problem does the pull request solve?

Components were moved out of emulsify-ds into this project. Accessibility testing for those components should also happen here.

Manually maintaining a list of components to test leads to coverage gaps. The existing tooling in Emulsify-ds does not report errors if a listed component does not exist, so components that are renamed are inadvertently removed from testing.

Documentation Update (required)

Accessibility testing is covered at https://docs.emulsify.info/emulsify-drupal/emulsify-drupal/advanced-usage/accessibility-testing but it could be moved to the Compound section.

How to review this PR

  • npm install
  • npm run a11y

Closing issues

Closes #42

@calebtr-metro
Copy link
Collaborator Author

Questions I have:

  • what's up with package-lock.json and the version? I'm not sure where that is coming from or what I should do about it.

  • The emulsify-drupal project has more automatic testing. Does compound drop jest entirely? I think this means we don't have to move the twatch and coverage scripts as well, though the test script could run the a11y tests.

  • This pins the pa11y project to 5.3, calling puppeteer 1.9; the current version of pa11y, 6.2.3 uses puppeteer 9 but it was mad about a CORS error. I downgraded to 5.3 because that was what was in emulsify-drupal and working. How to approach this?

@ModulesUnraveled ModulesUnraveled self-requested a review July 17, 2022 17:13
@ModulesUnraveled
Copy link
Contributor

I'm getting various errors when I try to run the a11y script.

Sometimes it's this:

(node:80642) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
(node:80642) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:80642) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGTERM listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:80642) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGHUP listeners added to [process]. Use emitter.setMaxListeners() to increase limit
/Users/brian/Websites/GitProjects/emulsify/compound/node_modules/puppeteer/lib/Launcher.js:349
      reject(new Error([
             ^

Error: Failed to launch chrome!
Received signal 11 SEGV_MAPERR 000000000000
 [0x000116e0bc99]
 [0x000116d29183]
 [0x000116e0bbb1]
 [0x7ff81af66dfd]
 [0x000000000000]
 [0x7ff81af4cf70]
[end of stack trace]


TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md

    at onClose (/Users/brian/Websites/GitProjects/emulsify/compound/node_modules/puppeteer/lib/Launcher.js:349:14)
    at Interface.<anonymous> (/Users/brian/Websites/GitProjects/emulsify/compound/node_modules/puppeteer/lib/Launcher.js:338:50)
    at Interface.emit (node:events:402:35)
    at Interface.close (node:readline:586:8)
    at Socket.onend (node:readline:277:10)
    at Socket.emit (node:events:402:35)
    at endReadableNT (node:internal/streams/readable:1343:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)

And other times this:

info => Preview built (5.47 s)
WARN asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
WARN This can impact web performance.
WARN Assets: 
WARN   main.5bf212b7.iframe.bundle.js (392 KiB)
WARN   vendors~main.6af87b60.iframe.bundle.js (3.02 MiB)
WARN entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
WARN Entrypoints:
WARN   main (3.4 MiB)
WARN       runtime~main.785ce5d0.iframe.bundle.js
WARN       vendors~main.6af87b60.iframe.bundle.js
WARN       main.5bf212b7.iframe.bundle.js
WARN 
info => Output directory: /Users/brian/Websites/GitProjects/emulsify/compound/.out
info connecting to: http://localhost:54888/iframe.html
Error: Navigation failed because browser has disconnected!
    at CDPSession.<anonymous> (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/LifecycleWatcher.js:46:107)
    at CDPSession.emit (node:events:390:28)
    at CDPSession._onClosed (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/Connection.js:215:10)
    at Connection._onClose (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/Connection.js:138:15)
    at WebSocket.<anonymous> (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/WebSocketTransport.js:48:22)
    at WebSocket.onClose (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/ws/lib/event-target.js:124:16)
    at WebSocket.emit (node:events:390:28)
    at WebSocket.emitClose (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/ws/lib/websocket.js:191:10)
    at Socket.socketOnClose (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/ws/lib/websocket.js:850:15)
    at Socket.emit (node:events:390:28)
  -- ASYNC --
    at Frame.<anonymous> (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/helper.js:111:15)
    at Page.goto (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/Page.js:672:49)
    at Page.<anonymous> (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/puppeteer-core/lib/helper.js:112:23)
    at read (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/@storybook/cli/dist/cjs/extract.js:27:14)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async extract (/Users/brian/.npm/_npx/d28cfb4b54958d89/node_modules/@storybook/cli/dist/cjs/extract.js:90:18)

Can you verify this is still working for you?

@ModulesUnraveled
Copy link
Contributor

And to answer your questions:

  • The version shouldn't be a problem. the release management process will handle the versions automatically. It should be 0.0.0-develop locally, so I think it's okay.
  • Correct, we dropped jest because we weren't actually using it. A previous developer had set it up, but it wasn't implemented thoroughly, nor understood by many of us. So when we created Compound we left it out and have a ticket to figure out what we want to use, but it hasn't been a priority yet.
  • I think it would be preferable to update to the latest version. Maybe that would fix the issues I'm seeing? If you still get the error, post it here, and maybe we can figure it out together.

@calebtr-metro
Copy link
Collaborator Author

I get the asset size warning; if it is related to this branch it might be because when I updated pa11y the storybook version went to 6.5.9. The good news is that we can do something about bundle size as of 6.4. (https://storybook.js.org/blog/storybook-on-demand-architecture/). But that may be a separate ticket?

The new pa11y was complaining about uncaught rejected promises. To get it to work, I refactored lintReportAndExit to use a more vanilla promise statement instead of running it through Ramda. If it would be better to stick with Ramda, it is a little opaque to me but I can start working through some tutorials.

I'll leave the a11y.test.js file (a jest test) until that testing question is resolved. For what it's worth, we're pretty happy with a combination of Chromatic for visual tests and Cypress for functional tests. The newest version of Cypress has a components option I haven't explored yet. Cypress can run a11y/axe tests also, but so far the script here is a little easier to set up.

I excluded two additional axe rules from reporting errors, bypass and frame-tested. bypass is a page-level rule and doesn't apply to most components. It would be good to include it for page templates though hm. frame-tested came up on the video embed. If a component were loading a local iframe, we'd want to test it.

Rule-ignoring could also be refactored - pa11y supports this directly now, but the script here was written to run all tests and filter out the ones from the a11y.config.js ignore list. There's no reason you can't pass the ignore parameter to pa11y in your local config of course.

So this is an improvement, but more could be done:

  • use pa11y API for ignoring rules
  • only ignore page-level accessibility rules on page components
  • exclude iframe testing rule for the atoms-videos--full component only

@ModulesUnraveled ModulesUnraveled added 👍 Ready for Review Work is ready for review. and removed 👁 Review in progress Under review. labels Jul 26, 2022
@sync-by-unito Sync by Unito
Copy link

sync-by-unito bot commented Mar 26, 2024

➤ Callin Mullaney commented:

Closing as A11y testing is part of Emulsify-Core integration and no longer needs it's own integration for Compound - https://github.com/emulsify-ds/emulsify-core/blob/main/config/a11y.config.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ModulesUnraveled ModulesUnraveled Awaiting requested review from ModulesUnraveled

Assignees
No one assigned
Labels
👍 Ready for Review Work is ready for review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Automate generation of components list for accessibility testing
3 participants
@calebtr-metro @ModulesUnraveled @callinmullaney