Conversation
ScharfViktor
force-pushed
the
a11yTests
branch
2 times, most recently
from
010e3d7 to
84199fd
Compare
AlexAndBear
force-pushed
the
a11yTests
branch
3 times, most recently
from
403c9ab to
ecd30b2
Compare
|
I think that's enough for the first step. what I do:
|
ScharfViktor
requested review from
AlexAndBear,
JammingBen,
Copilot and
individual-it
There was a problem hiding this comment.
Pull Request Overview
This PR integrates automated accessibility checks into the end-to-end tests by leveraging AxeBuilder for Playwright. The key changes include:
- Introducing the checkAccessibility function and patching Playwright’s locator methods (click, fill, press) to trigger accessibility checks.
- Enhancing session and actor objects to pass an accessibility flag, and adding new view mode options.
- Updating environment and Cucumber steps to enable accessibility tests and extend resource view mode handling.
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/support/utils/accessibility.ts | Adds accessibility check function and patches locator methods to run a11y checks after actions |
| tests/e2e/support/objects/runtime/session.ts | Extends login and sign-in methods to optionally run accessibility checks |
| tests/e2e/support/objects/app-files/resource/index.ts tests/e2e/support/objects/app-files/resource/actions.ts |
Introduces a switchViewMode function and expands target options for view mode toggling |
| tests/e2e/support/environment/actor/actor.ts | Applies the a11y patch conditionally based on the A11Y_TEST env variable |
| tests/e2e/cucumber/* | Updates step definitions and environment configuration to support accessibility tests |
| package.json | Adds @axe-core/playwright dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
Looks very clean and straight forward to me ! run pnpm lint --fix |
| '{string} check accessibility of the {string} page', | ||
| async function (this: World, stepUser: string, place: string): Promise<void> { | ||
| const { page } = this.actorsEnvironment.getActor({ key: stepUser }) | ||
| await checkAccessibility(page, `personal space page`, place) |
There was a problem hiding this comment.
personal space page will be only true for some tests.
Can we get enough context e.g. from the URL or maybe we can put it into the step definition
'{string} checks the accessibility of the DOM element {string} on the {string}
so in the feature file it will be: When "Alice" checks the accessibility of the DOM element ".files-view-wrapper" on the "personal space page"
There was a problem hiding this comment.
Thank you, I'll leave your comment unresolved and fix it in the next PR.
| import { checkAccessibility } from '../../../support/utils/accessibility' | ||
|
|
||
| Then( | ||
| '{string} check accessibility of the {string} page', |
There was a problem hiding this comment.
I would suggest to change it to
'{string} checks the accessibility of the DOM element {string} on the current page',
related #400
This PR implements the checkAccessibility() function to check accessibility using e2e playwright tests with the following recommended rule tags: ['wcag2a', 'wcag2aa', 'wcag21a', 'wcag21aa', 'best-practice']
Next, we need to decide on a strategy, namely:
A11Y_TEST=true).checkAccessibility()I experimented and wrapped each click with
patchPageForA11yhere is result of running one test with enabled/disabled
A11Y_TESTrunning one suite without a11y:
10sec:
running one suite witht a11y` 17 sec:
tests become fragile.
Log
running one suite:
without a11y
HEADLESS=true A11Y_TEST=false pnpm test:e2e:cucumber tests/e2e/cucumber/features/smoke/with a11y for all tests
HEADLESS=true A11Y_TEST=true pnpm test:e2e:cucumber tests/e2e/cucumber/features/smoke/error: