Implement within_window

[mhoran]

Resolves issue #129. Uses the same mechanism as the Capybara Selenium driver to determine which window to switch to: http://git.io/XrUlqw#L89. Note that I've defined window_handle and window_handles on the Driver instead of the Browser, which is incompatible with the (unofficial?) Selenium API.

[mhoran]

if you have any pointers to any refactoring that may help get this pulled in, that'd be great.

Though I no longer need support for this for any current project, I do know that the community is in general interested in support.

[jferris]

Hey Matt,

Sorry to take so long to review this. It's a pretty big change, and I wanted to make sure I looked through it carefully. I just have one question: how will extra windows be closed at the end of each scenario? I assume we'll want to clean them up in reset somehow.

[mhoran]

Good point, @jferris. I went ahead and rebased from master to get started on this, but my work didn't apply cleanly. I've gotten things to the point where they compile, but the test suite hangs towards the end. It looks like this is going to take some time to straighten out, and I don't have much of that right now. I'll try to take care of it when I can, though.

[mhoran]

I traced down the deadlock to #308. In some cases, the TCPServer used in the suite may receive two simultaneous connections, which blocks the thread. I've fixed this in my branch and am moving on to the reset! functionality.

[mhoran]

I've made the necessary changes to clear new windows on reset. Please let me know if you have any additional questions or concerns.

[masone]

Thanks for your effort @mhoran
I have just tested mhoran:master with my application I am currently testing with the Selenium driver. I stumbled upon two problems.

With the Selenium driver, I used to be able to query the window by its title.

within_window('Window title') do
end

I get a "Unable to locate window" error when querying by title so I ended up doing something similar like you do it in the tests.

within_window(page.driver.browser.get_window_handles.last) do
end

The second thing may be very specific to my application. I want to test my Facebook dialogs (FB.ui in the Javascript SDK). I log in with a user, open a dialog, click a button in the dialog. Problem is, that the dialog shows a quite generic error instead of the actual content. The popups is triggered via Javascript from my site but the HTML is loaded directly from Facebook. The visiting user has an active session at Facebook and is authenticated with OAuth via the Javascript SDK.
It works well when using the Selenium driver. Since I have no idea how capybara-webkit works, my best guess would be that it has to do with sessions / cookies. Does that ring any bells?

I'd be very happy to be able to test my dialogs with webkit.
Let me know if I can be of any help!

[jferris]

@mhoran I got a number of "too many open files" failures when trying to merge this. I'll try to look at those and get this merged this week.

[jferris]

@masone my plan is to merge this as-is, once I've figured out the errors. It seems like finding windows by title isn't part of the core capybara API, since this passes the window specs. I'd be interested in supporting it, though, if you'd like to open another pull request.

[mhoran]

@masone, my guess is this is a timing issue. If the new window hasn't loaded by the time you select by title, it will fail. This mechanism should probably be made more robust, but I don't think it would be straightforward to do so. I'll look into it, though.

@jferris, I bet the deleteLater is never happening when the driver is reset. This would leave file descriptors and signals connected, which could lead to too many open files.

[mhoran]

@masone, I've set up a test project using Facebook auth and within_window via title selector here: http://git.io/38bFwA.

There are a few caveats. First, the reason you may be getting the generic error is that your hostname within capybara-webkit may not match that configured in your Facebook application settings. See the monkey patch in features/support/server.rb.

Secondly, the DOM may have not completely loaded at the time the within_window command is received, so although capybara-webkit may think the page has loaded (and therefore accepts the command), title querying doesn't find the page. I've added a sleep to the corresponding step definition to work around this. Of course, that's not great, so I recommend the window_handle route.

[mhoran]

@jferris, I just pushed up a possible fix for the too many open files failures. it seems that deleteLater never does anything because capybara-webkit is single threaded, and control never returns to the main loop. Using delete is safe in this situation because there's no chance that another thread will try to access the object.

I had to remove the disconnect from PageLoadingCommand as this was causing a segmentation fault after the delete. Deleting an object automatically disconnects all signals, so this should be fine.

Note that deleteLater is currently used throughout the project, and my guess is it's not working as expected elsewhere.

[mhoran]

I cleaned up the commits a bit and also refactored the previous "too many open files" fix so that the disconnect can remain in place. The PageLoadingCommand doesn't need the actual page that's loading, just a signal that the current page has loaded, so I've wired it up to the WebPageManager instead.

[mhoran]

@masone, I was actually able to track down the issue with find window by title (and URL). As expected, it's a race condition between the loading of a new window and the issuance of the within_window command. The specs for this functionality pass because the content is available before the within_window command is issued. However, in the real world, this won't always be the case, and so the feature is brittle.

PageLoadingCommand needs to be extended to be aware of whether it has created a new window which is loading. I began looking into this, but seeing as how this is a massive pull request as is, I'd like to hold off on that until later. For now, you can use either the window handle method, the window name method, or simply sleep until the new window has loaded.

[mhoran]

Turns out deleteLater actually does work, but my debugging wasn't showing the deallocation. I'm using deleteLater now as this is a bit safer.

The "too many open files" fix turned out to be ensuring that the NetworkAccessManager is created with the WebPage as its parent, so when it is deleted in reset the NetworkAccessManager gets deallocated. I tested this with lsof and both memory and open files were equivalent to that of the commit before the introduction of within_window support.

[piotrze]

I've switched to webkit and Your branch mhoran:master fixed my tests with window_handles method. Good work.

[mhoran]

Rebased to current master.

[jferris]

Thanks for all your work, Matt. I merged this into master.

[strunkandwhite]

I'm working on a project using within_window and it seems like cookies aren't being preserved between windows - a user that's signed in on window A isn't signed in on (new) window B. Do you guys have any ideas? Thanks y'all.

[jferris]

I haven't heard that one before, so it seems like you should open a new issue. If you have a failing test case or a sample of code that shows the behavior, that would be helpful. A pull request would also be awesome.

[mhoran]

The issue is that the NetworkCookieJar is not shared between windows. I've written a test, though I'm not convinced it's failing properly. The proper test may require resolution of the outstanding issue where new commands can be issued before a new window has finished loading. (New commands on that window will block, but commands in the existing window will execute.)

I'll spend some time this weekend getting this worked out, unless someone else gets to it first.

[jferris]

@mhoran - thanks for looking into this. @halogenandtoast and I started work on a branch that synchronizes page loads between windows. I'll check in with him on that today.