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.

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

[a11y] add initial accessibility functional tests #43584

Merged
merged 34 commits into from
Nov 7, 2019

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Aug 20, 2019

Fixes #43016

In an effort to start doing automated a11y testing in Kibana, this implements to FTR configs, one for OSS and one for X-Pack that both start Kibana and test a single view against the wcag2a and wcag2aa rule sets. This is mostly for the purpose of testing reporting/rules, and see how we want to proceed.

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@timroes
Copy link
Contributor

timroes commented Aug 27, 2019

@spalger Does it make sense having those as a completely separate config? Wouldn’t it be better if we could use that a11y testing function anywhere in any other functional test. That way we wouldn’t need to setup different test paths to get to a specific page just to a11y test it, but just use the regular e.g. discover test suite that brings us anyway to e.g. the context view and then just quickly a11y test this with one call?

@elasticmachine

This comment has been minimized.

@myasonik
Copy link
Contributor

jenkins test this please


checks-reporter-with-killswitch "Kibana accessibility tests" \
node scripts/functional_tests \
--debug --bail \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove --bail? We can get all the failures at once otherwise.

CC @dmlemeshko

@myasonik
Copy link
Contributor

I can't push into your branch so here's a patch of the changes to fix the Discover changes. (Change .txt to .patch to apply)
mypatch.txt

Not super sure what's going on with the login page...

CC @bhavyarm

@elasticmachine

This comment has been minimized.

@spalger
Copy link
Contributor Author

spalger commented Aug 30, 2019

@timroes

Does it make sense having those as a completely separate config? Wouldn’t it be better if we could use that a11y testing function anywhere in any other functional test. That way we wouldn’t need to setup different test paths to get to a specific page just to a11y test it, but just use the regular e.g. discover test suite that brings us anyway to e.g. the context view and then just quickly a11y test this with one call?

I'm not opposed to exposing the a11y service to other tests, I just think it would be nice to have the tests isolated for now and maybe merge them later. Ideally most of the setup/navigation logic will be shared with the functional config anyway so that we can merge them later if we like pretty easily. Or maybe we just always have accessibility tests sprinkled about as well as a clump of a11y tests isolated just for easy of development... 🤷‍♂ either works for me.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

…y-functional-tests

# Conflicts:
#	.ci/jobs.yml
#	.ci/run.sh
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@bhavyarm
Copy link
Contributor

bhavyarm commented Nov 6, 2019

Retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

1 similar comment
@bhavyarm
Copy link
Contributor

bhavyarm commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 6, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 7, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@myasonik
Copy link
Contributor

myasonik commented Nov 7, 2019

@spalger That's 10 green builds!

@spalger
Copy link
Contributor Author

spalger commented Nov 7, 2019

Huzzah!

@spalger spalger merged commit 0010447 into elastic:master Nov 7, 2019
spalger pushed a commit to spalger/kibana that referenced this pull request Nov 7, 2019
* [a11y] add initial accessibility functional tests

* add accessibility jobs

* fix config path

* remove percy setup from scripts

* disable color-contrast rule

* apply changes from @myasonik

* define aria-controls/owns props even when suggestions aren't visible

* [ftr/a11y] only throw error when there are errors

* adding tests for management page

* add a11y test for management page

* adding ignore rules' to a11y

* accessibility test for kibana home

* 7 passing tests, 0 failures

* jest snapshot update

* support a11y test in pipeline job

* update a11y test script for pipelines

* use oss compatible ci setup

* fix exclude syntax

* add default exclusion syntax
@spalger
Copy link
Contributor Author

spalger commented Nov 20, 2019

7.x/7.6: 9a6724c

@spalger spalger deleted the implement/a11y-functional-tests branch November 20, 2019 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team test_infra v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prototype of aXe into FTR and percy for accessibility
5 participants