Run aXe on a few pages during CI #344

toolness · 2017-07-11T12:27:24Z

Fixes #326.

This runs axe-core on a few pages of the standards during CI and npm test runs. In future PRs we can add support for running it on more pages.

A successful run looks something like this:

It requires that Chrome/Chromium v59 or higher be installed on the host system. I decided on headless Chrome because:

It's maintained by the Google Chrome team, so it's likely to be very well-maintained going forward. Possibly even moreso than Phantom; I vaguely recall seeing a Tweet from Ariya Hidayat a while back saying that Phantom was going to be end-of-life'd since Chromium is headless is officially supported now, but I could be totally wrong on that.
I haven't done any formal measurements, but headless Chrome seems really fast compared to Phantom.
In the vast majority of cases, it's just one less thing for developers to install--every developer I know has Chrome installed on their system, and our use of chrome-launcher ensures that it will be found, regardless of host OS.
I noticed we have a (kind of old) WIP PR to add support for Google Lighthouse in [WIP] Testing lightouse #210, so this PR might make continuing work on that easier too,
Chrome is a real browser, unlike PhantomJS, so it's likely that any errors can be reproduced and debugged using Chrome running in non-headless (headed?) mode.

To do:

Ensure this works on CircleCI.
Ensure we're running aXe on the actual front page and not e.g. a "network error" page.
Ensure that docker-uswds can run this too.
Update README to indicate that developers should have Chrome/Chromium installed on their systems for tests to run properly.
Modify crawl.js to use static-server.js to prevent duplication of code.

toolness · 2017-07-11T16:02:00Z

Hmm, so this is a fair bit of code, as the Chrome DevTools Protocol is fairly low-level. There might be a higher-level wrapper available that could reduce the size of our code, though I'm not sure if it'd necessarily meet our particular needs or not. Even axe-core's own PhantomJS wrapper didn't meet all the needs for the a11y-metrics dashboard so I had to write my own.

msecret · 2017-07-12T19:14:30Z

config/static-server.js

+}
+
+module.exports = () => {
+  return new Promise((resolve, reject) => {


Does this need a rejection handler somewhere?

Ah good CATCH, will add!

msecret · 2017-07-12T19:18:08Z

config/run-axe.js

+  const chromeHost = chrome.host || 'localhost';
+  console.log(`Static file server is listening at ${server.url}.`);
+  console.log(`Chrome is debuggable on http://${chromeHost}:${chrome.port}.`);
+  console.log(`Running aXe on:`);


Was there more text that was supposed to go here?

Nah, it's just the heading text that appears before each individual page that's visited. The final output looks like this:

msecret · 2017-07-12T19:18:40Z

config/run-axe.js

+  console.log(`Running aXe on:`);
+
+  CDP({
+    host: chrome.host,


Above, it sounds like chrome.host may not exist. Should this be chromeHost? Why wouldn't chrome.host not exist anyway?

Oh, it will just be undefined if it doesn't exit, which CDP just then defaults to localhost--but I agree, I think it'd be less confusing / more explicit if we just passed chromeHost here.

I was hoping that the API for chrome would set host to whatever the ultimate "resolved" value was, whether that was localhost (in the case where Chrome is launched locally) or something explicit (in the case where we connect to a remote Chrome in e.g. a different Docker container). However, that wasn't the case--in the case where Chrome is launched locally, host isn't set at all, so I had to manually default it to localhost via that chromeHost variable. It's a bit annoying because the APIs that connect all these different Chrome packages isn't quite as consistent as I'd hoped, and this is a workaround for that.

msecret · 2017-07-12T19:19:49Z

config/run-axe.js

+      } else {
+        const page = pagesLeft.pop();
+        const url = `${server.url}${page}`;
+        process.stdout.write(`  ${page} `);


What's this doing? Why isn't it a simple console.log?

Oh, I wanted it to look cool like all the fancy test runners!

msecret · 2017-07-12T19:23:22Z

config/run-axe.js

+      Page.enable(),
+      Network.enable(),
+    ]).then(() => {
+      Network.responseReceived(({response}) => {


Is this when any page resource is received? Or just the main HTML document?

I believe it's when any page resource is received. So this effectively overlaps a bit with our crawler from #321. It was easier to do it that way than it was to check the URL and see if it matched the page we were requesting, but I could add such a conditional if you think it'd be better that way.

Seems OK to have this extra check

Hmm, so I thought about it a bit, and realized that maybe it's actually good that it's counting any sub-resource failure as a 404 too. For example, if it's 404'ing on CSS or JS, then that changes the accessibility profile of the page--calculated color contrast ratios may be incorrect, or certain ARIA attributes may not be present on the evaluated page.

Another thing is that the crawler from #321 isn't actually 100% accurate--the library we're using uses regular expressions to "scrape" resources for URLs, so it's actually possible that it might miss sub-resources that are noticed by actually loading a page in Chrome. So it might still be good to hard fail on sub-resource failures for the sake of redundancy, too.

What do you think? At the very least, I think we should log warnings on sub-resource fails.

Yea I agree that it's good to consider sub-resource 404s as failures.

msecret · 2017-07-12T19:24:07Z

config/run-axe.js

+      });
+      Page.loadEventFired(() => {
+        Runtime.evaluate({
+          expression: AXE_JS + ';(' + runAxe.toString() + ')()',


Is this possible to write using a template string so it's easier to parse?

msecret · 2017-07-12T19:25:06Z

config/run-axe.js

+              errorFound = true;
+            }
+          } else {
+            console.log('Unexpected result from CDP!');


What's CDP?

Oh, it stands for Chrome DevTools Protocol... I'll expand it to just say that!

toolness · 2017-07-12T22:06:43Z

Ok, I think I addressed all your issues, though I had second thoughts about ignoring sub-resource loading failures, which I commented on above.

toolness · 2017-07-12T23:14:31Z

Hooray, thanks! 💃

toolness added 8 commits July 11, 2017 08:24

Add 'npm run axe' command.

6d736a1

Install Chrome on circle, hopefully.

c1e786b

Add support for REMOTE_CHROME_URL.

b7688db

Update README.md to include info on Chrome.

5e001b2

Raise errors on network failure, HTTP errors.

4731a04

Remove stale TODO.

dc2a867

Don't allow Chrome instances to go unkilled.

36070be

Use strict mode.

dcd5070

toolness changed the title ~~[WIP] Run aXe during CI~~ Run aXe on front page during CI Jul 11, 2017

crawl.js now uses static-server.js.

24c93cc

toolness requested a review from msecret July 11, 2017 16:02

Run aXe on multiple pages.

c863e0d

toolness changed the title ~~Run aXe on front page during CI~~ Run aXe on a few pages during CI Jul 11, 2017

Add colors.

c1117d0

msecret reviewed Jul 12, 2017

View reviewed changes

Fix issues with run-axe.js.

3c0c42c

msecret approved these changes Jul 12, 2017

View reviewed changes

toolness merged commit 496cab9 into develop Jul 12, 2017

toolness deleted the axe branch July 12, 2017 23:14

toolness mentioned this pull request Jul 25, 2017

Run aXe on (almost) all fractal components. uswds/uswds#2055

Merged

3 tasks

toolness mentioned this pull request Sep 6, 2017

Crawl site for 404s and run aXe during CI 18F/uswds-data#1

Merged

toolness mentioned this pull request Jun 8, 2018

Document how to debug headless Chrome / aXe errors #569

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run aXe on a few pages during CI #344

Run aXe on a few pages during CI #344

toolness commented Jul 11, 2017 •

edited

Loading

toolness commented Jul 11, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

msecret Jul 12, 2017

msecret Jul 12, 2017

msecret Jul 12, 2017

toolness Jul 12, 2017

toolness commented Jul 12, 2017

toolness commented Jul 12, 2017

Run aXe on a few pages during CI #344

Run aXe on a few pages during CI #344

Conversation

toolness commented Jul 11, 2017 • edited Loading

toolness commented Jul 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toolness commented Jul 12, 2017

toolness commented Jul 12, 2017

toolness commented Jul 11, 2017 •

edited

Loading