Experimental selenium driver #198

aaronrenner · 2017-04-14T19:57:22Z

This is a work in progress to add selenium support and allows the existing test suite to be run against a local instance of geckodriver or chromedriver. This is definitely a work in progress, but will show us how the existing test suite runs against different webdriver backends.

aaronrenner · 2017-04-14T20:01:05Z

lib/wallaby/experimental/selenium.ex

+    base_url = Keyword.get(opts, :remote_url, "http://localhost:4444/wd/hub/")
+    capabilities = Keyword.get(opts, :capabilities, %{})
+    create_session_fn = Keyword.get(opts, :create_session_fn,
+                                    &Wallaby.Phantom.Driver.create_session/2)


This is still calling Wallaby.Phantom.Driver.create_session to make the http request to selenium to start the session. Do you think all of the webdriver http requests should live in a common module like in #195, or would you prefer if each driver implements its own http requests?

Ideally they could live in their own module but for now lets separate them. Once we have support for 3 drivers (phantomjs, selenium, chromedriver, etc.) then we can see if they share enough functionality to merge the driver implementations.

aaronrenner · 2017-04-14T23:40:09Z

I'd also be interested in how you'd like to see the driver modules designed. Should I be writing each one of the browser interaction functions (like visit, click, etc) in each module to make them easier to customize behavior per driver? Since chromedriver is different than geckodriver and phantomjs (and Appium for that matter), should I have one driver per backend? If so, do you think each module should do something like use Wallaby.WebDriver.Driver to share common boilerplate and then override functions when we need to change something? I'm not sure if using use Wallaby.WebDriver.Driver to share boilerplate functionality would make things to confusing.

I'm looking forward to hearing your ideas so I can build it right the first time.

keathley · 2017-04-17T14:44:57Z

lib/wallaby/experimental/selenium.ex

+    base_url = Keyword.get(opts, :remote_url, "http://localhost:4444/wd/hub/")
+    capabilities = Keyword.get(opts, :capabilities, %{})
+    create_session_fn = Keyword.get(opts, :create_session_fn,
+                                    &Wallaby.Phantom.Driver.create_session/2)


Ideally they could live in their own module but for now lets separate them. Once we have support for 3 drivers (phantomjs, selenium, chromedriver, etc.) then we can see if they share enough functionality to merge the driver implementations.

keathley · 2017-04-17T14:46:01Z

lib/wallaby/session.ex

-    screenshots: list
+    server: pid | nil,
+    screenshots: list,
+    driver: module


I'm a big fan of this idea. This gives us a lot of flexibility for running different browsers against each other.

keathley · 2017-04-17T14:47:45Z

test/support/session_case.ex

@@ -8,7 +8,19 @@ defmodule Wallaby.SessionCase do
  end

  setup do
-    {:ok, session} = Wallaby.start_session
+    session_opts = case System.get_env("WEBDRIVER") do


I realize that this is just for testing so it doesn't matter right now, but if we decide to stick with an implementation like this we should probably prefix the env variable with WALLABY_. Totally not important right now though.

Just changed the variable name to WALLABY_WEBDRIVER.

keathley · 2017-04-17T14:50:59Z

I think for now we shouldn't try to share functionality between the different driver implementations. Once we have multiple implementations we can see exactly where the common code / patterns are and we can use shared code at that point. Its easier to remove common code once you can see all of it.

For now it would be a good idea to share a common api between the drivers. We should create a behaviour that each driver needs to conform to. If you want I can work on that tonight. Then we can use that as a basis for the other drivers.

aaronrenner · 2017-04-17T15:10:58Z

I was thinking all of the drivers would share the following functions:

create_session/2
delete_session/1
find_elements/2
set_value/2
clear/1
click/1
text/1
page_title/1
attribute/1
visit/2
current_url/1
current_url!/1
current_path!1/1
selected/1
displayed/1
displayed!/1
size/1
rect/1
take_screenshot/1
cookies/1
set_cookies/3
set_windows_size/3
get_windows_size/1
execute_script/3
send_keys/2
log/1
accept_dialogs/1
dismiss_dialogs/1
accept_alert/2
accept_confirm/2
dismiss_confirm/2
accept_prompt/3
dismiss_prompt/2

If a driver didn't support one of these functions, we could return {:error, :unsupported_command} or raise an exception. Is that what you were thinking?

