MCR-3080: bring back a11y tests in ci #2715

JasonLin0991 · 2024-08-29T03:15:16Z

Summary

Removed pa11y plugin
- Tried unsuccessfully to try to get pa11y to work again.
Added cypress-axe and axe-core to run our a11y tests.
Updated documentation.
Added 2 accessibly tests
- 'has no a11y violations on submission form with form input errors'
- 'has no a11y violations on CMS dashboards'
Removed protobufjs from package.json in services/cypress
Fixed an issue with local Cypress LD interceptions.
- We are using some kind of proxy for LD api requests so for local deployment we have to look for a different url.

The a11y logs for violations could be better. Rather than including it into this ticket I have a new one to improve the logs.
https://jiraent.cms.gov/browse/MCR-4416

These are two more tickets that add more a11y testing to other parts of the app that I think could really use it. They are not completely filled out and could use some refinement.
https://jiraent.cms.gov/browse/MCR-4417
https://jiraent.cms.gov/browse/MCR-4418

There are three violations with the accessibility testing. So to not increase the scope of this ticket, I made two tickets to address the violations

https://jiraent.cms.gov/browse/MCR-4420
https://jiraent.cms.gov/browse/MCR-4421

Related issues

Screenshots

Test cases covered

acccessibility.spec.ts

'has no a11y violations on submission form with form input errors'
'has no a11y violations on CMS dashboards'

QA guidance

…s-in-ci' into jl-mcr-3080-bring-back-a11y-tests-in-ci # Conflicts: # services/cypress/package.json

…s-in-ci' into jl-mcr-3080-bring-back-a11y-tests-in-ci

pnpm-lock.yaml

JasonLin0991 · 2024-08-29T21:29:23Z

package.json

