twalpole / apparition

Capybara driver for Chrome using CDP
MIT License
363 stars 46 forks source link

Capybara::Apparition::WrongWorld error when trying to evaluate script #12

Closed jordanfbrown closed 5 years ago

jordanfbrown commented 5 years ago

Thank you for the great project! We just converted our test suite (2,500 feature tests) from chromedriver and were able to uncover a bunch of real bugs in our product.

We have a couple of hacks that have emerged over time, one of which evaluates some Javascript and jQuery to make assertions about pending AJAX calls. The first part of that check looks like:

page.evaluate_script('window.jQuery != undefined')

However, that line is sometimes throwing an error:

Failure/Error: page.evaluate_script('window.jQuery != undefined')

Capybara::Apparition::WrongWorld:
  The element you are trying to access is not from the current page

Do you have any insight into why we would see an error like that just using page.evaluate_script, since it doesn't appear to be accessing an element at all?

Also, worth nothing that this error happens inconsistently. Maybe it has to do with the page refreshing?

twalpole commented 5 years ago

@jordanfbrown It could be the script is running while the page is changing (which would an apparition bug), what are the tests where it occurs doing around the script evaluation?

Note: with all drivers using Capybara you're going to be much better off using visual change assertions rather than pending Ajax hacks if at all possible.

jordanfbrown commented 5 years ago

The flow for the relevant part of the test is:

  1. User clicks a button
  2. The button triggers an AJAX call
  3. Our wait_for_ajax helper method is invoked
  4. Upon completion of the call, the page refreshes
  5. The above error is thrown at some point between 3 and 4

The test was initially written to use a helper, wait_for_ajax, which executes some Javascript code. My guess is that the page refresh is interrupting the assertion. I ended up just removing the wait_for_ajax call entirely in this case, but I wonder if there may actually be a bug here like you said where the script is running as the page refreshes. What do you think? Is there any more information I can provide? I can try and reproduce in this project's test suite if that would be helpful.

And to follow up on your comment - we definitely don't use this helper for any new specs, but it's a massive test suite so going through and updating all old cases to use visual assertion is unfortunately quite tedious.

twalpole commented 5 years ago

Ok, so it probably is the script being run across two different contexts due to page refresh. If you could produce a reproducing test case that would be a great help 👍I’ll probably get to look at this Sunday or Monday

jordanfbrown commented 5 years ago

@twalpole I was able to come up with a test case that reproduces the error consistently: https://github.com/jordanfbrown/apparition/commit/627a948c11cfe33377fb31c93e32a683a52aa282

twalpole commented 5 years ago

@jordanfbrown Great - could you please submit that test as a PR, and I'll take a look at fixing it Sunday/Monday -- thanks!

jordanfbrown commented 5 years ago

PR opened: https://github.com/twalpole/apparition/pull/13

Thanks again!

twalpole commented 5 years ago

@jordanfbrown I merged in the test and have been playing with it. Unfortunately the only reason it's consistently failing is because Capybara.default_max_wait_time is set to 0 seconds in that test (which it should never be in real usage) and it fails on the have_content('Clicked') not on the evaluate_script. If Capybara.default_max_wait_time` is set to a normal number of 2 seconds everything passes. I haven't been able to come up with a way that will reliably fail under normal usage. I'm going to add retrying for execute/evaluate_script and we'll see if that fixes the issue.

twalpole commented 5 years ago

@jordanfbrown Could you please try using the retry_script_on_wrong_world branch and see if that fixes your issue.

jordanfbrown commented 5 years ago

@twalpole I'm not seeing any additional changes in the retry_script_on_wrong_world branch. Did you push your changes up?

twalpole commented 5 years ago

@jordanfbrown Whoops -- forgot to actually commit -- sorry about that -- should be there now

twalpole commented 5 years ago

Fixed via 2537511db8abd6f679b1a774ac7051aa284c943e - Thanks for reporting and the help in verifying the fix!