keathley · 2017-04-17T19:48:48Z

That looks about right. I think we can get rid of the ! functions (current_url!, current_path!, displayed!). Once #95 lands (which I think @krankin is working on) we can remove logs from that list as well.

aaronrenner · 2017-04-21T19:01:09Z

I just checked in the Wallaby.Driver behavior and switched Wallaby.Element and Wallaby.Browser to get the current driver off the session in #201. I think this will make adding a Selenium driver much easier.

aaronrenner · 2017-04-27T21:48:43Z

I finally got the tests passing and this is code should be ready for a review. 🎉

There's quite a bit of changes here, so it might be worth reviewing commit by commit. Here are the highlights.

First and foremost I added Wallaby.Experimental.Selenium. It has most of the basic test cases working but there are a few things I haven't had time to port over from phantomjs. Those are dialogs, getting the file upload tests to work with selenium and screenshots (I don't think selenium supports them).
I set up the build to run the unit tests, and then integration tests for phantom, selenium 2, and selenium 3. It took some tinkering in start_webdriver.sh but I think I have a good build environment now. Here's the build matrix on Travis.
I've gone through and fixed several issues with integration tests that have come up since I started running them against Selenium. The biggest issue was selenium doesn't wait until the page loads (after visit, clicking a link, or clicking a button) before it runs the next command. That was giving us a lot of failures when asserting with the current_url function. The way I solved this was to create a couple page objects with ensure_page_loaded/1 functions that use find/2 to wait until the page has loaded successfully before continuing. I'm sure we can expand the page object paradigm if we want, but this seemed like an ok way to get the tests working.

There's definitely still more to do on this driver (like handling weird responses from BrowserStack), but I am hoping to get this code merged into master so the PR doesn't get huge as I fix more and more issues. I set it to @moduledoc false and it's under the Wallaby.Experimental namespace, so people should realize that it's not ready for primetime, yet. Please let me know if you'd like any changes and I'd love to get it merged as soon as we can.

keathley · 2017-04-28T14:05:53Z

@aaronrenner Regarding the issue with selenium waiting for page loads we may need to look at this: https://www.w3.org/TR/webdriver/#navigation

keathley

This looks great! Just a few small comments. I'll pull down the branch and play with it tonight. I can merge it after that.

keathley · 2017-04-28T14:07:47Z

integration_test/cases/browser/click_button_test.exs

@@ -1,6 +1,8 @@
 defmodule Wallaby.Integration.Browser.Actions.ClickButtonTest do
  use Wallaby.Integration.SessionCase, async: true

+  alias Wallaby.Integration.PageObjects.IndexPage


Small note but it might be misleading to call these Objects. I just call them Pages typically.

Seconded! :)

keathley · 2017-04-28T14:09:24Z

integration_test/cases/browser/current_path_test.exs

-      |> click_link("Page 1")
+      |> IndexPage.visit()
+      |> IndexPage.click_page_1_link()
+      |> Page1.ensure_page_loaded()


I love seeing all of these! Using best practices in our own tests is awesome!

keathley · 2017-04-28T14:27:40Z

integration_test/support/page_objects/index_page.ex

+
+  def click_page_1_link(session) do
+    session
+    |> click_link("Page 1")


We'll need to update this to use click(link("Page 1")).

keathley · 2017-04-28T14:31:04Z

lib/wallaby/experimental/selenium.ex

+
+  @spec start_session([start_session_opts]) :: {:ok, Session.t}
+  def start_session(opts \\ []) do
+    base_url = Keyword.get(opts, :remote_url, "http://localhost:4444/wd/hub/")


I suspect this option will need to change to something like :selenium_remote_url once we add chromedriver support as well.

You'd start a session by calling:

Wallaby.start_session(driver: Wallaby.Experimental.Selenium, remote_url: "http://localhost:4444/wd/hub")

The :driver option determines which module to call start_session/1 on and the rest of the options are passed through directly to the driver. That way chromedriver could look like this:

Wallaby.start_session(driver: Wallaby.Experimental.ChromeDriver, remote_url: "http://localhost:4444/wd/hub")

That makes sense.

keathley · 2017-04-28T14:35:57Z

lib/wallaby/experimental/selenium.ex

+    WebdriverClient.send_keys(parent, keys)
+  end
+
+  # Unsupported


I think we can remove this.

keathley · 2017-04-28T14:43:08Z

lib/wallaby/session.ex

@@ -44,7 +44,7 @@ defmodule Wallaby.Session do
    id: String.t,
    session_url: String.t,
    url: String.t,
-    server: pid(),
+    server: pid | nil,


This is a super small nitpick but could we use an atom here like :empty or :none or maybe even :selenium? Just to signify that we don't actually use the server key for certain drivers.

aaronrenner · 2017-05-02T02:35:25Z

@keathley I looked into the webdriver pageLoadStrategy setting that you mentioned in your link, but I am still getting issues with tests to check the current page using the currentUrl function. I was thinking we could add a function like IndexPage.loaded?/1that would find a unique element on the page to ensure the expected page has loaded. Maybe that's something that could be done as part of removing the deprecated Browser.click_button work.

Please let me know if there's anything else you'd like to see on this PR. I think it should be good to go after #209 is merged.

See elixir-wallaby#198 (comment)

Updates to build

I have to return %{“value” => nil} because everywhere uses Map.fetch(body, “value”) and if I didn’t have a value key in the map, this would errors. This API really needs to be changed.

I added page objects so I have somewhere to add checking to see if the page is loaded yet. With selenium, when we send a visit command, it doesn't wait until the page is completely loaded to run the next command.

If the screen height was less then 1234 pixels, the test would fail. This uses a much smaller screen height.

There were a couple issues with click_button test. 1. Selenium wasn't waiting until the page finished loading to get the current url, so these tests were failing 2. When the form was submitted it sent a POST request to the test server, which resulted in a 500 error. I updated it to send a get request, but then had to update the current url assertion since the form data was being included as url parameters.

The older version of Firefox injects the xmlns attribute in the html tag. We could probably improve this test if it continues to be an issue.

Also fixed an instance where I was using a deprecated api

See elixir-wallaby#198 (comment)

I have been having issues on travis ci where selenium times out when starting a session. This should take care of it.

aaronrenner · 2017-05-03T17:20:30Z

I think I finally took care of the flake tests and it's ready to merge. I added some retry logic to SessionCase.start_test_session and made it so Selenium.start_session can better handle error tuples. Let me know if you see anything else I missed.

PragTob · 2017-05-03T18:42:12Z

I was sort of thinking about giving this another look but honestly 1000 lines is a bit more than my casual evening read :D

If you're both 👍 on it then I'm in as well, otherwise I might get to look at it on the weekend but no promises. :)

aaronrenner · 2017-05-03T18:53:13Z

Sorry it's such a big PR. I'll make my next one much smaller. 😄

keathley · 2017-05-08T14:10:24Z

Ran through it one last time. Looks good to me 👍

@aaronrenner Do you feel good about merging this now?

* Was accidentally removed as part of #198

aaronrenner commented Apr 14, 2017

View reviewed changes

aaronrenner force-pushed the ar-selenium branch 2 times, most recently from 18d83a8 to 1e67d27 Compare April 14, 2017 20:05

keathley reviewed Apr 17, 2017

View reviewed changes

aaronrenner force-pushed the ar-selenium branch from 1e67d27 to 5bf37f5 Compare April 17, 2017 15:00

aaronrenner force-pushed the ar-selenium branch from 5bf37f5 to 2d4ed0c Compare April 17, 2017 22:36

aaronrenner force-pushed the ar-selenium branch 8 times, most recently from 9b8582f to bc1c5bf Compare April 27, 2017 21:13

aaronrenner changed the title ~~WIP - Experimental selenium driver~~ Experimental selenium driver Apr 27, 2017

aaronrenner mentioned this pull request Apr 27, 2017

Run against Erlang 19.3 & 18.3 #205

Merged

keathley reviewed Apr 28, 2017

View reviewed changes

aaronrenner force-pushed the ar-selenium branch from bc1c5bf to b9f26aa Compare May 1, 2017 19:36

aaronrenner force-pushed the ar-selenium branch from b9f26aa to 9bd561f Compare May 1, 2017 19:48

aaronrenner mentioned this pull request May 1, 2017

Fix leaking webdriver sessions in QueryTest #209

Merged

aaronrenner force-pushed the ar-selenium branch from 9bd561f to 0b77709 Compare May 2, 2017 14:03

aaronrenner added a commit to aaronrenner/wallaby that referenced this pull request May 2, 2017

Set default Session.server to :none

cf939a9

See elixir-wallaby#198 (comment)

keathley mentioned this pull request May 3, 2017

Chrome headless driver #211

Closed

aaronrenner added a commit to aaronrenner/wallaby that referenced this pull request May 3, 2017

Set default Session.server to :none

74a283f

See elixir-wallaby#198 (comment)

aaronrenner added 9 commits May 3, 2017 10:59

Experimental Selenium driver

7b61584

Updates to build

Handle 204 response from webdriver api

4fa09d1

I have to return %{“value” => nil} because everywhere uses Map.fetch(body, “value”) and if I didn’t have a value key in the map, this would errors. This API really needs to be changed.

Fix integration tests for selenium

150f583

I added page objects so I have somewhere to add checking to see if the page is loaded yet. With selenium, when we send a visit command, it doesn't wait until the page is completely loaded to run the next command.

Fix test failure on a small screen

554c8f7

If the screen height was less then 1234 pixels, the test would fail. This uses a much smaller screen height.

Fixing integration test on Selenium 2

a7efab2

The older version of Firefox injects the xmlns attribute in the html tag. We could probably improve this test if it continues to be an issue.

Renamed PageObjects to Pages

946e96b

Also fixed an instance where I was using a deprecated api

Set default Session.server to :none

36a4b4e

See elixir-wallaby#198 (comment)

Retry if can't start test session

7820dbe

I have been having issues on travis ci where selenium times out when starting a session. This should take care of it.

aaronrenner force-pushed the ar-selenium branch from 0b77709 to 7820dbe Compare May 3, 2017 17:01

keathley mentioned this pull request May 8, 2017

Remove click_button and click_link from Browser. #212

Merged

keathley merged commit c4a5754 into elixir-wallaby:master May 8, 2017

PragTob added a commit that referenced this pull request May 11, 2017

Reinstate coveralls test coverage reporting

8604fe1

* Was accidentally removed as part of #198

PragTob mentioned this pull request May 11, 2017

Reinstate coveralls test coverage reporting #222

Merged

keathley pushed a commit that referenced this pull request May 11, 2017

Reinstate coveralls test coverage reporting (#222)

11741b6

* Was accidentally removed as part of #198

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental selenium driver #198

Experimental selenium driver #198

aaronrenner commented Apr 14, 2017

aaronrenner Apr 14, 2017

keathley Apr 17, 2017

aaronrenner commented Apr 14, 2017

keathley Apr 17, 2017

keathley Apr 17, 2017

keathley Apr 17, 2017

aaronrenner Apr 17, 2017

keathley commented Apr 17, 2017

aaronrenner commented Apr 17, 2017

keathley commented Apr 17, 2017 •

edited

Loading

aaronrenner commented Apr 21, 2017

aaronrenner commented Apr 27, 2017

keathley commented Apr 28, 2017

keathley left a comment

keathley Apr 28, 2017

PragTob Apr 28, 2017

keathley Apr 28, 2017

keathley Apr 28, 2017

keathley Apr 28, 2017

aaronrenner Apr 28, 2017

keathley May 2, 2017

keathley Apr 28, 2017

keathley Apr 28, 2017

aaronrenner commented May 2, 2017 •

edited

Loading

aaronrenner commented May 3, 2017

PragTob commented May 3, 2017

aaronrenner commented May 3, 2017

keathley commented May 8, 2017 •

edited

Loading

Experimental selenium driver #198

Experimental selenium driver #198

Conversation

aaronrenner commented Apr 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronrenner commented Apr 14, 2017

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

keathley commented Apr 17, 2017

aaronrenner commented Apr 17, 2017

keathley commented Apr 17, 2017 • edited Loading

aaronrenner commented Apr 21, 2017

aaronrenner commented Apr 27, 2017

keathley commented Apr 28, 2017

keathley 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

aaronrenner commented May 2, 2017 • edited Loading

aaronrenner commented May 3, 2017

PragTob commented May 3, 2017

aaronrenner commented May 3, 2017

keathley commented May 8, 2017 • edited Loading

keathley commented Apr 17, 2017 •

edited

Loading

aaronrenner commented May 2, 2017 •

edited

Loading

keathley commented May 8, 2017 •

edited

Loading