search

emulsify-ds / compound

Compound is the default component collection for Emulsify
https://emulsify-ds.github.io/compound/
GNU General Public License v2.0
12 stars 12 forks source link

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

Closed calebtr-metro closed 4 months ago

calebtr-metro commented 2 years ago

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

Closing issues

Closes #42

calebtr-metro commented 2 years ago

Questions I have:

ModulesUnraveled commented 2 years ago

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 commented 2 years ago

And to answer your questions:

calebtr-metro commented 2 years ago

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:

sync-by-unito[bot] commented 4 months ago

➤ 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