- Notifications
You must be signed in to change notification settings - Fork 9
testing-library as a selector engine #330
Conversation
Going forward, two packages will be published: - **playwright-testing-library** to support the standalone queries with the **playwright** package (no longer supports extending the Playwright prototypes via `/extend`) - **@playwright-testing-library/test** to support both standalone queries with **@playwright/test** _as well as_ a Playwright Test [fixture](https://playwright.dev/docs/test-fixtures) (available at `@playwright-testing-library/test/fixture`) BREAKING CHANGE: Removes support for extending Playwright prototypes (`/extend` entrypoint) BREAKING CHANGE: Only Playwright 1.12+ is officially supported going forward Co-authored-by: Zacharias Björngren <zacharias.bjorngren@gmail.com>
I think you're talking about publishing playright-testing-library and @playwright-testing-library/test? That's irrelevant to this implementation and is because I want to continue to support both standalone playwright as well as @playwright/test. As for supporting @playwright/test via this approach or the original fixture... I don't think it would hurt to support both? I could see both being useful, but I definitely do like this approach because it returns Anyways, I still think I'm going to go ahead and release #329 as 4.0 and we can rebase this and release it after 4.0? I want to at least get the dual package publishing in because I do want to continue to support vanilla playwright in some form. @Zache, @sebinsua what do you think about this? In response to your "things of note":
@Zache would you be open to helping maintain this package after we sort all of this stuff out? I think once we do it's worth trying to get this moved into the official @testing-libary organization. |
| Oh, I almost forgot: thank you @Zache for once again digging into this and coming up with a creative solution. I was just starting to take a look at the selector engine stuff, and you are right, the documentation is quite limited. |
| I'd be honored to help maintain anything testing-library related :), and I'm looking at rebasing this on your fixture branch. Regarding the turtles it's just a regex for regexes, it probably won't cover all cases but at least it works for the ones I've tested. It absolutely makes sense to still support the queries via fixture since that would be the only way to give a sane option for providing options to the queries. Technically you could stuff JSON in there but lets not. One thing I'd like to see is to reuse the dom-testing-library that I add by "overloading" the page fixture for using the queries fixture, the performance impact might not be that big but it would feel much better. Or maybe this is a bad idea, because it does make sense to maybe tweak some of the base configuration of testing library to work better with selectors. Specifically I'm thinking about the hidden option for ByRole which by default ignores hidden elements, but in playwright selectors has their own opinions about hidden Also the queries should probably include a mixin for the fixtures like, I got the idea from https://github.com/bgotink/playwright-coverage/ |
1f66fa6 to 1e6f11c Compare | Hey @Zache. I'm very glad you acted on my comment! I also recently wrote my own version of this here, but as a single file and independent of this library. (Sorry for not posting this until now!) The main benefits of this approach, seem to be:
Regarding my version of this, I only support 'exact' matches and not Regexes. I have no need for a I also didn't allow any options to be set on
I also wondered about whether we need to somehow expose However, do we really need to do any of these things? The major value to me is being able to write something like |
| For ‘:visible’ it might be possible to understand how playwright does it and see if it’s worthwhile to mimic to pass on some options. The only one I’ve used to any extent before has been selector for ByLabeText and that is better solved with >>. …On Mon, 1 Nov 2021 at 16:48, Seb Insua ***@***.***> wrote: Hey @Zache <https://github.com/Zache>. I'm very glad you acted on my comment! I also recently wrote my own version of this <https://gist.github.com/sebinsua/205682c7af6966fdadc103b4e15d0d8b>, but as a single file and independent of this library. It'd be a good idea to compare approaches. The two main benefits from this approach, seem to be: - You don't need to await (await ... to interact with an ElementHandle. The locators are lazy so only the interaction (e.g. .click()) has to be awaited. - Tools like Inspector <https://playwright.dev/docs/inspector> and Trace Viewer <https://playwright.dev/docs/trace-viewer> work with your selectors, so debugging while writing the tests or after a failure is much easier. - ByLabelText doesn't have anything analogous in Playwright and UI testing is terrible without this API. Regarding my version of this, I only support 'exact' matches and not Regexes. I have no need for a turtle because of this. The one thing that I did which is a little unusual is that I stripped wrapping quote marks (e.g. ") and treated everything as exact. This was actually a little bit of a misunderstanding of Playwright's idioms on my behalf -- what I should probably be doing is using exact: true when a string is wrapped with quote marks but otherwise passing through the text as exact: false or perhaps a /regex/. My understanding of this idiom comes from the Playwright docs it says that the "text body can be escaped with single or double quotes to search for a text node with exact content" <https://playwright.dev/docs/selectors#text-selector>. I also didn't allow any options to be set on queryByRole <https://testing-library.com/docs/queries/byrole/>. Normally you would set text: /some text/i when you query by a role, but because we can chain custom selectors in Playwright this seems less important. Instead of letting @testing-library/dom handle this, you would write a selector like role=button >> text="Text inside button" to achieve the same thing. Specifically I'm thinking about the hidden option for ByRole <https://testing-library.com/docs/dom-testing-library/api-configuration#defaulthidden> which by default ignores hidden elements, but in playwright selectors has their own opinions about hidden and if I understand it correctly you should be able to use :visible to get the same result as { hidden: true } :visible is only for CSS selectors, nevertheless it would be nice for >> visible=true to work as expected. I also wondered about whether we need to somehow expose @testing-library options via the API. I thought it would be a bit horrendous to need to parse JSON/JS in a string, but if there are only a small number of options and they are not combined we could just combine them into the selector (e.g. roleHidden=button to include hidden buttons). If we did want to send more options through, I'd be tempted to re-implement the original query APIs on top of .locator(selector: string) since then the insanity of getting a user to hand-write in this DSL could be avoided and it could be typed. However, do we need to do any of these things? The major value to me is being able to write something like page.locator('label="First Name"').fill('Jamie') and to have this just work. The composability and debuggability benefits of this approach are also very valuable. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#330 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3AXQH5XPG3YBW4ULCPX3LUJ2ZDLANCNFSM5HBJ4Q5A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. |
| @Zache Ohh, I see what you're doing with @sebinsua this:
was my first thought after realizing we're forced to parse the Testing Library API out of the selector string... I really think this might make sense. What do you think about that @Zache? |
| I also thought of this, but when I made a small PoC I quickly noticed that for ByLabelText I needed to jump from a label to input via the for attribute and getting attribute values required async stuff with an element handle so you lose the convenience of locators not needing to be awaited. Maybe you can do some weird shit with XPath, or even see if playwright would like to implement support for things like for and aria-labelledby &c? …On Tue, 2 Nov 2021 at 20:29, Jamie Rolfs ***@***.***> wrote: @Zache <https://github.com/Zache> Ohh, I see what you're doing with turtle, you're just extracting the expression from the query selector string... I wonder if there's some happy middle ground between this and serializing/deserializing JSON. @sebinsua <https://github.com/sebinsua> this: I'd be tempted to re-implement the original query APIs on top of .locator(selector: string) since then the insanity of getting a user to hand-write selectors in a DSL could be avoided and the API could be typed. was my first thought after realizing we're forced to parse the Testing Library API out of the selector string... I really think this might make sense. What do you think about that @Zache <https://github.com/Zache>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#330 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3AXQGJJE4SHDERVYTVNFTUKBC3JANCNFSM5HBJ4Q5A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. |
| @Zache I'm a little confused by your comment:
It sounds like you're describing a use case where an It's also worth noting that locator instances do have methods ( Another idea that just occurred to me... maybe this is silly, but make the locator-based queries available at Standalone APIimport { webkit, selectors } from 'playwright'; import { getDocument, queries, registerSelectorEngine } from 'playwright-testing-library'; const browser = await webkit.launch() const page = await browser.newPage() // ... we could probably even do this lazily/on-demand, right? await registerSelectorEngine(selectors); // ElementHandle-based queries (current API) const document = await getDocument(page); const elementHandle = queries.getByLabelText(document, 'Label'); // Locator-based API (delegates to selector engine internally, serializing options, etc.) const locator = queries.getByLabelText.locator(page, 'Label');Fixture APIimport { test as baseTest, selectors } from '@playwright/test'; import { extend } from '@playwright-testing-library/test/fixture' // New `extend` method like your `mixin` version, could also register selectors or we could do it on demand const test = extend({ test: baseTest, selectors }); const { expect } = test // ElementHandle-based queries (current API) test('my form', ({ queries: { getByLabelText } }) => { const elementHandle = await getByLabelText('Label') // ... }) // Locator-based API test('my form', ({ queries: { getByLabelText } }) => { // Locator-based API (delegates to selector engine internally, serializing options, etc.) const locator = await getByLabelText.locator('Label') // ... }) |
| Not that an element handle was preferable but that to get attribute values you need to interact with an element handle which you of course can do from the locator with for example evaluateHandle. But doing so is an async API so you lose some of the “lazy evaluation” of locators. What I’m trying to say is that a typed query api can not return an actual locator if it was to be sync, it could of course return something implementing the same interface… …On Wed, 3 Nov 2021 at 00:12, Jamie Rolfs ***@***.***> wrote: @Zache <https://github.com/Zache> I'm a little confused by your comment: [...] I quickly noticed that for ByLabelText I needed to jump from a label to input via the for attribute and getting attribute values required async stuff with an element handle so you lose the convenience of locators not needing to be awaited. It sounds like you're describing a use case where an ElementHandle would be preferred over a Locator? I wasn't suggesting we replace the ElementHandle based API, rather that we could have a separate set of the query methods that return a Locator by leveraging the query selector engine. It's also worth noting that locator instances do have methods ( .elementHandle() <https://playwright.dev/docs/api/class-locator#locator-element-handle>) to convert them into ElementHandles, but that does require an additional await. Another idea that just occurred to me... maybe this is silly, but make the locator-based queries available at [query].locator? Standalone API import { webkit, selectors } from 'playwright';import { getDocument, queries, registerSelectorEngine } from 'playwright-testing-library'; const browser = await webkit.launch()const page = await browser.newPage() // ... we could probably even do this lazily/on-demand, right?await registerSelectorEngine(selectors); // ElementHandle-based queries (current API) const document = await getDocument(page);const elementHandle = queries.getByLabelText(document, 'Label'); // Locator-based API (delegates to selector engine internally, serializing options, etc.) const locator = queries.getByLabelText.locator(page, 'Label'); Fixture API import { test as baseTest, selectors } from ***@***.***/test';import { extend } from ***@***.***/test/fixture' // New `extend` method like your `mixin` version, could also register selectors or we could do it on demandconst test = extend({ test: baseTest, selectors }); const { expect } = test // ElementHandle-based queries (current API) test('my form', ({ queries: { getByLabelText } }) => { const elementHandle = await getByLabelText('Label') // ...}) // Locator-based API test('my form', ({ queries: { getByLabelText } }) => { // Locator-based API (delegates to selector engine internally, serializing options, etc.) const locator = await getByLabelText.locator('Label') // ...}) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#330 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3AXQGS2NY65UYBKJPRVPTUKB45HANCNFSM5HBJ4Q5A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. |
| Hi, I just wanted to add that it seems like Playwright is considering implementing a "role" selector, based on testing-library byRole: microsoft/playwright#11182 (the issue was added by a Playwright maintainer three days ago). |
| @pkerschbaum thanks for the heads up, the original thread was relevant as well. |
| I took a slightly different approach and implemented page.locator('role=button[name=/my button/i]');The syntax is similar to the built-in React selectors and Vue selectors. You can chain selectors with Ideally, this will be best to be implemented natively in Playwright. However, I think the experience is really close to being vanilla, and I really look forward to what the core team could improve 😍 . For now, feel free to give it a try or leave feedback in the issues. Keep in mind that it's still experimental and the API could change at any time though 😅 . |
| I'm gonna close this in favor of #403, but please head over there and let me know if something more like this would be more useful. |
After some struggle with the not that extensive documentation on adding a selector engine I finally got this working. From the first try of just using queries I saw that there are some other issues requiring 2 packages &c but hopefully that can be resolved.
I really really like the approach of using the selector engines, that it works with chaining (>>) and such things is extremely neat. I also like that by using the a selector engine we can keep the test in a more "playwrighty" style, only using the queryByAll queries and letting playwright handle errors when you want to click a locator that matches multiple elements.
Things of note:
Thank you @sebinsua for the motivation to do this!