[Enterprise Search] Set up cypress-axe tests #108465

cee-chen · 2021-08-12T22:56:34Z

Summary

Follow up to #108309

What this PR does:

Installs and sets up the cypress-axe dev dependency
DRY's out some axe-core configs that the current FTR a11y tests use to re-use in Cypress 0a51689 (cc @myasonik for review on this)
Adds checkA11y() that runs axe against our current placeholder/hello-world E2E tests

What this PR does NOT do:

Set up more a full set of checkA11y() tests for each product's views/routes ~~(hoping to do this in a follow-up PR)~~
- In light of the Cypress PR being reverted, I likely won't have time to expand on our E2E tests. Hoping this setup helps however and y'all continue to use the established checkA11y() helper for future tests.

As always, I recommend following along by commit.

Screencaps

In open mode, you should be able to inspect and see that 0 accessibility violations were detected:

Screencap showing how to debug cypress-axe accessibility failures:

QA

Setup
- Have Elasticsearch running on localhost:9200
- Have Enterprise Search running on localhost:3002
- In kibana.dev.yml, set csp.strict: false and csp.warnLegacyBrowsers: false
- Have Kibana running with --no-base-path and on localhost:5601
- cd to x-pack/plugins/enterprise_search/
Confirm sh cypress.sh run overview passes
Confirm sh cypress.sh run as passes
Confirm sh cypress.sh run ws passes

Checklist

Unit or functional tests were updated or added to match the most common scenarios

cee-chen

Misc comments. Excited to get this set up so we can add more axe checks to our app and potentially close out #79359!

cee-chen · 2021-08-12T22:59:25Z

test/accessibility/services/a11y/constants.ts

+
+import { ReporterVersion } from 'axe-core';
+
+export const AXE_CONFIG = {


@myasonik Let me know if you're cool with this setup (constants.ts file and all capitalized CONSTANTS vars)! If not I can definitely rename to anything that you prefer!

test/accessibility/services/a11y/constants.ts

...earch/public/applications/enterprise_search/components/product_selector/product_selector.tsx

x-pack/plugins/enterprise_search/public/applications/enterprise_search/index.scss

cee-chen · 2021-08-13T00:18:24Z

test/accessibility/services/a11y/analyze_with_axe.js

-            },
-          ],
-        });
+        window.axe.configure(config);


NB: the new AXE_CONFIG const has to get passed as a new config arg instead of being imported in-file, because of the way analyzeWithAxe gets called during browser runtime by Kibana's FTR tests. Otherwise you just get a bunch of undefined errors

myasonik

Awesome to see! 🚀

byronhulcher · 2021-08-13T13:26:44Z

In kibana.dev.yml, set csp.strict: false and csp.warnLegacyBrowsers: false
Have Kibana running with --no-base-path and on localhost:5601

Can we document this somewhere in the codebase? Maybe a x-pack/plugins/enterprise_search/public/applications/app_search/README.md file (we could also include some of the non-internal guidance from the Getting Started guide like Running Kibana, Where do I write code?, How do I write code?, Relevant guides)

byronhulcher · 2021-08-13T13:50:18Z

Hey constance, could you make a branch for me with an axe error? I'm having trouble generating one, I tried messing around with bad h* tags which is what has gotten me in trouble the most but my tests always pass with 0 accessibility failures. Just want to validate these appear in the CI as failures.

cee-chen · 2021-08-13T14:50:51Z

@byronhulcher Interesting, it's possible that the new axe config settings we imported from the Kibana a11y tests are ignoring those failures. I'll play around with it and see if we need to override anything.

Also as a heads up, #108309 was reverted due to causing Typescript build issues in master with our Cypress config, so I'm moving this PR back into draft until then. I'll be speaking to Spencer/the KibanaOps team to figure out our TS issues! Thanks Michail for the approval on this before your last day; with your review I feel confident about letting this PR sit for a bit and then merging later once Typescript is no longer causing issues.

cee-chen · 2021-08-13T15:09:10Z

Sorry, just saw your 1st comment also Byron (wish GitHub would let us thread non-code comments):

Can we document this somewhere in the codebase?

It's documented in our Kibana plugin README. Documentation was added in the previous Cypress setup PR:

https://github.com/constancecchen/kibana/blob/54978c8d5685bada2911d4c1911afaa22047a4d3/x-pack/plugins/enterprise_search/README.md#cypress-tests

cee-chen · 2021-08-13T18:31:14Z

@byronhulcher OK, I dug into validating heading levels and it turns out the settings we were inheriting from the base Kibana setup did indeed have those violations turned off (they're tagged under best-practices). I override them in our own configuration this PR (a982179) and checked with Michail just now - they weren't intentionally turned off, but were skipped due to time reasons on their end (not having the bandwidth to fix every best-practices error across Kibana at the time). He's super +1 to us adding it to our codebase.

With the extra config above, I added an extra branch with an example error - feel free to check it out! cypress-axe-error, commit cee-chen@52b8b2e

@see https://github.com/component-driven/cypress-axe

+ fix string union type error + remove unnecessary tsconfig exclude

- Mostly just re-exporting the shared command and checking for failures, I only ran this after the shared axe config settings and found no failures

- notably heading level issues (thanks Byron for catching this!) - however I now also need to set an ignore on a duplicate landmark violation caused by the global header (not sure why it's showing up - shouldn't it be out of context? bah) - remove option to pass args into checkA11y - I figure it's not super likely we'll need to override axe settings per-page (vs not running it), but we can pass it custom configs or args later if needed

cee-chen · 2021-08-16T20:50:59Z

OK, with #108560 merged in this should now be ready to review!

@byronhulcher, I know it's ON week, but this should hopefully be a relatively small PR and I was hoping to get your eyes on it (since your comments have already helped me improve our axe config settings!)

Similarly WS folks, would super appreciate a review whenever you have time so I can wrap up my last Enterprise Search PR!

scottybollinger

LGTM. Thanks!

cee-chen · 2021-08-23T14:40:55Z

@elasticmachine merge upstream

cee-chen · 2021-08-23T15:05:40Z

@byronhulcher @JasonStoltz 👋 Would super appreciate a quick review on this PR from either of y'all this week!

JasonStoltz · 2021-08-23T16:17:10Z

@constancecchen I will take a look

kibanamachine · 2021-08-23T16:55:27Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #145928 succeeded 722f827
💚 Build #145343 succeeded a982179bc65c0078cd58142b8a5fe3cdee01d112
💚 Build #145038 succeeded ba987b7f76c67f2a25b0b1812bfff1d90d948a07
💔 Build #145031 failed 32908b56b47703879a4c4704d991542db1525bbd

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

JasonStoltz

Great work Constance!

cee-chen · 2021-08-24T19:05:35Z

Thanks y'all!! Enjoy! 🎉

* Set up cypress-axe @see https://github.com/component-driven/cypress-axe * DRY out Kibana axe rules into constants that Cypress can use * Create shared & configured checkA11y command + fix string union type error + remove unnecessary tsconfig exclude * Add Overview plugin a11y tests * Add AS & WS placeholder a11y checks - Mostly just re-exporting the shared command and checking for failures, I only ran this after the shared axe config settings and found no failures * Configure our axe settings further to catch best practices - notably heading level issues (thanks Byron for catching this!) - however I now also need to set an ignore on a duplicate landmark violation caused by the global header (not sure why it's showing up - shouldn't it be out of context? bah) - remove option to pass args into checkA11y - I figure it's not super likely we'll need to override axe settings per-page (vs not running it), but we can pass it custom configs or args later if needed Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-08-24T19:08:35Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* Set up cypress-axe @see https://github.com/component-driven/cypress-axe * DRY out Kibana axe rules into constants that Cypress can use * Create shared & configured checkA11y command + fix string union type error + remove unnecessary tsconfig exclude * Add Overview plugin a11y tests * Add AS & WS placeholder a11y checks - Mostly just re-exporting the shared command and checking for failures, I only ran this after the shared axe config settings and found no failures * Configure our axe settings further to catch best practices - notably heading level issues (thanks Byron for catching this!) - however I now also need to set an ignore on a duplicate landmark violation caused by the global header (not sure why it's showing up - shouldn't it be out of context? bah) - remove option to pass args into checkA11y - I figure it's not super likely we'll need to override axe settings per-page (vs not running it), but we can pass it custom configs or args later if needed Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Constance <[email protected]>

cee-chen added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v7.15.0 labels Aug 12, 2021

cee-chen requested review from myasonik and a team August 12, 2021 22:56

cee-chen requested a review from a team as a code owner August 12, 2021 22:56

cee-chen requested review from a team August 12, 2021 22:56

cee-chen force-pushed the cypress-axe branch from 0242ce9 to 32908b5 Compare August 12, 2021 22:58

cee-chen commented Aug 12, 2021

View reviewed changes

cee-chen force-pushed the cypress-axe branch from 32908b5 to ba987b7 Compare August 13, 2021 00:15

cee-chen commented Aug 13, 2021

View reviewed changes

myasonik approved these changes Aug 13, 2021

View reviewed changes

cee-chen marked this pull request as draft August 13, 2021 14:51

cee-chen force-pushed the cypress-axe branch from ba987b7 to a982179 Compare August 13, 2021 18:14

cee-chen removed the request for review from a team August 13, 2021 18:26

cee-chen added 6 commits August 16, 2021 13:48

Set up cypress-axe

faff2b9

@see https://github.com/component-driven/cypress-axe

DRY out Kibana axe rules into constants that Cypress can use

acf5d5f

Create shared & configured checkA11y command

dcb7355

+ fix string union type error + remove unnecessary tsconfig exclude

Add Overview plugin a11y tests

450f74f

Add AS & WS placeholder a11y checks

1952a1c

- Mostly just re-exporting the shared command and checking for failures, I only ran this after the shared axe config settings and found no failures

cee-chen force-pushed the cypress-axe branch from a982179 to 722f827 Compare August 16, 2021 20:48

cee-chen marked this pull request as ready for review August 16, 2021 20:48

cee-chen mentioned this pull request Aug 16, 2021

Enterprise search needs a11y tests #79359

Open

scottybollinger approved these changes Aug 16, 2021

View reviewed changes

mistic added v7.16.0 and removed v7.15.0 labels Aug 18, 2021

Merge branch 'master' into cypress-axe

847e9ab

JasonStoltz approved these changes Aug 24, 2021

View reviewed changes

cee-chen merged commit 8dfdead into elastic:master Aug 24, 2021

cee-chen deleted the cypress-axe branch August 24, 2021 19:05

kibanamachine mentioned this pull request Aug 24, 2021

[7.x] [Enterprise Search] Set up cypress-axe tests (#108465) #109920

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enterprise Search] Set up cypress-axe tests #108465

[Enterprise Search] Set up cypress-axe tests #108465

cee-chen commented Aug 12, 2021 •

edited by JasonStoltz

Loading

cee-chen left a comment

cee-chen Aug 12, 2021

cee-chen Aug 13, 2021 •

edited

Loading

myasonik left a comment

byronhulcher commented Aug 13, 2021

byronhulcher commented Aug 13, 2021 •

edited

Loading

cee-chen commented Aug 13, 2021 •

edited

Loading

cee-chen commented Aug 13, 2021 •

edited

Loading

cee-chen commented Aug 13, 2021

cee-chen commented Aug 16, 2021

scottybollinger left a comment

cee-chen commented Aug 23, 2021

cee-chen commented Aug 23, 2021

JasonStoltz commented Aug 23, 2021

kibanamachine commented Aug 23, 2021

JasonStoltz left a comment

cee-chen commented Aug 24, 2021

kibanamachine commented Aug 24, 2021


		import { ReporterVersion } from 'axe-core';

		export const AXE_CONFIG = {

[Enterprise Search] Set up cypress-axe tests #108465

[Enterprise Search] Set up cypress-axe tests #108465

Conversation

cee-chen commented Aug 12, 2021 • edited by JasonStoltz Loading

Summary

Screencaps

QA

Checklist

cee-chen left a comment

Choose a reason for hiding this comment

cee-chen Aug 12, 2021

Choose a reason for hiding this comment

cee-chen Aug 13, 2021 • edited Loading

Choose a reason for hiding this comment

myasonik left a comment

Choose a reason for hiding this comment

byronhulcher commented Aug 13, 2021

byronhulcher commented Aug 13, 2021 • edited Loading

cee-chen commented Aug 13, 2021 • edited Loading

cee-chen commented Aug 13, 2021 • edited Loading

cee-chen commented Aug 13, 2021

cee-chen commented Aug 16, 2021

scottybollinger left a comment

Choose a reason for hiding this comment

cee-chen commented Aug 23, 2021

cee-chen commented Aug 23, 2021

JasonStoltz commented Aug 23, 2021

kibanamachine commented Aug 23, 2021

💚 Build Succeeded

Metrics [docs]

History

JasonStoltz left a comment

Choose a reason for hiding this comment

cee-chen commented Aug 24, 2021

kibanamachine commented Aug 24, 2021

💚 Backport successful

cee-chen commented Aug 12, 2021 •

edited by JasonStoltz

Loading

cee-chen Aug 13, 2021 •

edited

Loading

byronhulcher commented Aug 13, 2021 •

edited

Loading

cee-chen commented Aug 13, 2021 •

edited

Loading

cee-chen commented Aug 13, 2021 •

edited

Loading