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

[Bug]: Can't use radio value with .toMatchSnapshot() since Playwright 1.50.0 #34633

Closed
mduivcap opened this issue Feb 5, 2025 · 4 comments
Closed

Comments

@mduivcap
Copy link

mduivcap commented Feb 5, 2025

Version

1.50.1

Steps to reproduce

Example test

import { test, expect } from '@playwright/test';

test('test', async ({ page }) => {
  await page.goto('https://www.platformrijksoverheidonline.nl/actueel/agenda');
  await page.getByRole('button', { name: 'Toon Filters' }).click();
  await expect(page.locator('#facetBlock')).toMatchAriaSnapshot(`
    - group:
      - text: "Periode:"
      - radio "Geen periodefilter" [checked]: periodFilter.none
    `);
});

Expected behavior

With version 1.49.1 I could use the value of the radio button like the example above
- radio "Geen periodefilter" [checked]: periodFilter.none

Actual behavior

Since I updated to version 1.50.0 or 1.50.1 the test fails because of the value : periodFilter.none.
If I remove this part, the test passes again.
- radio "Geen periodefilter" [checked]

Additional context

Image

Environment

System:
    OS: Windows 11 10.0.22631
  Binaries:
    Node: 23.6.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.9.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 10.2.0 - ~\AppData\Local\pnpm\pnpm.EXE
  Languages:
    Bash: 5.1.16 - C:\WINDOWS\system32\bash.EXE
  npmPackages:
    @playwright/test: ^1.50.1 => 1.50.1
@mduivcap mduivcap changed the title [Bug]: Can't use radio value with .toMatchSnapshot() with Playwright 1.50.1 [Bug]: Can't use radio value with .toMatchSnapshot() with Playwright 1.50.0+ Feb 5, 2025
@mduivcap mduivcap changed the title [Bug]: Can't use radio value with .toMatchSnapshot() with Playwright 1.50.0+ [Bug]: Can't use radio value with .toMatchSnapshot() since Playwright 1.50.0 Feb 5, 2025
@dgozman
Copy link
Contributor

dgozman commented Feb 6, 2025

@mduivcap Thank you for your feedback, we really appreciate you using this new feature and sharing your thoughts!

There was a bugfix in #33941. The reason is that checkbox/radio values are not actually visible to the user and do not belong to the ARIA tree. Unfortunately, that broke your particular snapshot, so you'll have to rebase it. You can probably expect a few more small fixes like that - we are trying to minimize the impact while fixing all the oversights based on the customers' feedback.

Again, please share more feedback for this feature, we'd love to hear it!

@mduivcap
Copy link
Author

mduivcap commented Feb 7, 2025

I think this particular thing will be a quick fix. (just remove the values from the test).

My view (since you asked for it) is that I like having multiple options so I can see what works best for my situation to validate something. In the example above it does not matter, I could leave out the value.
However if you start to remove all kinds of functions/options that validate stuff that is not visible to the end user, you basically disable your test framework.
Imagine that this function will be taken out... (it's not visible)
https://playwright.dev/docs/api/class-locatorassertions#locator-assertions-to-have-attribute
Sometimes a text is just that, but for the inner workings of an application the non visible values matter and make sense to validate on as well.

@dgozman
Copy link
Contributor

dgozman commented Feb 7, 2025

@mduivcap Thank you for sharing your perspective. Playwright is providing various tools, many of them operate on non-user-visible values. However, aria snapshot is supposed to follow the accessibility standards, and those usually only include visible text, although not always.

Anyway, closing this issue since there is no action planned. Thank you for filing!

@dgozman dgozman closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2025
@mduivcap
Copy link
Author

mduivcap commented Feb 7, 2025

I once had to write tests for a website that wanted to reuse all test on all language variantions of the site (since this also tested if the SAP implementation worked etc). So I always have this example in mind when I create tests. Now I could also add some layers in the test framework to cover all translations for all elements, OR I could leave out that complexity and just deal with different selectors and validations. So that is always on my mind with these kind of discussions. No wrong/right (as long as the test does what it's suppose to do), but I like simplicity in my test framework when possible. So this is always on my mind in these discussions.

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

No branches or pull requests

2 participants