@@ -71,6 +71,8 @@
    "devDependencies": {
        "c8": "^10.1.2",
        "cypress": "^13.13.2",
+        "axe-core": "^4.10.0",
+        "cypress-axe": "^1.5.0",


@mojotalantikite I added these two dependencies here too. Correct me if things changed but, I did this because Cypress in CI runs from root and on local is runs from services/cypress without this here CI could not find the dependencies. A lot has changed since so is there a better, but simple, way to not have to install in both places?

I could do some configuration in each test to point to the location of the install based on deployment, but if installing the deps in both doesn't cost or complicate much would this be ok?

@mojotalantikite @JasonLin0991 imo this is also a good time to consider if cypress should be moved considering the new world we are. Throwing it out there, not trying to suggest moving the entire folder is needed ...just want us to think expansively since we are also doing a lot around modularizing and simplifying our top level architecture right now so maybe we are seeing another opportunity here.

This is totally fine. I personally think it's clearer to declare the dependencies in each service's package.json that needs them. Back with Yarn 1 everything got hoisted and when we moved to pnpm it started complaining about missing packages, as it was easy to just ignore the dependency if it was already installed elsewhere with Yarn.

Plus with pnpm everything is symlinked around, so it's not actually going to install it in two places, it's just going to throw in a symlink from the global package store to each service's node_modules that declares it.

haworku · 2024-08-30T15:20:35Z

docs/architectural-decision-records/005-frontend-a11y-toolset.md

@@ -63,3 +78,6 @@ Together these tools include linting, testing, and auditing loaded web pages for
 [axe-core-cli]: https://www.npmjs.com/package/@axe-core/cli
 [cypress-audit]: https://github.com/mfrachet/cypress-audit
 [lighthouse-ci]: https://github.com/GoogleChrome/lighthouse-ci


ADRs aren't supposed to be edited in hindsight. They are meant to be a point in time record.

So what you do here - this would be a new ADR to update frontend a11y toolset to use cypress-axe and then you also edit ADR 005 to say for the pa11y decision - superseded by ADR 00XXX. Theres already an example of this with protobufs where ADR 024 supercedes ADR 009.

haworku · 2024-08-30T15:26:24Z

docs/architectural-decision-records/005-frontend-a11y-toolset.md

@@ -19,7 +19,7 @@ Together these tools include linting, testing, and auditing loaded web pages for

 [eslint jsx-a11y][eslint-a11y] Provides React-friendly code linting which allows developers to find accessibility issues during local development.

-[pa11y][pa11y] and [pa11y-ci][pa11y-ci] - Accessibility test runner for end to end testing in the command line or Node.js. End to end testing can catch concerns that are not noticeable in linting or unit tests since tests run on browser.
+[cypress-axe][cypress-axe] and [axe-core][axe-copre] - Accessibility test runner for end to end testing in the command line or Node.js. End to end testing can catch concerns that are not noticeable in linting or unit tests since tests run on browser. Our previous choice [pa11y][pa11y] seems to not be maintained anymore and no longer works with our end to end testing.

 [jest-axe][jest-axe] - This tool provides additional jest matchers, like `.toHaveNoViolations`, to use in unit tests. This may be redundant when end to end accessibility testing is present.


Also a request - could you please also clarify that jest-axeis removed from our tool set? Reason is we aren't using it and it was removed from package.json I think awhile ago so its not in use.

haworku · 2024-08-30T15:28:23Z

services/cypress/integration/stateWorkflow/stateSubmissionForm/accessiblility.spec.ts

-            cy.deprecatedNavigateV1Form('CONTINUE')
-            cy.deprecatedNavigateV1Form('CONTINUE')
+        cy.fillOutContractActionAndRateCertification()
+        cy.deprecatedNavigateV1Form('CONTINUE_FROM_START_NEW')


I thought Pearl merged her work and start, submission and contract details are are all on v2. I think this could be using the v2 helpers?

I don't see a v2 of fillOutContractActionAndRateCertification. I do see startNewContractOnlySubmissionWithBaseContractV2, but the purpose of the code here is to:

check a11y violations on the empty form with field errors

fill out the form and continue so that we have a submission to go to each form page.

Using startNewContractOnlySubmissionWithBaseContractV2 would fill this out and continue before we could check for a11y violations.

Look at navigateContractAndRatesForm and navigateContractForm - I was looking at the navigate command. We don't want to be use the deprecated commands anymore at all we should be removing them.

That's fine to use fillOUtContractActionAndRateCertification agree we aren't done with the whole form yet

haworku · 2024-08-30T15:53:07Z

services/cypress/support/accessibilityCommands.ts

+        cy.checkA11y('', {
+            runOnly: {
+                type: 'tag',
+                values: ['wcag22aa']


I don't think this is working - I added basic accessibility violations (like an image missing alt text, a button with no text content) and there were no errors. When I change this value I start to get errors again. I think it is not working for some version saw a github issue or two as well

I think I'm seeing best-practice work lets just use that if you agree that 22aa is broken and so is wcagaa right now.

Just to make visible our conversation. Rules for each standard are not cumulative. So in order to test all rules, all the standards must be included. If we only include 'wcag22aa' then only the specific rules added in for that standard will be tested.

The configuration has been updated to include the complete rules

cy.checkA11y('', { runOnly: { type: 'tag', values: ['wcag2a','wcag2aa', 'wcag21a', 'wcag21aa','wcag22aa']

Hi @JasonLin0991 and @haworku , Even in this above eg:
cy.checkA11y('', {
runOnly: {
type: 'tag',
values: ['wcag2a','wcag2aa', 'wcag21a', 'wcag21aa','wcag22aa']

No violations are getting shown for wcag21a and wcag22aa.

@sethims I followed the axe-core docs under axe.run() to figure out that the tags need to be combined. You can look through the rule of each standard using axe.getRules(). Compare the rules from getRules with the specific rule violations you are testing for. That should help with debugging. Good luck!

JasonLin0991 added 4 commits August 28, 2024 11:37

ADd and implement cypress-axe

10fe077

Add accessibility commands

36ceb71

Fix launch darkly intercepts for local deployment.

ca34107

Add two accessibility tests.

41333e9

JasonLin0991 marked this pull request as draft August 29, 2024 03:15

Reverting bad merge

1014ee0

JasonLin0991 force-pushed the jl-mcr-3080-bring-back-a11y-tests-in-ci branch from ee5389e to 1014ee0 Compare August 29, 2024 03:39

github-actions bot deployed to review-apps August 29, 2024 03:40 View deployment

JasonLin0991 added 7 commits August 28, 2024 23:44

ADd and implement cypress-axe

eebe3cc

Add accessibility commands

3f9a1e2

Fix launch darkly intercepts for local deployment.

64f664f

Add two accessibility tests.

2167a6c

Reverting bad merge

0bc8e79

Merge remote-tracking branch 'origin/jl-mcr-3080-bring-back-a11y-test…

ce879d7

…s-in-ci' into jl-mcr-3080-bring-back-a11y-tests-in-ci # Conflicts: # services/cypress/package.json

lock file

7861efd

github-actions bot deployed to review-apps August 29, 2024 03:55 View deployment

JasonLin0991 added 2 commits August 28, 2024 23:59

Reverting bad merge again

cfae62b

Merge remote-tracking branch 'origin/jl-mcr-3080-bring-back-a11y-test…

e2e2df3

…s-in-ci' into jl-mcr-3080-bring-back-a11y-tests-in-ci

github-actions bot deployed to review-apps August 29, 2024 04:03 View deployment

Fix lock file.

585d269

github-actions bot deployed to review-apps August 29, 2024 04:12 View deployment

JasonLin0991 added 3 commits August 29, 2024 00:12

Fix lock file.

f6ff9a9

Remove pa11y.

58fdd78

Add axe-core and cypress-axe to root package.json

14c2fe6

github-actions bot deployed to review-apps August 29, 2024 04:21 View deployment

github-actions bot deployed to review-apps August 29, 2024 04:30 View deployment

JasonLin0991 added 3 commits August 29, 2024 12:49

Cleanup

edd8960

Use WCAG 2.2 Level AA

833c0ae

Lock file

e60db8f

github-actions bot deployed to review-apps August 29, 2024 16:55 View deployment

github-actions bot deployed to review-apps August 29, 2024 16:57 View deployment

Add documentation

3402246

github-actions bot deployed to review-apps August 29, 2024 20:27 View deployment

Rename tests

09dea90

github-actions bot deployed to review-apps August 29, 2024 21:19 View deployment

JasonLin0991 marked this pull request as ready for review August 29, 2024 21:20

JasonLin0991 requested review from mojotalantikite and haworku and removed request for mojotalantikite August 29, 2024 21:20

JasonLin0991 commented Aug 29, 2024

View reviewed changes

pnpm-lock.yaml Show resolved Hide resolved

JasonLin0991 commented Aug 29, 2024

View reviewed changes

JasonLin0991 requested review from pearl-truss and macrael August 29, 2024 21:29

haworku reviewed Aug 30, 2024

View reviewed changes

Merge branch 'main' into jl-mcr-3080-bring-back-a11y-tests-in-ci

75a1d3a

haworku reviewed Aug 30, 2024

View reviewed changes

JasonLin0991 added 3 commits August 30, 2024 15:29

Update ADR

6fe5157

Replace depreciated command

ea421f5

Skipping violations and made tickets to address them.

eb4b34d

github-actions bot deployed to review-apps August 30, 2024 19:32 View deployment

Update doc

321d164

JasonLin0991 requested a review from haworku August 30, 2024 19:40

github-actions bot deployed to review-apps August 30, 2024 19:41 View deployment

haworku approved these changes Aug 30, 2024

View reviewed changes

JasonLin0991 merged commit a270f44 into main Aug 30, 2024
28 checks passed

JasonLin0991 deleted the jl-mcr-3080-bring-back-a11y-tests-in-ci branch August 30, 2024 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MCR-3080: bring back a11y tests in ci #2715

MCR-3080: bring back a11y tests in ci #2715

JasonLin0991 commented Aug 29, 2024 •

edited

Loading

JasonLin0991 Aug 29, 2024

haworku Aug 30, 2024 •

edited

Loading

mojotalantikite Aug 30, 2024

haworku Aug 30, 2024

haworku Aug 30, 2024

haworku Aug 30, 2024

JasonLin0991 Aug 30, 2024

haworku Aug 30, 2024 •

edited

Loading

haworku Aug 30, 2024 •

edited

Loading

haworku Aug 30, 2024

JasonLin0991 Aug 30, 2024 •

edited

Loading

sethims Sep 3, 2024

JasonLin0991 Sep 5, 2024 •

edited

Loading

MCR-3080: bring back a11y tests in ci #2715

MCR-3080: bring back a11y tests in ci #2715

Conversation

JasonLin0991 commented Aug 29, 2024 • edited Loading

Summary

Related issues

Screenshots

Test cases covered

QA guidance

Choose a reason for hiding this comment

haworku Aug 30, 2024 • edited Loading

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

haworku Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

haworku Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonLin0991 Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonLin0991 Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

JasonLin0991 commented Aug 29, 2024 •

edited

Loading

haworku Aug 30, 2024 •

edited

Loading

haworku Aug 30, 2024 •

edited

Loading

haworku Aug 30, 2024 •

edited

Loading

JasonLin0991 Aug 30, 2024 •

edited

Loading

JasonLin0991 Sep 5, 2024 •

edited

Loading