From 2c3309c0cb758c6541399c9ddfd046b2fbbf5bcd Mon Sep 17 00:00:00 2001 From: German Velasco Date: Thu, 30 May 2024 14:11:47 -0400 Subject: [PATCH 1/8] Implement basic versions of Page.Keyboard What changed? ============= We implement a basic version of `Page.Keyboard` functions whose specs were commented out in the module. The test cases are the simplest versions I could come up with (having borrowed heavily from [Python's keyboard tests]) The implementation also follows [Python's implementations]. [Python's keyboard tests]: https://github.com/microsoft/playwright-python/blob/2402e1283de6049cb66ea8ec6f303ba681890d2a/tests/async/test_keyboard.py#L1 [Python's implementations]: https://github.com/microsoft/playwright-python/blob/2402e1283de6049cb66ea8ec6f303ba681890d2a/playwright/_impl/_input.py#L25-L38 Noteworthy Highlights ---------------------- 1. Our implementation takes a `%Page{}` struct rather than what the specs suggested (a `Keyboard.t()`). I don't know if it's possible to pass down a narrower type, but as far as I could tell `Page` had both the `session` and `guid` (which are needed for the `Channel.post` call). 2. We namespace `Playwright.Keyboard` under `Page` (i.e. `Playwright.Page.Keyboard`) to match the directory structure and since that seems to match the Playwright documentation as well. 3. Finally, we add a new `test/assets/` directory where we include a new `inputs/keyboard.html` page. That page is [copied directly from Python][keyboard.html], so it follows Python's style of testing. In particular, our tests needed the `getResult()` function to get the values back for our test assertions. [keyboard.html]: https://github.com/microsoft/playwright-python/blob/2402e1283de6049cb66ea8ec6f303ba681890d2a/tests/assets/input/keyboard.html#L1 --- lib/playwright/page/keyboard.ex | 38 ++++++++---- test/assets/input/keyboard.html | 42 +++++++++++++ test/integration/page/keyboard_test.exs | 78 +++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 test/assets/input/keyboard.html create mode 100644 test/integration/page/keyboard_test.exs diff --git a/lib/playwright/page/keyboard.ex b/lib/playwright/page/keyboard.ex index bb0c6ddb..3df6406e 100644 --- a/lib/playwright/page/keyboard.ex +++ b/lib/playwright/page/keyboard.ex @@ -1,19 +1,35 @@ -defmodule Playwright.Keyboard do +defmodule Playwright.Page.Keyboard do @moduledoc false - # @spec down(t(), binary()) :: :ok - # def down(keyboard, key) + alias Playwright.Page + alias Playwright.SDK.Channel - # @spec insert_text(t(), binary()) :: :ok - # def insert_text(keyboard, text) + @spec type(Page.t(), binary()) :: :ok + def type(page, text) do + channel_post(page, :keyboard_type, %{text: text}) + end - # @spec press(t(), binary(), options()) :: :ok - # def press(keyboard, key, options \\ %{}) + @spec insert_text(Page.t(), binary()) :: :ok + def insert_text(page, text) do + channel_post(page, :keyboard_type, %{text: text}) + end - # @spec type(t(), binary(), options()) :: :ok - # def type(keyboard, text, options \\ %{}) + @spec up(Page.t(), binary()) :: :ok + def up(page, key) do + channel_post(page, :keyboard_up, %{key: key}) + end - # @spec up(t(), binary()) :: :ok - # def up(keyboard, key) + @spec down(Page.t(), binary()) :: :ok + def down(page, key) do + channel_post(page, :keyboard_down, %{key: key}) + end + @spec press(Page.t(), binary()) :: :ok + def press(page, key) do + channel_post(page, :keyboard_press, %{key: key}) + end + + defp channel_post(%Page{} = page, action, data) do + Channel.post(page.session, {:guid, page.guid}, action, data) + end end diff --git a/test/assets/input/keyboard.html b/test/assets/input/keyboard.html new file mode 100644 index 00000000..4788b0d1 --- /dev/null +++ b/test/assets/input/keyboard.html @@ -0,0 +1,42 @@ + + + + Keyboard test + + + + + + diff --git a/test/integration/page/keyboard_test.exs b/test/integration/page/keyboard_test.exs new file mode 100644 index 00000000..fdd89512 --- /dev/null +++ b/test/integration/page/keyboard_test.exs @@ -0,0 +1,78 @@ +defmodule Playwright.Page.KeyboardTest do + use Playwright.TestCase, async: true + + alias Playwright.Page + alias Playwright.Page.Keyboard + + describe "type" do + test "keyboard type into a textbox", %{page: page} do + Page.evaluate(page, """ + const textarea = document.createElement('textarea'); + document.body.appendChild(textarea); + textarea.focus(); + """) + + text = "Hello world. I am the text that was typed!" + + Keyboard.type(page, text) + + assert Page.evaluate(page, ~s[document.querySelector("textarea").value]) == text + end + end + + describe "insert_text" do + test "should send characters inserted", %{page: page} do + Page.evaluate(page, """ + const textarea = document.createElement('textarea'); + document.body.appendChild(textarea); + textarea.focus(); + """) + + text = "Hello world. I am the text that was typed!" + + Keyboard.insert_text(page, text) + + assert Page.evaluate(page, ~s[document.querySelector("textarea").value]) == text + end + end + + describe "up" do + test "sends proper code", %{page: page, assets: assets} do + Page.goto(page, assets.prefix <> "/input/keyboard.html") + + Keyboard.up(page, ";") + + assert Page.evaluate(page, "() => getResult()") == + "Keyup: ; Semicolon 186 []" + end + end + + describe "down" do + test "sends proper code", %{page: page, assets: assets} do + Page.goto(page, assets.prefix <> "/input/keyboard.html") + + Keyboard.down(page, "Control") + + assert Page.evaluate(page, "() => getResult()") == + "Keydown: Control ControlLeft 17 [Control]" + end + end + + describe "press" do + test "test should press plus", %{page: page, assets: assets} do + Page.goto(page, assets.prefix <> "/input/keyboard.html") + + Keyboard.press(page, "+") + + assert Page.evaluate(page, "() => getResult()") == + [ + # 192 is ` keyCode + "Keydown: + Equal 187 []", + # 126 is ~ charCode + "Keypress: + Equal 43 43 []", + "Keyup: + Equal 187 []" + ] + |> Enum.join("\n") + end + end +end From d5787dc3a7dd8b1afc554e22b65318a6766703d5 Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:05:04 -0700 Subject: [PATCH 2/8] Adjust `Keyboard` to `use ...ChannelOwner` --- lib/playwright/page/keyboard.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/playwright/page/keyboard.ex b/lib/playwright/page/keyboard.ex index 3df6406e..e5dfe409 100644 --- a/lib/playwright/page/keyboard.ex +++ b/lib/playwright/page/keyboard.ex @@ -1,8 +1,8 @@ defmodule Playwright.Page.Keyboard do @moduledoc false + use Playwright.SDK.ChannelOwner alias Playwright.Page - alias Playwright.SDK.Channel @spec type(Page.t(), binary()) :: :ok def type(page, text) do From 225a14350c1cdcad2076ca368a6c8504de19cb2b Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:06:08 -0700 Subject: [PATCH 3/8] Tidy: alpha-sort & group functions --- lib/playwright/page/keyboard.ex | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/playwright/page/keyboard.ex b/lib/playwright/page/keyboard.ex index e5dfe409..940f23a9 100644 --- a/lib/playwright/page/keyboard.ex +++ b/lib/playwright/page/keyboard.ex @@ -4,9 +4,12 @@ defmodule Playwright.Page.Keyboard do use Playwright.SDK.ChannelOwner alias Playwright.Page - @spec type(Page.t(), binary()) :: :ok - def type(page, text) do - channel_post(page, :keyboard_type, %{text: text}) + # API + # --------------------------------------------------------------------------- + + @spec down(Page.t(), binary()) :: :ok + def down(page, key) do + channel_post(page, :keyboard_down, %{key: key}) end @spec insert_text(Page.t(), binary()) :: :ok @@ -14,21 +17,24 @@ defmodule Playwright.Page.Keyboard do channel_post(page, :keyboard_type, %{text: text}) end - @spec up(Page.t(), binary()) :: :ok - def up(page, key) do - channel_post(page, :keyboard_up, %{key: key}) + @spec press(Page.t(), binary()) :: :ok + def press(page, key) do + channel_post(page, :keyboard_press, %{key: key}) end - @spec down(Page.t(), binary()) :: :ok - def down(page, key) do - channel_post(page, :keyboard_down, %{key: key}) + @spec type(Page.t(), binary()) :: :ok + def type(page, text) do + channel_post(page, :keyboard_type, %{text: text}) end - @spec press(Page.t(), binary()) :: :ok - def press(page, key) do - channel_post(page, :keyboard_press, %{key: key}) + @spec up(Page.t(), binary()) :: :ok + def up(page, key) do + channel_post(page, :keyboard_up, %{key: key}) end + # private + # --------------------------------------------------------------------------- + defp channel_post(%Page{} = page, action, data) do Channel.post(page.session, {:guid, page.guid}, action, data) end From e9aea6955d08e85f9f4e98d2975a99375583635f Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:06:32 -0700 Subject: [PATCH 4/8] Tidy/fix: Remove `keyboard.html` test asset A reasonable idea, but unnecessary: The [`playwright-assets`](https://github.com/mechanical-orchard/playwright-assets) dependency provides all of the test assets. Indeed, the `keyboard.html` file that was being utilized in the new keyboard tests is that from the `playwright-assets` server, and not this (removed) file in `test/assets`. --- test/assets/input/keyboard.html | 42 --------------------------------- 1 file changed, 42 deletions(-) delete mode 100644 test/assets/input/keyboard.html diff --git a/test/assets/input/keyboard.html b/test/assets/input/keyboard.html deleted file mode 100644 index 4788b0d1..00000000 --- a/test/assets/input/keyboard.html +++ /dev/null @@ -1,42 +0,0 @@ - - - - Keyboard test - - - - - - From f4b4be0bbf17c8072608c147ffb45fdb3a66bdb8 Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:18:03 -0700 Subject: [PATCH 5/8] Fix: Remove `npm install` from CI build For the time being, Node packages are installed directly into `priv/static`, and utilized from there. This is not the desired long-term solution, but is what is needed for the moment. --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c9eb5790..cdfee7a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,9 @@ jobs: otp-version: '26.2.5' - name: Install Elixir dependencies run: mix deps.get - - name: Install Node dependencies - run: npm install --prefix assets + # NOTE: not needed for now, while assets are + # directly installed to `priv/static`. + # - name: Install Node dependencies + # run: npm install --prefix assets - name: Run tests run: mix test From 5d7274dee496619e3624b363608da4acda0e7d8d Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:22:15 -0700 Subject: [PATCH 6/8] Fix: Add browser install step to CI --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdfee7a9..83fe3740 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,8 @@ jobs: otp-version: '26.2.5' - name: Install Elixir dependencies run: mix deps.get + - name: Install Playwright dependencies (e.g., browsers) + run: mix playwright.install # NOTE: not needed for now, while assets are # directly installed to `priv/static`. # - name: Install Node dependencies From bb2613c971e157003f5451efe9cb6e93f09d8a25 Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:27:08 -0700 Subject: [PATCH 7/8] Fix: `mix playwright.install` also installs deps --- lib/playwright/sdk/cli.ex | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/playwright/sdk/cli.ex b/lib/playwright/sdk/cli.ex index 7a1c2e41..c80881b2 100644 --- a/lib/playwright/sdk/cli.ex +++ b/lib/playwright/sdk/cli.ex @@ -6,21 +6,13 @@ defmodule Playwright.SDK.CLI do require Logger def install do - Logger.info("Installing playwright browsers") + Logger.info("Installing playwright browsers and dependencies") cli_path = config_cli() || default_cli() - {result, exit_status} = System.cmd(cli_path, ["install"]) + {result, exit_status} = System.cmd(cli_path, ["install", "--with-deps"]) Logger.info(result) if exit_status != 0, do: raise("Failed to install playwright browsers") end - def install_deps do - Logger.info("Installing playwright deps") - cli_path = config_cli() || default_cli() - {result, exit_status} = System.cmd(cli_path, ["installdeps"]) - Logger.info(result) - if exit_status != 0, do: raise("Failed to install playwright deps") - end - # private # ---------------------------------------------------------------------------- From 5b2171a5784c41a3a59dfea4966142b435343459 Mon Sep 17 00:00:00 2001 From: Corey Innis Date: Fri, 31 May 2024 00:31:54 -0700 Subject: [PATCH 8/8] Fix: bump test timeouts: 200ms -> 500ms 200ms allows for some flakiness. Most things will still finish faster. If the event doesn't happen in 500ms, that's probably interesting, finally. --- test/integration/locator_test.exs | 18 +++++++++--------- test/integration/page/expect_test.exs | 16 ++++++++-------- test/integration/page_test.exs | 6 +++--- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/test/integration/locator_test.exs b/test/integration/locator_test.exs index 36fff140..8b5da1ca 100644 --- a/test/integration/locator_test.exs +++ b/test/integration/locator_test.exs @@ -40,7 +40,7 @@ defmodule Playwright.LocatorTest do describe "Locator.check/2" do setup(%{assets: assets, page: page}) do - options = %{timeout: 200} + options = %{timeout: 500} page |> Page.goto(assets.prefix <> "/empty.html") page |> Page.set_content("") @@ -59,13 +59,13 @@ defmodule Playwright.LocatorTest do frame = Page.main_frame(page) locator = Locator.new(frame, "input#bogus") - assert {:error, %Error{message: "Timeout 200ms exceeded."}} = Locator.check(locator, options) + assert {:error, %Error{message: "Timeout 500ms exceeded."}} = Locator.check(locator, options) end end describe "Locator.click/2" do setup(%{assets: assets, page: page}) do - options = %{timeout: 200} + options = %{timeout: 500} page |> Page.goto(assets.prefix <> "/empty.html") page |> Page.set_content("yo") @@ -84,14 +84,14 @@ defmodule Playwright.LocatorTest do frame = Page.main_frame(page) locator = Locator.new(frame, "a#bogus") - assert {:error, %Error{message: "Timeout 200ms exceeded."}} = Locator.click(locator, options) + assert {:error, %Error{message: "Timeout 500ms exceeded."}} = Locator.click(locator, options) end test "clicking a button", %{assets: assets, page: page} do locator = Page.locator(page, "button") page |> Page.goto(assets.prefix <> "/input/button.html") - Locator.click(locator, %{timeout: 200}) + Locator.click(locator, %{timeout: 500}) assert Page.evaluate(page, "window['result']") == "Clicked" end end @@ -112,7 +112,7 @@ defmodule Playwright.LocatorTest do } """) - Locator.dblclick(locator, %{timeout: 200}) + Locator.dblclick(locator, %{timeout: 500}) assert Page.evaluate(page, "window['double']") == true assert Page.evaluate(page, "window['result']") == "Clicked" end @@ -694,7 +694,7 @@ defmodule Playwright.LocatorTest do describe "Locator.uncheck/2" do setup(%{assets: assets, page: page}) do - options = %{timeout: 200} + options = %{timeout: 500} page |> Page.goto(assets.prefix <> "/empty.html") page |> Page.set_content("") @@ -712,13 +712,13 @@ defmodule Playwright.LocatorTest do test "returns a timeout error when unable to 'uncheck'", %{options: options, page: page} do locator = Page.locator(page, "input#bogus") - assert {:error, %Error{message: "Timeout 200ms exceeded."}} = Locator.uncheck(locator, options) + assert {:error, %Error{message: "Timeout 500ms exceeded."}} = Locator.uncheck(locator, options) end end describe "Locator.wait_for/2" do setup(%{assets: assets, page: page}) do - options = %{timeout: 200} + options = %{timeout: 500} page |> Page.goto(assets.prefix <> "/empty.html") diff --git a/test/integration/page/expect_test.exs b/test/integration/page/expect_test.exs index 79068cd0..c8a3405b 100644 --- a/test/integration/page/expect_test.exs +++ b/test/integration/page/expect_test.exs @@ -37,10 +37,10 @@ defmodule Playwright.Page.NetworkTest do test "w/ an event and a timeout", %{page: page} do {:error, %Error{message: message}} = Page.expect_event(page, :request_finished, %{ - timeout: 200 + timeout: 500 }) - assert message == "Timeout 200ms exceeded." + assert message == "Timeout 500ms exceeded." end test "w/ an event, a (truthy) predicate, and a timeout", %{assets: assets, page: page} do @@ -51,7 +51,7 @@ defmodule Playwright.Page.NetworkTest do predicate: fn _, _ -> true end, - timeout: 200 + timeout: 500 }) assert event.type == :request_finished @@ -65,10 +65,10 @@ defmodule Playwright.Page.NetworkTest do predicate: fn _, _ -> false end, - timeout: 200 + timeout: 500 }) - assert message == "Timeout 200ms exceeded." + assert message == "Timeout 500ms exceeded." end end @@ -115,14 +115,14 @@ defmodule Playwright.Page.NetworkTest do predicate: fn _, _ -> false end, - timeout: 200 + timeout: 500 }, fn -> Page.goto(page, assets.empty) end ) - assert message == "Timeout 200ms exceeded." + assert message == "Timeout 500ms exceeded." end test "w/ an event and a timeout", %{assets: assets, page: page} do @@ -131,7 +131,7 @@ defmodule Playwright.Page.NetworkTest do page, :request_finished, %{ - timeout: 200 + timeout: 500 }, fn -> Page.goto(page, assets.empty) diff --git a/test/integration/page_test.exs b/test/integration/page_test.exs index 01635d4c..f417ed8f 100644 --- a/test/integration/page_test.exs +++ b/test/integration/page_test.exs @@ -220,8 +220,8 @@ defmodule Playwright.PageTest do test "with a single option given mismatched attributes, returns a timeout", %{assets: assets, page: page} do page |> Page.goto(assets.prefix <> "/input/select.html") - assert {:error, %Error{message: "Timeout 200ms exceeded."}} = - Page.select_option(page, "select", %{value: "green", label: "Brown"}, %{timeout: 200}) + assert {:error, %Error{message: "Timeout 500ms exceeded."}} = + Page.select_option(page, "select", %{value: "green", label: "Brown"}, %{timeout: 500}) end test "with multiple options and a single-option select, selects the first", %{assets: assets, page: page} do @@ -342,7 +342,7 @@ defmodule Playwright.PageTest do assert page |> Page.get_attribute("div#outer", "name") == "value" assert page |> Page.get_attribute("div#outer", "foo") == nil - assert({:error, %Error{}} = Page.get_attribute(page, "glorp", "foo", %{timeout: 200})) + assert({:error, %Error{}} = Page.get_attribute(page, "glorp", "foo", %{timeout: 500})) end end