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

Add Playwright driver #136

Closed
wants to merge 1 commit into from
Closed

Add Playwright driver #136

wants to merge 1 commit into from

Conversation

ftes
Copy link
Contributor

@ftes ftes commented Oct 6, 2024

@germsvel I know you're working on this anyway. I'm just leaving this here in case you want to have a look.

I wanted to play around with playwright a bit to get more input for my upcoming CodeBEAM talk.
Fairly smooth sailing so far. I see you've been active in playwright-elixir getting things ready 🙂 .

@@ -66,13 +66,13 @@ defmodule PhoenixTest.PageView do
data-to="/page/delete_record"
data-csrf="sometoken"
>
Data-method Delete
Data_method Delete
Copy link
Contributor Author

@ftes ftes Oct 6, 2024

Choose a reason for hiding this comment

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

Playwright treats exact: false as case insensitive. PhoenixTest remains case sensitive.

@ftes ftes force-pushed the feature/playwright branch from d035d38 to bfa8f06 Compare October 6, 2024 09:28
@ftes ftes mentioned this pull request Oct 6, 2024
@ftes ftes force-pushed the feature/playwright branch 3 times, most recently from b983204 to ca83bfb Compare October 6, 2024 18:26
@ftes ftes changed the title Add playwright driver Add Playwright driver Oct 6, 2024
lib/phoenix_test/playwright.ex Outdated Show resolved Hide resolved
config/runtime.exs Outdated Show resolved Hide resolved
page = Playwright.Browser.new_page(context.browser)
ExUnit.Callbacks.on_exit(fn -> Playwright.Page.close(page) end)

[conn: page]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just overwrites the conn context field with a playwright page.

Probably not the best way of doing this.
Just chose this as it was minimally invasive.

@@ -0,0 +1,66 @@
defmodule PhoenixTest.Case do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly copied from PlaywrightTest.Case

|> fill_in("Email", with: nil)
|> within("#email-form", fn session ->
session
|> fill_in("Email", with: "email")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add text first, them remove.
Otherwise this is a no-op when using playwright.

conn
|> visit("/live/index")
|> fill_in("Email", with: "some@example.com")
|> within("#email-form", fn session ->
Copy link
Contributor Author

@ftes ftes Oct 24, 2024

Choose a reason for hiding this comment

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

Workaround for #142

Implicit and explicit labels not treated equally by Live and Static drivers: explicit wins, no error is.

But Playwright treats them equally and raises an error.

<label for="email">Email</label>
<input id="email" name="email" />

<label>Email<input type="email" name="email" /></label>

@ftes ftes force-pushed the feature/playwright branch 3 times, most recently from 2b617c1 to 47160e7 Compare October 24, 2024 07:29
@@ -69,5 +69,8 @@ jobs:
- name: Check Formatting
run: mix format --check-formatted

- name: Install playwright browsers
run: mix playwright.install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cache?

@ftes ftes force-pushed the feature/playwright branch 2 times, most recently from 2823f40 to c7402bf Compare October 24, 2024 07:43
@@ -0,0 +1,66 @@
defmodule PhoenixTest.Case do
@moduledoc false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add docs (API tbd)

use PhoenixTest.Case,
  playwright: chromium,
  headless: false,
  slow_mo: 500

@ftes ftes force-pushed the feature/playwright branch from c7402bf to 7ff956e Compare October 24, 2024 07:47
@ftes
Copy link
Contributor Author

ftes commented Oct 24, 2024

mix playwright.install made set up in CI easy :) No caching yet tho.

@ftes
Copy link
Contributor Author

ftes commented Oct 24, 2024

I don't think we need to use much of what playwright-elixir offers (or will offer in the future).
Idea: Talk directly to the playwright server. Implement only the relevant parts of the Websocket protocol.
Maybe I'm being naive and playwright-elixir is already doing more heavy lifting relevant for this use case than I think.

Somewhat comparable to cuprite. Buth with playwright (server) as an additional abstraction (component) inbetween, instead of raw CDP.

@germsvel
Copy link
Owner

Talk directly to the playwright server. Implement only the relevant parts of the Websocket protocol.
Maybe I'm being naive and playwright-elixir is already doing more heavy lifting relevant for this use case than I think.

Somewhat comparable to cuprite. Buth with playwright (server) as an additional abstraction (component) inbetween, instead of raw CDP.

That's exactly where I landed too. I don't know if I'm misrepresenting what playwright-elixir is doing for us, but I like the idea of cuprite.

I started exploring that (even started creating a project to handle Elixir <-> CDP communication through websockets), but had to halt due to time constraints.

So... now, I don't know if we need Wallaby or Playwright. It would be cool (but not sure if a ton more work) to create an Elixir CDP wrapper that allows us to communicate with chrome (kind of like ferrum which cuprite uses). And then PhoenixTest just uses that.

@ftes
Copy link
Contributor Author

ftes commented Oct 25, 2024

Elixir CDP wrapper: bitcrowd/chromic_pdf might be a good starting point.

But I still think "vanilla" playwright (not via playwright-elixir) might also be a good option.
Higher-level abstraction (less for us to implement?), multi browser support out of the box, tooling (e.g. installing browsers).

@germsvel
Copy link
Owner

Elixir CDP wrapper: bitcrowd/chromic_pdf might be a good starting point.

Oh cool. I didn't know about that package. I'll take a look.

But I still think "vanilla" playwright (not via playwright-elixir) might also be a good option.
Higher-level abstraction (less for us to implement?), multi browser support out of the box, tooling (e.g. installing browsers).

Yeah, that's an interesting idea. I didn't realize that's what you had suggested at first (though, now I see it 😄 ). I like that idea too. I can see how we need maybe 1/10th of what playwright-elixir is trying to do. We could do it directly with playwright, and leverage what we already have in phoenix_test the way you have been thinking about implementing Wallaby and Playwright-Elixir (meaning, we rely on render_html a lot and then do assertions and stuff like that on our side)

@ftes
Copy link
Contributor Author

ftes commented Oct 31, 2024

Closing in favour of #145

@ftes ftes closed this Oct 31, 2024
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.

2 participants