Skip to content

Switch a11y tests to private storefront and single set of snapshots - #3416

Merged
shethj merged 2 commits into
developfrom
e2e/nightly-a11y-update
Oct 22, 2025
Merged

shethj merged 2 commits into
developfrom
e2e/nightly-a11y-update

Conversation

@shethj

@shethj shethj commented Oct 22, 2025 •

Copy link
Copy Markdown
Contributor

Description

We run a11y tests with E2E on per-pr as well as nightly workflows. The current setup was split up between running a11y on public client storefront for nightly and private client for per-pr workflows.

This caused storefront setups with different storefrontdata and required different sets of snapshots.
A snapshot for a11y is basically a set of known violations for a storefront.

The issue now is whenever a PR updates snapshots (removes violations because they fixed something), we need to sync snapshots on the other workflow.

To avoid this, a better idea is to run both per-pr and nightly workflow against a private client setup so the storefront setup is the same and we can use 1 set of snapshots and avoid having to maintain a sync between the two.

This PR updates tests, workflows and playwright config to run against a single set of snapshots.

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • Update playwright config
  • Update e2e and e2e-pr workflows to run updated a11y test
  • Rename snapshot directories

How to Test-Drive This PR

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@shethj
shethj requested a review from a team as a code owner October 22, 2025 18:04
@cc-prodsec

cc-prodsec commented Oct 22, 2025 •

Copy link
Copy Markdown
Collaborator

✅ Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
✅ Licenses 0 0 0 0 0 issues
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@alexvuong

Copy link
Copy Markdown
Contributor

Which instance are snapshots created on? Some of the violations are gone which look strange to me. We know there are some violations that can't be fixed due to the fact it comes from Chakra UI.

"helpUrl": "https://dequeuniversity.com/rules/axe/4.10/aria-allowed-attr?application=playwright",
"nodes": [
{
"html": "<form id=\"...\" aria-haspopup=\"dialog\" aria-expanded=\"false\" aria-controls=\"popover-content-...\">",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one can't never be fixed, it requires the change from Chakra UI component which we have no control off, I am wondering why it is not detected

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Umm... I still see the violation as a part of the snapshot. So it is being detected. This deletion change is not really removing the violation. We had 2 separate copies of snapshots and I deleted one of them. But the violations are still a part of the other copy.

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Agreed. It is still a part of the snapshots as known violation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh i see, sorry I did not realize we are having 3 snapshot batches. Makes sense!

@alexvuong

alexvuong commented Oct 22, 2025 •

Copy link
Copy Markdown
Contributor

@shethj Dumb question, using the same private setup is a good start, but may not be enough.
I am wondering how are these sites look like on MRT? Are they sharing the same config on deployment (e.g if RefArch vs RefArchGlobal in same client, or if the Productile Config is also affect the data being returned from api despite using the same client/store data)

@shethj

shethj commented Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

@shethj Dumb question, using the same private setup is a good start, but may not be enough. I am wondering how are these sites look like on MRT? Are they sharing the same config on deployment (e.g if RefArch vs RefArchGlobal in same client, or if the Productile Config is also affect the data being returned from api despite using the same client/store data)

After this PR is merged the setups will be identical. Both using the same instance, same site, same SLAS Client.

@shethj
shethj merged commit a2e9964 into develop Oct 22, 2025
63 of 65 checks passed
@shethj
shethj deleted the e2e/nightly-a11y-update branch October 22, 2025 21:44
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.

4 participants