elixir-wallaby / wallaby

Concurrent browser tests for your Elixir web apps.
https://twitter.com/elixir_wallaby
MIT License
1.68k stars 198 forks source link

Slow-ish phantomjs interaction times (a single fill_in takes ~500ms) #164

Closed PragTob closed 4 years ago

PragTob commented 7 years ago

This is with current master, elixir 1.4.0, erlang 19.1 and phantomjs version 2.1.1 (installed via npm install -g phantomjs-prebuilt) on Linux Mint 18.1 (basically Ubuntu 16.04).

I wrote an acceptance test for our application and noticed how our tests took so much longer, at first I thought it was the phantomjs startup but then measured just in the test - it took 12 seconds. Now the test is pretty thorough, and fills out a big form.

Still, that is way longer than it should take in my opinion (and feel like it'd take with capybara in ruby but of course I haven't tested it, maybe I'm just sensitive to test run times in our elixir applications).

I created a little reproduction script for wallaby:

{:ok, server} = Wallaby.TestServer.start
Application.put_env(:wallaby, :base_url, server.base_url)

use Wallaby.DSL

{:ok, session} = Wallaby.start_session

session = visit(session, "forms.html")

IO.puts "fill_in start"
{time, _} = :timer.tc fn -> fill_in(session, Query.text_field("name"), with: "Chris") end
IO.puts "Finale fill in time #{time / 1000} ms"

Wallaby.end_session(session)

I also added a couple of targeted timers to see where the slowness is coming from (you can check them out on my slowness branch)

The result is somewhat as expected, the slowdown seems to come from the communication with the phantomjs browser:

fill_in start
make_request of driver - 76.92 -body: {"value":".//*[self::input | self::textarea][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'radio' or ./@type = 'checkbox' or ./@type = 'hidden' or ./@type = 'file')][(((./@id = \"name\" or ./@name = \"name\") or ./@placeholder = \"name\") or ./@id = //label[contains(normalize-space(string(.)), \"name\")]/@for)] | .//label[contains(normalize-space(string(.)), \"name\")]//.//*[self::input | self::textarea][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'radio' or ./@type = 'checkbox' or ./@type = 'hidden' or ./@type = 'file')]","using":"xpath"}
make_request of driver - 48.024 -body: {"type":"browser"}
driver find_elements 125.729
make_request of driver - 78.834 -body: 
make_request of driver - 47.898 -body: {"type":"browser"}
Browser.execute_query execution time 256.394 ms
make_request of driver - 72.57 -body: 
make_request of driver - 46.537 -body: {"type":"browser"}
make_request of driver - 73.669 -body: {"value":["Chris"]}
make_request of driver - 44.03 -body: {"type":"browser"}
Element.fill_in time: 239.773 ms
Full fill_in time 496.354 ms

E.g. It seems every interaction takes ~ 250ms - first finding the element (first Browser.execute_query) takes that time and then filling in an element takes about the same time as well.

Can anything be done here? Are these times normal and I'm just unaware of it and everything is slow? ;P

E.g. could some of these requests be saved? Is there a faster way of interaction? Could look to capybara/hound for checking what might be done.

PragTob commented 7 years ago

Other than getting the number of requests down, it seems like there are different interaction methods: http://phantomjs.org/inter-process-communication.html

If my glance at the code didn't betray me poltergeist (the capybara phantomjs driver) uses the socket interaction out of all of these.

keathley commented 7 years ago

Poltergeist is somewhat different in that it implements an entire client in javascript and then connects to the tests via websockets. I would definitely like to speed up test times as much as we can but building a bespoke client hasn't seemed worth it yet :).

I think there are a couple of ways to help optimize wallaby + phantom. The first is to remove the explicit checks for JS logs. We should be able to get those from the phantom server as output. That will save us an extra http request per operation which should be an easy time savings.

We also do several extra queries during "finding". This is typically stuff like checking for inner text or visibility. We could probably optimize some of that as well. We would need to do more profiling first obviously.

PragTob commented 7 years ago

Couldn't we also just "borrow" the poltergeist JS client and then just implement the elixir websocket part of it? Would save us some of the trouble... hoping that wallaby's requirements can be satisified with whatever poltergeist can do :) (I'm an optimist, but enough of a realist to fear that there is some very capybara specific stuff in there)

Thanks for the quick answer!

I'm also happy to add benchee and a couple of relevant benchmarks :)

keathley commented 7 years ago

Having benchmarks with benchee would be awesome! I think we should start there so that we know what we're dealing with. If we can optimize in a few places then we might be able to avoid implementing our own version of webdriver.

If we do need to start thinking about going the poltergeist route then its probably worth looking at what they have for inspiration. Right now we check a lot of properties on an element whenever we query for something. Each of those checks is an http call. We also have to retry a lot to stay in sync with the browser. I've thought for a while that it might make sense to send over queries and actions to the browser but do all of the synchronization and querying in the browser. That removes all of the overhead of extra http calls and also makes syncing easier. It would also allow us to handle some of the issues with webdriver.

I've also thought about automatically transforming all queries into xpath queries. Right now you might end up doing an http call to check visibility or to check text (counting is done in elixir without needing to do an webdriver call). We could do all of that with xpath. That would eliminate all the other webdriver calls and it would help to reduce some of the issues with checking all these elements.

Anyway those are just some ideas. I'm open to lots of solutions. We should benchmark and see if there are some easy wins. At least that way we'll know what we're dealing with.

PragTob commented 7 years ago

As an update here I ran and wrote benchmarks for capybara and wallaby, hound still needs some work: https://github.com/PragTob/acceptance_test_speed

TLDR; is filling a form field in is almost a hundred times faster atm with capybara, so we have some space to improve (that's the full blown poltergeist appraoch though which will take _ALOT of time when/if we get to it)

hisapy commented 7 years ago

Hey guys,

I'm sorry for what I'm about to say. Unfortunately, despite I love Wallaby's syntax, I'm going to have to switch back to Hound mostly due to the following issues

  1. Constant connection refused errors. I don't know if it's something related to how the PhantomJS process is handled, PhantomJS crashes and I it's really complicated to debug. It's so weird that for example, these tests sometimes work when I take_screenshot and sometimes don't.
  2. Tests in Wallaby are very slow compared to Hound. I'm running scenarios for a SPA and the difference is about 60 secs.

That said, I'm keeping an eye on this project and on this issue in particular because I think I'd like to contribute to it to try to fix the above mentioned issues.

Besides, I was trying to write a Driver for NightmareJS but currently that's something beyond my time and abilities, and also because NightmareJS API is mostly for scripting and not actually a web driver or something you can make requests to.

keathley commented 7 years ago

@hisapy If Hound suits your needs then you should use it. Its a fine library. If you want to provide more detail about the connection refused issues I can take a look at it. If PhantomJS is crashing then that seems like a major issue. I haven't seen that before so it would be good to know about.

If you ever want to contribute feel free to just jump in. If you have any questions I'm sure all of us would be happy to help. 👍

PragTob commented 7 years ago

:wave:

With the new benchee version with hooks I could finally benchmark hound vs. wallaby and yup we are indeed slower :'( Full results/benchmarks at https://github.com/PragTob/acceptance_test_speed

This is the direct comparison result:

##### With input filling a form field #####
Name              ips        average  deviation         median         99th %
hound            7.28      137.30 ms     ±1.62%      136.02 ms      144.03 ms
wallaby          2.79      358.74 ms     ±1.13%      356.10 ms      376.06 ms

Comparison:
hound            7.28
wallaby          2.79 - 2.61x slower

##### With input find css by id #####
Name              ips        average  deviation         median         99th %
hound           22.63       44.18 ms     ±1.95%       44.00 ms       48.01 ms
wallaby          5.66      176.66 ms     ±1.04%      176.00 ms      187.55 ms

Comparison:
hound           22.63
wallaby          5.66 - 4.00x slower

##### With input visit url #####
Name              ips        average  deviation         median         99th %
hound           19.02       52.59 ms     ±2.69%       52.01 ms       56.06 ms
wallaby         10.35       96.66 ms     ±1.65%       96.00 ms      103.97 ms

Comparison:
hound           19.02
wallaby         10.35 - 1.84x slower

-> For my simplish scenarios we are ~2 to 4 times slower than hound. Especially a find operation seems to be expensive. We are a lot slower there than hound and do about 6 iterations per second. As an (even worse) comparison capybara with poltergeist does more than 1200 - so it's 200 times faster.

So, we have quite some potential for improvements :D

stevegraham commented 6 years ago

It looks like Wallaby does the following:

Are all of these necessary? Would there be a problem with adding a visibility check to the Path query? I have removed the clear field and log calls from an experimental fork with no adverse consequence. What's the rationale for including them?

keathley commented 6 years ago

@stevegraham What do you mean when you say "adding a visibility check to the Path query"?

I generated some flame graphs a while ago and wallaby spends the majority of its time checking for visibility of elements. This is because checking for visibility is pretty hard. Its not enough to check the element itself. The algorithm also needs to check each of the elements parent elements. I'm nervous to remove the visibility check altogether because it may allow tests to interact with dom elements that are hidden. We should test that out though and ensure that my assumptions are correct. If we could remove the visibility check altogether I believe that would give us a pretty substantial speed increase. Maybe you can benchmark that on your branch to confirm that assumption?

As for the clear call, iirc in some browsers sending text doesn't first clear the text from the field. So we have to send it manually.

stevegraham commented 6 years ago

I'm not making any assumptions. I'm asking questions.

keathley commented 6 years ago

Sorry! I didn't mean to imply that you were making assumptions. More that I've made assumptions and those assumptions might be wrong. You're totally correct to ask those questions. I wrote this code a while ago and I can't quite remember why I made all of the decisions that I made :heart:

Specifically we may not need the visibility check or the clear call at all. If your fork is working then thats already a great start.

michallepicki commented 5 years ago

@hisapy @stevegraham We now have a way to skip checking element visibility with the visible: :any option. Additionally if tests are very slow when written in a declarative way, if there are multiple interactions with one element, and the element on the page is not modified in the DOM, a possible optimization is finding the element first and memoizing the Element struct instead of always passing the query. Let us know if you have any ideas on what we can do to improve Wallaby performance, all feedback and ideas are very appreciated!

@PragTob @keathley We need to check if the issue in question affects WebDriver as well. There's a plan to deprecate PhantomJS backend altogether (see #437)

PragTob commented 5 years ago

@michallepicki I'm pretty sure it affects web driver as well as iirc the root cause I found is us making a lot of requests and the requests just taking its time. In my experience webdriver was even slower than phantomjs which is why I wasn't on the "deprecate phantom" wagon too early. I fear the only way to make it really fast would be a different architecture. Short term what you mentioned and/or other APIs for mass interactions could alleviate the problem.