Add Playwright driver (no deps) #145

ftes · 2024-10-27T20:03:53Z

Add Playwright test driver.
It communicates with the playwright node.js server via a Port.
No additional dependencies.

Details

bump to elixir 1.18 to use parameterized ExUnit tests (run live/static tests also with playwright)
add minimal playwright protocol (communicate with playwright node.js server via Port)
add `PhoenixTest.Playwright driver (very close to feature parity, some known inconsistencies that are documented via tests)
add PhoenixTest.Case to allow easily switching drivers via @tag :playwright
add optional deps ecto + postgres to test sandboxing of concurrent browser tests

Out of scope (just document how to)

installing: nodejs, npm package playwright, playwright browser(s)

lib/phoenix_test/playwright/connection.ex

test/test_helper.exs

ftes · 2024-10-28T18:27:41Z

In my eyes this is the most promising approach of the three so far (#136 , #74 #145).

The amount of code required to interface with playwright via a Port is fairly small.
And this gives us the flexibility to tailor the exact behaviour to PhoenixTest.

@germsvel If you have time I'd appreciate hearing your thoughts.

lib/phoenix_test/playwright.ex

test/phoenix_test/playwright_test.exs

test/phoenix_test/assertions_test.exs

ftes · 2024-10-30T06:43:42Z

test/phoenix_test/live_test.exs

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


Workaround for #142

ftes · 2024-10-30T06:43:55Z

test/phoenix_test/live_test.exs

      conn
      |> visit("/live/index")
-      |> fill_in("Email", with: "someone@example.com")
-      |> assert_has(input(label: "Email", value: "someone@example.com"))
+      |> within("#email-form", fn session ->


Workaround for #142

ftes · 2024-10-30T06:44:07Z

test/phoenix_test/live_test.exs

      conn
      |> visit("/live/index")
-      |> fill_in("Email", with: "someone@example.com")
+      |> within("#email-form", fn session ->


Workaround for #142

ftes · 2024-10-30T06:44:15Z

test/phoenix_test/live_test.exs

      conn
      |> visit("/live/index")
-      |> fill_in("Email", with: "frodo@example.com")
+      |> within("#email-form", fn session ->


Workaround for #142

ftes · 2024-10-30T11:33:29Z

.github/workflows/ci.yml

    - name: Run tests
-      run: mix test
+      run: "mix test || if [[ $? = 2 ]]; then mix test --failed; else false; fi" 


Browser (network) based tests tend to be flaky.

Options:

Increase timeouts -> test suite takes longer to run (every refute has to wait for time out)

Keep timeout small, re run failed tests

I opted for 2.

ftes · 2024-10-30T16:05:41Z

test/phoenix_test/live_test.exs

+      conn
+      |> visit("/live/index")
+      |> click_button("Change page title")
+      |> assert_has("title", text: "Title changed!")


Using render_page_title fails when running via Playwright.
Since the live title patch is not visible immediately in the browser.

Rather, use assert_has, which uses a retry mechanism.

ftes · 2024-10-30T16:06:57Z

.tool-versions

@@ -1,2 +1,2 @@
 erlang 27.0
-elixir 1.17.2-otp-27
+elixir main


We need the 1.18 parameterized tests!

ftes · 2024-10-30T16:07:35Z

lib/phoenix_test.ex

@@ -230,6 +230,10 @@ defmodule PhoenixTest do
    end
  end

+  def visit(driver, path) when is_struct(driver) do


This seems like the easiest, extensible way of adding custom drivers.

Yeah, seems sensible to me. 👍

ftes · 2024-10-30T16:17:11Z

test/phoenix_test/live_test.exs

@@ -299,7 +304,7 @@ defmodule PhoenixTest.LiveTest do
      |> within("#email-form", fn session ->
        fill_in(session, "Email", with: "someone@example.com")
      end)
-      |> assert_has(input(label: "Email", value: "someone@example.com"))
+      |> assert_has("#form-data", text: "email: someone@example.com")


Was a bad test:

The input is not controlled (value not set by server).
When a user inputs a value in the browser, the HTML value attribute is not changed.

Fix: Assert on the server-rendered #form-data instead.

ftes · 2024-11-24T10:16:42Z

Progress update:

I've slowly been converting Wallaby tests in a production project to PhoenixTest + Playwright.

I've added some more features along the way (especially: record a Playwright trace for failing tests)
The foundation of this PR seems to be pretty solid.
Feedback from more users would be good.

I'm debating how to continue this work.
@germsvel you seem to be pretty tight on time atm.
So I'll keep my work separate for the time being.

@ Everyone: If you're interested in using this, let me know.
Then I'll consider publishing the standalone Playwright driver to github or hex.pm.

germsvel · 2024-11-24T19:35:58Z

@ftes love the progress! I'm sorry I haven't had the time to review the PR. Because of the size of it, I need a good few hours to review it properly and give helpful feedback.

Something I've been thinking about: what if we make the protocol layer open for external libraries? We would have to change some of the protocol so it can be fully implemented by external libraries, but I think that would allow us to build other adapters at will. So, for example, you could add this adapter as an external library (e.g. phoenix_test_playwright or something like that), and I don't have to be the blocker to all this good work.

What do you think?

ftes · 2024-11-25T07:55:35Z

what if we make the protocol layer open for external libraries

I think that's a great idea - as long as it doesn't make PhoenixTest too complicated.

We could start by listing necessary changes (new issue?):

support visit/2 for custom drivers (e.g. Support visit/2 for custom drivers #149)
let driver choose default selector/locator (e.g. don't generate default Locators.Button in PhoenixTest.click_button)

germsvel

@ftes I haven't finished reviewing the full PR (I haven't looked at the test changes), but hopefully this is a good start (regardless of whether this comes directly into PhoenixTest or we make it a separate library).

Seeing how advanced this is, I'm open to including it directly into PhoenixTest (if you want to do that). I have an alternative solution in progress using Chrome Driver Protocol directly, but in that solution, we still have to provide the path to Chromium. So, if we're going to provide the path to a binary/cli thing, then maybe it makes more sense to just go full on with playwright.

If you prefer to keep it separate (which I also totally understand. Might be easier to move faster), I'd be happy for us to work more towards making the Driver protocol open for that.

One thing that I'm not super clear on yet is -- how close to feature parity is this? Do you consider it ready to merge ASAP, and we're good to go? Or are there things you'd like to finish before this could become part of PhoenixTest?

Any and all thoughts welcome!

germsvel · 2024-12-06T09:44:20Z

config/test.exs

+  otp_app: :phoenix_test,
+  playwright: [
+    browser: [browser: :chromium, headless: System.get_env("PLAYWRIGHT_HEADLESS", "t") in ~w(t true)],
+    cli: "priv/static/assets/node_modules/playwright/cli.js",


What version of playwright are we using/supporting? I imagine we're testing with a specific version?

Good question. I honestly have no idea how stable the playwright protocol is.
Since its considered internal, there probably are no strong guarantees, so its difficult to say.

germsvel · 2024-12-06T09:45:01Z

config/test.exs

+  ecto_repos: [PhoenixTest.Repo],
+  otp_app: :phoenix_test,
+  playwright: [
+    browser: [browser: :chromium, headless: System.get_env("PLAYWRIGHT_HEADLESS", "t") in ~w(t true)],


👍 nice to have this out of the box.

germsvel · 2024-12-06T09:45:18Z

lib/phoenix_test.ex

@@ -230,6 +230,10 @@ defmodule PhoenixTest do
    end
  end

+  def visit(driver, path) when is_struct(driver) do


Yeah, seems sensible to me. 👍

germsvel · 2024-12-06T09:49:29Z

lib/phoenix_test/case.ex

+  setup_all context do
+    trace = Application.fetch_env!(:phoenix_test, :playwright)[:trace]
+
+    case context do
+      %{playwright: true} ->
+        [browser_id: Case.Playwright.launch_browser(@playwright_opts), trace: trace]
+
+      %{playwright: opts} when is_list(opts) ->
+        opts = Keyword.merge(@playwright_opts, opts)
+        [browser_id: Case.Playwright.launch_browser(opts), trace: trace]


Since this is in a setup_all. Does this mean that we'll launch Playwright browser once so long as any test uses playwright?

Yes, exactly. The browser is only launched once.
And then one session within that browser per test (think new user profile + window).

And Connection.ensure_started provides lazy launching of the playwright node.js server, so it is only started if actually required by a test.

germsvel · 2024-12-06T09:50:16Z

lib/phoenix_test/case.ex

+
+  defmodule Playwright do
+    @moduledoc false
+    import PhoenixTest.Playwright.Connection


Don't have to change, but might be easier to follow the code below if we alias Connection instead of importing it here.

germsvel · 2024-12-06T10:19:35Z

lib/phoenix_test/playwright/frame.ex

@@ -0,0 +1,125 @@
+defmodule PhoenixTest.Playwright.Frame do


Nice module! 👍

germsvel · 2024-12-06T10:20:21Z

lib/phoenix_test/playwright/message.ex

+  @moduledoc """
+  Parse playwright messages.
+  A single `Port` message can contain multiple Playwright messages and/or a fraction of a message.
+  Such a message fraction is stored in `bufffer` and continued in the next `Port` message.


Tiny typo. Thank for the comment! Helpful to know why we need this.

Suggested change

Such a message fraction is stored in `bufffer` and continued in the next `Port` message.

Such a message fraction is stored in `buffer` and continued in the next `Port` message.

germsvel · 2024-12-06T10:21:58Z

lib/phoenix_test/playwright/port.ex

+    end
+
+    cmd = "run-driver"
+    port = Port.open({:spawn, "#{cli} #{cmd}"}, [:binary])


I asked this before in the PR, but worth asking here again: have you run into any zombie OS processes?

I know the Port docs mention a fix around it. I've used it in the past, and it worked quite well. Not sure if we need it here, but just pointing it out in case it's helpful.

I haven't seen any yet.
In general my experience with playwright has been far better than with Wallaby.
The playwright server seems to take better care of terminating the browser if a test is cancelled mid-run.

I'll keep my eyes open.

germsvel · 2024-12-06T10:25:28Z

lib/phoenix_test/playwright/port.ex

+      {k, v} when is_map(v) -> {String.to_atom(k), atom_keys(v)}
+      {k, list} when is_list(list) -> {String.to_atom(k), Enum.map(list, fn v -> atom_keys(v) end)}
+      {k, v} -> {String.to_atom(k), v}


Any concerns here with using String.to_atom/1? It's typically not a good idea with data we don't control, right? Are we at risk of hitting the max number of atoms at some point? And could we use to_exiting_atom/1 instead? Or do we do some other type of mapping?

Good question.
Since we're in a context of tests, I wouldn't worry too much.
We're not talking about a long-running system.

I think the keys we're converting to atoms are all playwright defined, i.e. there's a limited well-defined amount.
But since I don't know all playwright message types I can't say for sure whether or not e.g. some "user-defined" HTML node attribute might not pop up as a nested message key somewhere.

But even then, we're talking about tests and "user provided" would (mostly) mean "provided by the test set up".

I wouldn't want to use String.to_existing_atom because I'm worried that users might get ArgumentErrors if they trigger playwright messages with new keys (e.g. because they're using unwrap in ways that triggers new message types).

The safe way would be to keep everything as strings, but that would just make the code so much more verbose.

germsvel · 2024-12-06T10:28:22Z

lib/phoenix_test/playwright/selector.ex

+  def unquote(:and)(left, :none), do: left
+  def unquote(:and)(left, right), do: concat(left, "internal:and=#{Jason.encode!(right)}")


Is there a different function name we could use here so it doesn't conflict with and, and so we don't have to unquote here? Or do you think it's worth it so we can have the 1-1 mapping with Playwright? (I think that's why we're doing it, right?)

Yes, I tried to keep the Selector, Frame modules as minimal playwright wrappers which ideally reflect playwright concepts 1:1.

We could always go with and_ or something in that direction.

ftes · 2024-12-06T11:03:46Z

how close to feature parity is this

Good question! I've been using a copy of this for some weeks in a client project now.
A few small things have changed after migrating dozens of Wallaby tests.

What I've gotten rid of there is the calls to Assertions.
In this PR I tried to keep the exact same interface - including generating the exact same errors.
While this provides the best user experience, it also introduces more coupling that might make it more difficult to change code in future (no matter if the playwright driver is internal or external).

So the driver is definitely usable.
But I expect changes as soon as further people use it.

I'm undecided:

Pro internal: Running the existing test suite against the Playwright driver is great to ensure consistent behaviour
Con internal: As mentioned above, I'm unsure whether trying to mimicLive/Static drivers 100% is a good goal. In some places (e.g. role-based selectors), it is easiest and most efficient to use existing Playwright concepts. Trying to be 100% compatible precludes that option. Keeping things separate makes that dividing line more apparent.

Taking everything into consideration, I think it might be best to keep it separate.
And ensure that PhoenixTest is external-driver-friendly.

Excited for the vanilla CDP driver! It will be great to compare and see the different tradeoffs.

ftes · 2024-12-09T19:57:19Z

I've published the current version:
https://hexdocs.pm/phoenix_test_playwright

To anyone interested:
I'm very grateful for any feedback 🙏

ftes · 2024-12-13T11:42:22Z

Closing this for now.

ftes force-pushed the feature/playwright-no-dep branch 3 times, most recently from 4be4ce7 to 01d51d4 Compare October 27, 2024 20:19

ftes commented Oct 27, 2024

View reviewed changes

lib/phoenix_test/playwright/connection.ex Outdated Show resolved Hide resolved

ftes force-pushed the feature/playwright-no-dep branch 4 times, most recently from 8d6a378 to bcb492f Compare October 28, 2024 04:35

ftes commented Oct 28, 2024

View reviewed changes

lib/phoenix_test/playwright/connection.ex Outdated Show resolved Hide resolved

ftes commented Oct 28, 2024

View reviewed changes

test/test_helper.exs Outdated Show resolved Hide resolved

ftes force-pushed the feature/playwright-no-dep branch from bcb492f to fbfd376 Compare October 28, 2024 18:22

ftes marked this pull request as ready for review October 28, 2024 18:24

ftes force-pushed the feature/playwright-no-dep branch from fbfd376 to ee8bd29 Compare October 28, 2024 19:45

ftes commented Oct 28, 2024

View reviewed changes

lib/phoenix_test/playwright.ex Outdated Show resolved Hide resolved

ftes force-pushed the feature/playwright-no-dep branch 2 times, most recently from 9d399d5 to 6946294 Compare October 29, 2024 05:28

ftes commented Oct 29, 2024

View reviewed changes

test/phoenix_test/playwright_test.exs Outdated Show resolved Hide resolved

ftes commented Oct 29, 2024

View reviewed changes

test/phoenix_test/playwright_test.exs Show resolved Hide resolved

ftes commented Oct 29, 2024

View reviewed changes

test/phoenix_test/assertions_test.exs Outdated Show resolved Hide resolved

ftes commented Oct 29, 2024

View reviewed changes

test/phoenix_test/assertions_test.exs Outdated Show resolved Hide resolved

ftes force-pushed the feature/playwright-no-dep branch 5 times, most recently from 842463a to 30ad889 Compare October 29, 2024 20:23

ftes commented Oct 30, 2024

View reviewed changes

ftes force-pushed the feature/playwright-no-dep branch from 84e83f0 to a2eb819 Compare October 30, 2024 11:38

ftes commented Oct 30, 2024

View reviewed changes

ftes force-pushed the feature/playwright-no-dep branch from a2eb819 to 213e241 Compare October 30, 2024 16:21

ftes changed the title ~~Add Playwright driver (no playwright-elixir)~~ Add Playwright driver (no deps) Oct 31, 2024

This was referenced Oct 31, 2024

Add Playwright driver #136

Closed

Add Wallaby driver #74

Closed

ftes force-pushed the feature/playwright-no-dep branch 2 times, most recently from 75c51ba to 400397c Compare November 1, 2024 06:48

Retry failed tests, retry with configurable timeout

48bb144

ftes force-pushed the feature/playwright-no-dep branch 5 times, most recently from 8c9cbe7 to c0d57bf Compare November 4, 2024 07:43

Test Ecto sandbox

f16ab9d

ftes force-pushed the feature/playwright-no-dep branch from c0d57bf to f16ab9d Compare November 5, 2024 07:27

Add tracing

e16c76c

Install browsers --only-shell

61cc852

germsvel reviewed Dec 6, 2024

View reviewed changes

ftes closed this Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Playwright driver (no deps) #145

Add Playwright driver (no deps) #145

ftes commented Oct 27, 2024 •

edited

Loading

ftes commented Oct 28, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

ftes Oct 30, 2024

germsvel Dec 6, 2024

ftes Oct 30, 2024

ftes commented Nov 24, 2024 •

edited

Loading

germsvel commented Nov 24, 2024

ftes commented Nov 25, 2024 •

edited

Loading

germsvel left a comment

germsvel Dec 6, 2024

ftes Dec 6, 2024

germsvel Dec 6, 2024

germsvel Dec 6, 2024

germsvel Dec 6, 2024

ftes Dec 6, 2024

germsvel Dec 6, 2024

germsvel Dec 6, 2024

germsvel Dec 6, 2024

germsvel Dec 6, 2024

ftes Dec 6, 2024

germsvel Dec 6, 2024

ftes Dec 6, 2024

germsvel Dec 6, 2024

ftes Dec 6, 2024

ftes commented Dec 6, 2024

ftes commented Dec 9, 2024

ftes commented Dec 13, 2024

	Such a message fraction is stored in `bufffer` and continued in the next `Port` message.
	Such a message fraction is stored in `buffer` and continued in the next `Port` message.

		def unquote(:and)(left, :none), do: left
		def unquote(:and)(left, right), do: concat(left, "internal:and=#{Jason.encode!(right)}")

Add Playwright driver (no deps) #145

Add Playwright driver (no deps) #145

Conversation

ftes commented Oct 27, 2024 • edited Loading

Details

Out of scope (just document how to)

ftes commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ftes commented Nov 24, 2024 • edited Loading

germsvel commented Nov 24, 2024

ftes commented Nov 25, 2024 • edited Loading

germsvel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ftes commented Dec 6, 2024

ftes commented Dec 9, 2024

ftes commented Dec 13, 2024

ftes commented Oct 27, 2024 •

edited

Loading

ftes commented Nov 24, 2024 •

edited

Loading

ftes commented Nov 25, 2024 •

edited

Loading