thoughtbot / capybara-webkit

A Capybara driver for headless WebKit to test JavaScript web apps
https://thoughtbot.com/open-source
MIT License
1.97k stars 427 forks source link

Update to modern RSpec #976

Closed twalpole closed 7 years ago

twalpole commented 7 years ago

This updates the tests to expect syntax, and fixes all warnings/deprecation when using RSpec 3.5.x. Some tests which will now pass against Capybara 2.12.0+ are also enabled to run on those versions. The tests get run with both RSpec 2.x and 3.5.x on travis.

One Capybara test that had to be disabled (passed on RSpec 2.x but failed on 3.x because the block passed to raise_error was ignored when RSpec 2.x ran the test). The test is "Capybara::Session webkit node #reload without automatic reload should not automatically reload" which fails because it returns the wrong type of error. This is caused by the fact that capybara-webkit allows access to detached nodes when Capybara.automatic_reload is false (other drivers report that the element is no longer valid). I'm not sure what the point of this behavior is, but it is intentional and is tested for in capybara-webkits tests.

jferris commented 7 years ago

This is caused by the fact that capybara-webkit allows access to detached nodes when Capybara.automatic_reload is false (other drivers report that the element is no longer valid).

We introduced that flag for backwards compatibility (that was how capybara-webkit happened to behave before Capybara added auto-reloading).

We also had some concerns about implementing it without causing performance issues, so we left in an option to disable the DOM attachment check. I haven't run benchmarks to compare in a long time, but there was a significant performance improvement by not using the auto-reload feature. However, I've run most of my test suites using capybara-webkit and auto-reloading enabled, and I haven't had any problems.

We could likely deprecate this flag and remove it in the next major release.

twalpole commented 7 years ago

@jferris That explains it - updated the comment

jferris commented 7 years ago

Thanks! This is merged into master.