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

test: run axe-playwright a11y tests using test-storybook #2400

Merged
merged 18 commits into from
Sep 25, 2024

Conversation

unekinn
Copy link
Contributor

@unekinn unekinn commented Sep 11, 2024

ci: enable Biome's "noSvgWithoutTitle" rule, with some exceptions

fix(storybook): delay Storybook tests in Tooltip/DefaultOpen to let tooltip fade in
Fixes false positive color-contrast violation due to axe checking the contrast while the tooltip was not fully faded in yet

refactor(css): animate skeleton background instead of opacity to avoid false positive in a11y contrast test

fix(storybook): disable a11y tests for size/alignment testing stories

fix(react): set Switch wrapper to aria-disabled when disabled=true
Fixes Axe a11y tests reporting insufficient contrast on the description text when the Switch is disabled

fix(storybook): axe violation svg-img-alt in Tabs

fix(storybook): axe violation svg-img-alt in ToggleGroup

fix(storybook): axe violation aria-allowed-role in Alert

fix(storybook): axe violations svg-img-alt & button-name in Badge

fix(storybook): axe violations svg-img-alt & button-name in Button

fix(storybook): axe violations empty-table-header & label in Table

fix(storybook): axe violation scrollable-region-focusable in Table

fix(storybook): axe violation label in Combobox

fix(storybook): axe violation select-name in Showcase

build: make Biome ignore coverage dir by reading .gitignore

ci: add test report publishing also for unit tests

ci: move test coverage report into test.yml to avoid testing twice

test: run axe-playwright a11y tests using test-storybook

Closes #2384
Closes #2452

Copy link

changeset-bot bot commented Sep 11, 2024

⚠️ No Changeset found

Latest commit: 44e698b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Sep 11, 2024

Preview deployments for this pull request:

Storybook - 25. Sep 2024 - 10:16

Copy link
Contributor

github-actions bot commented Sep 11, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 62.07% 4267 / 6874
🔵 Statements 62.07% 4267 / 6874
🔵 Functions 85.85% 170 / 198
🔵 Branches 75.69% 573 / 757
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
packages/react/src/components/form/Switch/Switch.tsx 100% 85.71% 100% 100%
Generated in workflow #9

@unekinn unekinn force-pushed the ci-storybook-testing branch 4 times, most recently from 79bdc51 to e4186e9 Compare September 11, 2024 12:52
@unekinn
Copy link
Contributor Author

unekinn commented Sep 11, 2024

Feedback necessary

@mimarz I don't feel like the checks.yml workflow is quite the right place for this to run.

Suggestion: I think the Storybook tests should be run after unit tests, which happen in checks-packages.yml. A bonus is that yarn build is already run there, which saves some build time. However, that particular workflow is currently only run on changes to packages/**.

For the storybook tests to function reliably, we would need to change the workflow config in checks-packages.yml to

  pull_request:
    paths:
      - 'packages/**'
      - 'apps/storybook/**

Is that okay?

@mimarz
Copy link
Collaborator

mimarz commented Sep 11, 2024

Feedback necessary

@mimarz I don't feel like the checks.yml workflow is quite the right place for this to run.

Suggestion: I think the Storybook tests should be run after unit tests, which happen in checks-packages.yml. A bonus is that yarn build is already run there, which saves some build time. However, that particular workflow is currently only run on changes to packages/**.

For the storybook tests to function reliably, we would need to change the workflow config in checks-packages.yml to

  pull_request:

    paths:

      - 'packages/**'

      - 'apps/storybook/**

Is that okay?

Jepp. It could probably be renamed to a "Test" action as the checks changed a bit after we moved to biomejs.

@unekinn unekinn force-pushed the ci-storybook-testing branch 15 times, most recently from baae872 to 489d9d0 Compare September 16, 2024 16:10
unekinn added a commit that referenced this pull request Sep 16, 2024
This is just a placeholder workflow, the actual contents of the workflow
will be populated in #2400. But [the workflow dispatch
GUI](https://github.com/digdir/designsystemet/actions/workflows/test-reports.yml)
(and `gh workflow run` CLI) only works if the workflow file is already
present on the default branch.
@unekinn unekinn force-pushed the ci-storybook-testing branch 4 times, most recently from 3cbdd7b to 73fa181 Compare September 18, 2024 10:10
@unekinn unekinn force-pushed the ci-storybook-testing branch 3 times, most recently from 7f60bd8 to f9b9ec2 Compare September 19, 2024 15:06
@unekinn unekinn marked this pull request as ready for review September 19, 2024 15:10
biome.jsonc Show resolved Hide resolved
Copy link
Collaborator

@mimarz mimarz left a comment

Choose a reason for hiding this comment

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

Nice work!

Interesting to see how this testing will go :D

Fixes Axe a11y tests reporting insufficient contrast on the description text when the Switch is disabled
…ooltip fade in

Fixes false positive color-contrast violation due to axe checking the contrast while the tooltip was not fully faded in yet
@unekinn unekinn force-pushed the ci-storybook-testing branch from 591976e to 44e698b Compare September 25, 2024 08:13
@unekinn unekinn merged commit 6f07d0c into next Sep 25, 2024
7 checks passed
@unekinn unekinn deleted the ci-storybook-testing branch September 25, 2024 09:32
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.

investigation: accessibility violations reported by axe Add axe testing for our react components
2 participants