Update the Capybara versions being tested against and implement missing APIs

[twalpole]

This implements the Driver#switch_to_frame and Driver#execute/evaluate_script argument passing APIs from Capybara 2.11 and the upcoming 2.12. It also fixes issues with #disabled? on elements in disabled fieldsets that are now tested for it Capybara.

The one issue left is a frame being closed while it is the active frame (within_frame and switch_to_frame). Looking through the code I THINK it's an issue with the JavascriptInvocation object and CapybaraInvocation object being used after the frame they are used with has been deleted. Not being all that familiar with the code though it would be great if someone could confirm that is the issue and propose a solution. I would love to get Capybara-Webkit running on the latest Capybaras again so users can utilize the new features.

[mhoran]

This is awesome, thanks @twalpole.

It looks like at least one of the APIs (QJsonDocument) you've used is incompatible with Qt 4.8. It's really time for us to drop support for that, but we wanted to release a 2.0 before doing so. I'd also like to have some test coverage for the new JavaScript functionality to ensure that we don't break that block of code in capybara-webkit.

I may have some time to look into the other issues over the weekend.

[twalpole]

@mhoran I've moved the JSON encoding of the args back to Ruby which I think should get around the lack of convenient JSON methods in Qt 4.8. As for tests, these behaviors are all tested in the Capybara provided tests (currently only in the master branch which will become 2.12 soon), what else would you like tested in capybara-webkit?

[jferris]

As for tests, these behaviors are all tested in the Capybara provided tests (currently only in the master branch which will become 2.12 soon), what else would you like tested in capybara-webkit?

We've still written unit tests for any code we add to capybara-webkit. Part of that is that I use TDD on everything, so it feels strange to me to write code without writing some tests first. However, it's also important for the odd bugs we've had to workaround in Qt, WebKit, and QtWebKit.

Although the Capybara-provided tests are very useful for regression testing driver behaviors, there are many issues that only exist in our particular implementation. There are strange edge cases like "it should follow a redirect in an iframe even if a content type is not provided," which have only ever come up with QtWebKit.

[twalpole]

@jferris That's great, except I don't know what edge cases are going to come up just in QtWebKit for this functionality. I started by writing the tests in Capybara, getting the selenium driver to pass those, then wrote code for Poltergeist to get those same tests passing there, and am now submitting a PR to get capybara-webkit to pass the tests. So I am doing TDD, it's just that all the tests were written once in Capybara. I'm open to adding whatever tests you'd like to capybara-webkit, and if you want the current Capybara tests duplicated in capybara-webkit that's fine too if it moves support for newer versions of Capybara forward (partly for selfish reasons since I'm tired of having to give multiple answers for questions on how to do things depending on whether a user is on 2.7 or latest)

[twalpole]

@mhoran I have confirmed the crash is definitely because the JS object added to the frame -https://github.com/thoughtbot/capybara-webkit/blob/master/src/JavascriptInvocation.cpp#L36 - is gone when the action called from https://github.com/thoughtbot/capybara-webkit/blob/master/src/capybara.js#L213 closes the frame and attempts to return back to JS land and therefore crashes. Not sure if there's a nice Qt solution for that ( I tried calling the action from setTimeout and allocating the JavascriptInvocation object on the heap rather than the stack, which solved this issue but broke a bunch of other things ) or whether some of the actions need to be changed so the mouse events are actually generated after JS methods have returned (rather than calling the JavascriptInvocation action from the JS). I would guess this could also be the cause of things occasionally hanging when a page is redirected from JS too - issue #873

[mhoran]

Yeah, that's what I thought the problem was. I'll see if I can figure something out.

On January 6, 2017 4:32:20 PM EST, Thomas Walpole notifications@github.com wrote:

@mhoran I have confirmed the crash is definitely because the JS object added to the frame -https://github.com/thoughtbot/capybara-webkit/blob/master/src/JavascriptInvocation.cpp#L36

• is gone when the action called from https://github.com/thoughtbot/capybara-webkit/blob/master/src/capybara.js#L213 attempts to return back to JS land and therefore crashes. Not sure if there's a nice Qt solution for that - I tried calling the action from setTimeout and allocating the JavascriptInvocation object on the heap rather than the stack, which solved this issue but broke a bunch of other things - or whether some of the actions need to be changed so the mouse events are actually generated without or after JS methods have returned (rather than calling the JavascriptInvocation action the JS)

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/thoughtbot/capybara-webkit/pull/972#issuecomment-271013023

-- Sent from my mobile device.

[twalpole]

@mhoran Any update on this? This may be worth merging into master with a known issue about frames that are closed during the test since that's already an issue with the release version, it just wasn't tested for back then.

[twalpole]

I worked around the crash by moving a left clicks actual click to after the JS has been invoked. Technically someone could set up their app to close a frame by some other action too, but I don't think it would be all that common or likely, so there may be need to support that currently. I also added a pointer to the current frames parent so it is still accessible if/when a frame closes and goes away. This should make all of Capybaras current master branch tests pass, and make capybara-webkit compatible with the soon to be released Capybara 2.12

[mhoran]

Nice work, @twalpole! I had to do a similar thing with capybara-chromium, although for a slightly different reason. The separate browser and renderer threads introduce all sorts of asynchronicity, which makes synchronizing things like click events even more challenging.

I guess a middle click, and perhaps double click, and maybe even other clicks could trigger this behavior and should go down the new code path. But that brings me to my next point: I'd definitely like some test coverage, in capybara-webkit, for this fix. I think we could test it by asserting that a JS event finishes processing when a click is received. That way, we don't regress this behavior.

You could argue that this is again covered by the existing test suite, but we've always covered situations like this with a regression suite that covers the specific behavior that was changed. This is a very capybara-webkit specific change, so I wouldn't expect a test to be introduced upstream.

Re: testing for the evaluate_script changes, I still think some specific tests around the handling of passed-in arguments (specifically, handling elements vs. JSON serializing basic types) would put my mind at ease about the change in behavior. I'm arguing for these test changes because the Driver class itself has changed to make it possible to implement this feature, and the implementation is specific to capybara-webkit.

[twalpole]

I've added basic tests for the argument passing and switch_to_frame stuff

[jferris]

@twalpole thanks for all your work on this! I have some free time tomorrow to get this merged and released.

[jferris]

@twalpole thanks again! I just merged this into master as 43ebe80704e118ba690d62049fe346fbe787e050 and released 1.12.0 to RubyGems with these updates.