Skip to content

fix(aria): Improved Aria snapshot diffing #36101

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

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

agg23
Copy link
Contributor

@agg23 agg23 commented May 27, 2025

Fixes #34555.

The current implementation of Aria Snapshots is tailored towards automated workflows and doesn't handle well when humans have to interact with failures. In particular, the result of our snapshotting algorithm is naively diffed by the Jest string comparison function. Aria snapshots support more complex constructs like embedded regex (via /some\wregex/), and thus any failure in a snapshot will flag all regular expressions as a diff, highlighting them in red. In any situation this is hard to parse, but on larger snapshots it requires manual evaluation of every regex in the template to find the actual line that's causing your failure.

This solution annotates our match result with the actual lines in the final rendered "template" string we know are matching. For a given matched line, the toMatchAriaSnapshot() matcher copies the actual received string over to the expected template string, which is passed to the Jest differ as before.

The result is that any lines we considered to be matching will also be considered matching by Jest, therefore not notifying the user of fake problems.

@agg23 agg23 requested a review from pavelfeldman May 27, 2025 15:14
Copy link
Contributor

Test results for "tests 1"

8 flaky ⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @chromium-ubuntu-22.04-node18
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @chromium-ubuntu-22.04-node20
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @chromium-ubuntu-22.04-node22
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:986:7 › cli codegen › should not throw csp directive violation errors @firefox-ubuntu-22.04-node18
⚠️ [firefox-library] › library/inspector/cli-codegen-aria.spec.ts:76:7 › should update aria snapshot highlight @firefox-ubuntu-22.04-node18
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @ubuntu-22.04-chromium-tip-of-tree
⚠️ [webkit-library] › library/ignorehttpserrors.spec.ts:30:3 › should isolate contexts @webkit-ubuntu-22.04-node18
⚠️ [playwright-test] › ui-mode-test-watch.spec.ts:145:5 › should watch all @windows-latest-node18-1

39285 passed, 808 skipped
✔️✔️✔️

Merge workflow run.

@pavelfeldman
Copy link
Member

I acknowledge the problem and I'd like to fix it, but the implementation raises some questions:

  • I noticed you are using line numbers. we have a tree structure that we serialize into yaml. You don't quite know the line numbers in yaml, so I'm not sure how this is supposed to work
  • Would it make more sense to return two versions of the snapshot, one raw for rebaseline and one rendered with the regexes retained for the better diff?

@agg23 agg23 marked this pull request as draft May 29, 2025 14:29
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.

[BUG] aria snapshot's regex marked as errors if the test fails
2 participants