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

Adding accessibility tests #117

Merged
merged 8 commits into from
Oct 30, 2023
Merged

Conversation

dmanners
Copy link
Contributor

Overview

A MR that's aim is to provide the option for accessibility tests using cypress-axe. #110

Pipeline

Currently, these tests are optional and as far as we can see do not run in the pipeline. Is this something that should be considered for the project or is leaving them optional fine for now?

Order History Test Improvement

In our project we have 2 users setup that are imported in pipeline. 1 user has orders and 1 user does not have orders thus allowing us to test the order history page with and without orders easily without having to create an order via the tests itself and having a consistent test data state. This would be one suggesting for improvement we see here and also with the account tests in general to consider how items like orders are created for test cases. At the moment we have left this test exactly the same as the main account order history test.

Address cleanup improvements

Again in our project we have our database cleaned for each test run, so we know that state at the beginning of each test. Here with the address test we are not 100% sure what the user's address state is in the spec. In the end I think we should test the address book with no addresses, with only default addresses and with extra addresses but for these tests we have stuck to the tests like they are in the normal test specs. At the moment we have left this test exactly the same as the main account order history test.

Splitting account tests

As the cypress-axe assertion will by default be treated as an error when you add multiple tests into one spec the test spec will exit on first fail. It is possible to ignore this with a flag, for example see the checkout step tests, but the tests will then be marked as successful. We feel if the accessibility tests fail then they should really fail rather than fail silently. For this reason we have split all tests out into individual test specs so you get a better overview of the real errors.

Tests allow failure

There are two cases when we have set the "allow failure" flag as true. Firstly in the checkout tests when we validate step 1 of the checkout and then process to step 2. Since there are errors in step 1 cypress by default would stop at these and not continue to step 2. The second time is during any account test with an after section. If there is a failure in the test itself then the after section, for example address cleanup, would not happen. So in this case we need to mark the test as allow failure to run the test clean up step.

Hyvä tests

In our internal project we only have the single theme based on luma and as such to keep things simple have only provided the accessibility tests for luma, though it should be easily possible to create a second accessibility spec for hyvä if and when desired.

dmanners and others added 8 commits September 28, 2023 10:20
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
…entos#110)

Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
Co-authored-by: Peter Höcherl <peter.hoecherl@deutschebahn.com>
Co-authored-by: Odilo Oehmichen <odilo.oehmichen@deutschebahn.com>
@peterjaap
Copy link
Contributor

That's amazing work!

Since we don't use Luma anymore, I'll take your word on this that it works. You've gained my trust over the years :)

Thanks!

@peterjaap peterjaap merged commit 0dce888 into elgentos:main Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants