✨ Add support for find queries in locator fixture #488

jrolfs · 2022-09-01T04:55:08Z

~~Refactors~~ Merged in #489

Isolate 'standard' page (no timeout) in describe blocks

Use queries from Testing Library to enumerate queries (instead of internal hard-coded query name array, with the intention of removing all of the legacy ElementHandle implementation in a future major release)

lib/fixture/locator/helpers.ts

sebinsua · 2022-09-01T09:49:00Z

test/fixture/locators.test.ts

+      expect(text).toEqual(['Hello h1', 'Hello h2'])
+    })
+
+    test('throws Testing Library error when locator times out', async ({queries}) => {


Is there any reason why we called this queries and not screen?

@gajus seemed to expose the methods on Page itself. Would this be possible? Are we against doing this for some reason?

Both things I have considered... I wasn't sure how much benefit we'd get from attaching stuff to page. From your experience, would this be convenient? We could attach within as well.

That said, I do like how switching to screen would resemble TL. Playwright's Page is like a real "screen," though.

I quite like screen for familiarity with Testing Library. And, to be honest, I would prefer to not add any methods to Page as I think this would be unclear (e.g. "why do these methods exist when they're not on the Playwright website?!").

We adopted the pattern I showed quite without regrets, for what it is worth.

I find that reducing verbosity in tests is extremely important to reduce test fatigue. These things add up

Another idea -- could we leave page untouched but make screen itself be Queries & Page, so it is clear that you are using Testing Library but you do not need to juggle both?

That makes it independent but also reduces verbosity in tests.

Are there issues with doing this? What happens if there is a method that returns a Page? Is there anything we can do to avoid people needing to pass the Locators we produce into within(...) in order to make the Testing Library queries available again?

In practice, being able to chain .locator(...) as you can do in Playwright is quite helpful. My code got more explicit but worse when I used within(...). 😅

(Thinking aloud to see if other's have opinions.)

Ooo, I quite like that idea re: screen + page. I think I'll probably go that route.

As for your question, we could use a similar approach to augment the Locator returned from queries with the query methods (so you could chain), but my concern there is departing from the Testing Library API.

I'm think gonna address this stuff in a separate PR.

lib/fixture/locator/helpers.ts

sebinsua · 2022-09-01T10:04:33Z

test/fixture/locators.test.ts

+
+      expect(text).toEqual(['Hello h1', 'Hello h2'])
+    })
+


If we're going to support Locator#waitFor.state we should add an extra <div> element with a style of visibility: hidden into fixtures/late-page.html which is not added by a setTimeout and is already attached to <body>. That way we could write a test of this passing in state: 'hidden' and state: 'attached' to check the behaviour.

Does it make any sense to support hidden and detached just because waitFor supports them? playwright.dev/docs/api/class-locator#locator-wait-for
Right now I'm snagging the type from Playwright, but I'm thinking visible and attached really only make sense https://github.com/testing-library/playwright-testing-library/pull/488/files#diff-04543a94352d1484d0a0add940de7e7b20b0e8d7d58ca1e3aa8d705ba7de95cdR55-R58

Related: waitForElementToBeRemoved

I went ahead and removed hidden and detached from the types for now. Unfortunately our follow-up getBy query for the Testing Library error kinda breaks down when the element is attached but not visible because the getBy query doesn't throw an error in that case 😑.

lib/fixture/locator/helpers.ts

sebinsua · 2022-09-01T10:20:25Z

This all looks good to me. I wrote some comments with my understanding of things! (I also wrote further comments within the GitHub issue.)

jrolfs · 2022-09-02T08:11:02Z

lib/fixture/locator/helpers.ts

+            )}`,
+          )
+          .first()
+          .waitFor({state, timeout: 100})


Bleh, this hard-coded timeout is to handle the hidden element case I mentioned where getBy* doesn't throw 😑.

jrolfs · 2022-09-02T08:16:56Z

I think this may be in a reasonable spot to release as another beta, assuming I can sort whatever's going on with Semantic Release. Worst case we switch to next 🙃 (or maybe alpha, heh)

sebinsua · 2022-09-02T11:08:08Z

Can you expose queriesFor when you publish a new version?

(until we have an official API for \`Locator\` queries that doesn't require a **@playwright/test** fixture)

github-actions · 2022-09-03T05:15:20Z

🎉 This PR is included in version 4.4.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

jrolfs self-assigned this Sep 1, 2022

jrolfs requested a review from sebinsua September 1, 2022 04:55

jrolfs force-pushed the feature/locator-find-queries branch from d5c4eea to ac532bf Compare September 1, 2022 04:55

jrolfs force-pushed the beta branch from 3801877 to 907586b Compare September 1, 2022 04:56

jrolfs force-pushed the feature/locator-find-queries branch 2 times, most recently from 5602bd7 to 61ab503 Compare September 1, 2022 06:02

This was referenced Sep 1, 2022

Expect assertions can't be used due to returning ElementHandles instead of Locators #430

Closed

♻️ Refactors from locator find query branch #489

Merged

jrolfs force-pushed the feature/locator-find-queries branch from 61ab503 to 0aff077 Compare September 1, 2022 06:19

sebinsua reviewed Sep 1, 2022

View reviewed changes

lib/fixture/locator/helpers.ts Outdated Show resolved Hide resolved

sebinsua reviewed Sep 1, 2022

View reviewed changes

lib/fixture/locator/helpers.ts Outdated Show resolved Hide resolved

sebinsua reviewed Sep 1, 2022

View reviewed changes

lib/fixture/locator/helpers.ts Outdated Show resolved Hide resolved

sebinsua reviewed Sep 1, 2022

View reviewed changes

lib/fixture/locator/helpers.ts Outdated Show resolved Hide resolved

jrolfs force-pushed the beta branch from 43a2716 to bd7fb5c Compare September 1, 2022 20:34

jrolfs force-pushed the feature/locator-find-queries branch 3 times, most recently from ce9630e to c6a148c Compare September 2, 2022 08:04

jrolfs marked this pull request as ready for review September 2, 2022 08:07

jrolfs force-pushed the feature/locator-find-queries branch 2 times, most recently from cb567cb to 739e085 Compare September 2, 2022 08:09

jrolfs commented Sep 2, 2022

View reviewed changes

jrolfs requested a review from sebinsua September 2, 2022 08:15

sebinsua approved these changes Sep 2, 2022

View reviewed changes

jrolfs added 2 commits September 2, 2022 18:09

feat(fixture): add support for find* queries in locator fixture

53fc2ce

docs(readme): include find queries in readme description

741de81

jrolfs added 2 commits September 2, 2022 18:15

feat(fixture): expose unofficial queriesFor helper

877b043

(until we have an official API for \`Locator\` queries that doesn't require a **@playwright/test** fixture)

docs: add inline documentation for configure

a9cb165

jrolfs force-pushed the feature/locator-find-queries branch from 739e085 to a9cb165 Compare September 3, 2022 01:16

jrolfs merged commit f01deb4 into beta Sep 3, 2022

jrolfs deleted the feature/locator-find-queries branch September 3, 2022 01:37

github-actions bot added the released on @alpha label Sep 3, 2022

jrolfs mentioned this pull request Sep 8, 2022

✨ New locator based fixtures (4.4.0-beta.6) #431

Merged

✨ Add support for find queries in locator fixture #488

✨ Add support for find queries in locator fixture #488

Uh oh!

Conversation

jrolfs commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactors Merged in #489

Uh oh!

Uh oh!

sebinsua Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

jrolfs Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebinsua Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

gajus Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

sebinsua Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrolfs Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

jrolfs Sep 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sebinsua Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrolfs Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrolfs Sep 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sebinsua commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrolfs Sep 2, 2022

Choose a reason for hiding this comment

Uh oh!

jrolfs commented Sep 2, 2022

Uh oh!

sebinsua commented Sep 2, 2022

Uh oh!

github-actions bot commented Sep 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jrolfs commented Sep 1, 2022 •

edited

Loading

jrolfs Sep 1, 2022 •

edited

Loading

sebinsua Sep 1, 2022 •

edited

Loading

sebinsua Sep 1, 2022 •

edited

Loading

jrolfs Sep 2, 2022 •

edited

Loading

sebinsua commented Sep 1, 2022 •

edited

Loading