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

Component accessibility testing #1352

Merged
merged 82 commits into from
Jun 29, 2023
Merged

Conversation

xman343
Copy link
Contributor

@xman343 xman343 commented Jun 12, 2023

Changes

Adding a Cypress script that tests all the components in the examples folder for accessibility violations using cypress-axe.

Testing

To run all tests headlessly: yarn test --filter=a11y from root.

To run interactively inside browser (locally): yarn workspace a11y open.

Docs

N/A

@xman343 xman343 self-assigned this Jun 13, 2023
package-lock.json Outdated Show resolved Hide resolved
@mayank99

This comment was marked as outdated.

@mayank99

This comment was marked as outdated.

@mayank99 mayank99 changed the title Xander/component accessibility testing Component accessibility testing Jun 14, 2023
@mayank99
Copy link
Contributor

Another option you might want to explore is Cypress component testing. It is considered stable, unlike Playwright component testing (which is still experimental), so it's possible that you might be able to loop over components in cypress just fine. Definitely worth experimenting. If you run into similar limitations, then you can carry on with my suggestion above about using playwright e2e testing instead.

Note: You'll have to use cypress-axe instead of @axe-core/playwright.

@xman343
Copy link
Contributor Author

xman343 commented Jun 28, 2023

I think we should run these tests in CI as well.

You can add a new job in build.yml that looks something like this:

  a11y:
    name: Test for a11y violations
    needs: install
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3

      - name: Use Node 18.X
        uses: actions/setup-node@v3
        with:
          node-version: 18.x

      - name: Cache node_modules
        uses: actions/cache@v3
        id: cache-node-modules
        with:
          path: |
            node_modules
            packages/*/node_modules
            apps/*/node_modules
            playgrounds/*/node_modules
          key: modules-${{ runner.os }}-${{ hashFiles('yarn.lock') }}

      - run: yarn test --filter=a11y

Added!

testing/a11y/package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 working great both locally and in CI.

Now lets start fixing all reported violations in future PRs.

@xman343 xman343 enabled auto-merge June 29, 2023 13:40
@mayank99 mayank99 disabled auto-merge June 29, 2023 13:40
Copy link
Contributor

@LostABike LostABike left a comment

Choose a reason for hiding this comment

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

Testing/playing around with this PR. If I understand correctly, we are running a pre-written script axe-core on all of our components? I am able to run the test, and it would tell me there are components that fail accessibility but doesn't specify how. Does it simply test for aria-label on all components? nevermind I just saw the exact test rules and descriptions! @xman343 @mayank99

@xman343
Copy link
Contributor Author

xman343 commented Jun 29, 2023

Testing/playing around with this PR. If I understand correctly, we are running a pre-written script axe-core on all of our components? I am able to run the test, and it would tell me there are components that fail accessibility but doesn't specify how. Does it simply test for aria-label on all components? nevermind I just saw the exact test rules and descriptions! @xman343 @mayank99

Anything I should add, or does the code look good to you?

@LostABike
Copy link
Contributor

Testing/playing around with this PR. If I understand correctly, we are running a pre-written script axe-core on all of our components? I am able to run the test, and it would tell me there are components that fail accessibility but doesn't specify how. Does it simply test for aria-label on all components? nevermind I just saw the exact test rules and descriptions! @xman343 @mayank99

Anything I should add, or does the code look good to you?

I'm not too well-versed in configurating modules and scripts so I'm just learning from your code :D But I have read them and I think they look good. Also tested functionality and seems to be working as expected 👍

@xman343 xman343 enabled auto-merge June 29, 2023 14:58
@xman343 xman343 added this pull request to the merge queue Jun 29, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi i updated this because visual tests were failing even after retrying 7 times.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 29, 2023
@mayank99 mayank99 added this pull request to the merge queue Jun 29, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 29, 2023
@xman343 xman343 added this pull request to the merge queue Jun 29, 2023
Merged via the queue into dev with commit d1dc494 Jun 29, 2023
@xman343 xman343 deleted the xander/component-accessibility-testing branch June 29, 2023 20:26
@mayank99 mayank99 linked an issue Jul 5, 2023 that may be closed by this pull request
23 tasks
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.

Testing accessibility for all components
3 participants