Don't run so many browser instances

[jdudek]

Currently bbq creates new Capybara session for each TestUser in tests. That means running separate browser process when using Selenium. Spawning those processes takes a considerable amount of time on each tests run.

I think we could speed up our tests and also use less memory if we reuse Capybara sessions between tests. A few weeks ago I wrote a sample implementation: https://gist.github.com/1456271

I'm pretty sure it could be improved. (I don't like the idea of passing TestUser class, but I needed Bbq::TestUser#page to call reset!; maybe we need an object encapsulating Capybara session and pass that object to TestUser.new). Also, this implementation needs thorough testing. I recall it didn't work quite well in some cases...

What do you think?

[paneq]

Some time ago I've been thinking exactly about that (meaning browsers pool). The only possible problem that I see might be that I had problems when running multiple browsers and some of them were inactive for longer time (about 30s). Take a look at "Timeout::Error" section that I added to our README. Anyway I think it should be a default behavior (to use browser from pool) due to the long browser startup time and in case it does not work well (might happen for various reasons) it should be easy to overwrite that setting like this:

TestUser.new(:pool => false)

That would always start a new browser instance that should be automatically closed at the end of test (ticket #11).

[jdudek]

Thanks for you feedback. I'll try to make better implementation and then we'll see if it works. Your proposed API is less explicit, but seems more natural than mine. I also agree that it's good opportunity to fix #11.

I also see Timeout::Error from time to time, maybe I should switch to Mongrel.

[paneq]

Actually, maybe it is a good idea to make it more explicit.

user   = browsers_pool.next(:email => "janek@janki.pl", :password=> "p@55w0rd") # from pool
user2 = TestUser.new(:email => "robert@roberty.pl", :password=> "hardPassword")  # without pool (same options allowed)

We could have create one pool at the beginning of testing and set it to an instance_variable @browsers_pool at the setup of each test (so that it is always the same object). The problem with my previous API proposal is that browser pool would be some kind of global accessible from TestUser class. I think it would a better separation if TestUser knows nothing about pool. Usually objects in pool are not aware of pool at all.

[paneq]

Or maybe:

user = TestUser.new(:email => "janek@janki.pl", :password=> "p@55w0rd", :pool => browsers_pool)

The thing is: We have a pool of browsers, but not TestUser. And the api should reflect that fact.

[paneq]

I looked at your gist and what I dislike I exactly connected with my previous comment. The api and implementation looks like we want to have a pool of TestUsers but it is not our intention. We want TestUsers to get browsers from pool.

[jdudek]

Fixed in master.