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

[Issue #677]: Add accessibility checks against pr preview env #84

Closed
wants to merge 58 commits into from

Conversation

rylew1
Copy link

@rylew1 rylew1 commented Jul 2, 2024

Ticket

Resolves #677

Changes

  • Add accessibility checks

Run of accessibility job:

Run set -e # Ensure the script fails if any command fails

> app@0.1.0 test:pa11y-desktop
> pa11y-ci --config .pa11yci-desktop.json

Running Pa11y on 1 URLs:
 > http://p-84-app-dev-[10](https://github.com/navapbc/platform-test-nextjs/actions/runs/9812208957/job/27095994046?pr=84#step:10:11)41375336.us-east-1.elb.amazonaws.com/ - 0 errors

✔ 1/1 URLs passed

> app@0.1.0 test:pa11y-mobile
> pa11y-ci --config .pa11yci-mobile.json

Running Pa11y on 1 URLs:
 > http://p-84-app-dev-104[13](https://github.com/navapbc/platform-test-nextjs/actions/runs/9812208957/job/27095994046?pr=84#step:10:14)75336.us-east-1.elb.amazonaws.com/ - 0 errors

✔ 1/1 URLs passed
pa11y-ci tests finished.

Preview environment

package.json Outdated Show resolved Hide resolved
@rylew1 rylew1 requested review from lorenyu July 2, 2024 01:04
Copy link

github-actions bot commented Jul 2, 2024

Coverage report for app

St.
Category Percentage Covered / Total
🟢 Statements 93.1% 81/87
🟢 Branches 82.35% 14/17
🟢 Functions 93.33% 14/15
🟢 Lines 93.59% 73/78

Test suite run success

16 tests passing in 5 suites.

Report generated by 🧪jest coverage report action from af8e2bd

@rylew1 rylew1 marked this pull request as ready for review July 3, 2024 15:00
@rylew1 rylew1 force-pushed the rylew/677-add-pa11y-checks branch from f7f6f50 to abd3dc7 Compare July 3, 2024 17:21
package.json Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
accessibility/.pa11yci-mobile.json Outdated Show resolved Hide resolved
accessibility/.pa11yci-mobile.json Outdated Show resolved Hide resolved
accessibility/.pa11yci-mobile.json Outdated Show resolved Hide resolved
accessibility/.pa11yci-mobile.json Outdated Show resolved Hide resolved
accessibility/.pa11yci-mobile.json Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Take a look at this comment on the e2e PR. I'd think that we'd want to do something similar here in order to support multiple apps and in order to be able to support running the accessibility scans outside the context of a PR environment. In other words, I think we'll want to have this workflow take an app_name input in addition to a service_endpoint input, then use that to load app-specific configurations (since different apps will have different sets of URLs).

Copy link
Author

Choose a reason for hiding this comment

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

Added a quick if/else statement in the workflow

Copy link
Collaborator

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

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

🚢

@lorenyu lorenyu self-requested a review July 9, 2024 17:20
@rylew1
Copy link
Author

rylew1 commented Jul 16, 2024

I think we can close this but keep it as a reference - we moved to e2e playwright a11y instead for the platform : #85

@rylew1
Copy link
Author

rylew1 commented Oct 16, 2024

We ended up using a11y checks in playwright rather than shipping a separate (pa11y) dependency for it - see the e2e PR on the template-infra repo https://github.com/navapbc/template-infra/pull/694/files

Closing this

@rylew1 rylew1 closed this Oct 16, 2024
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