Drop Capybara.server_boot_timeout #589

Closed
joliss opened this Issue Jan 3, 2012 · 4 comments

Comments

Projects
None yet
2 participants
Collaborator

joliss commented Jan 3, 2012

/cc @madadam

Quoting my commit message in c444d33:

Remove broken tests for server_boot_timeout

When they were added in 09d6335, responsive? already only hit /__identify__, which does not hit the server, but gets special-cased in Capybara's Identify server class (and runs instantly).

So it's not clear what constitutes a "slow" server that might time out at boot, and thus why server_boot_timeout needs to be configurable. A server that is slow to respond to HTTP requests would not trigger the boot timeout. All we really seem to be waiting for within the timeout window is for Thin or WEBrick to come up.

It's not clear to me at this point that this was ever a useful option. Should we perhaps remove it for Capybara 2.0? If it comes up for anyone, we can re-add it in 2.x, this time with proper tests to show why it's needed.

Contributor

madadam commented Jan 3, 2012

Hey there. The motivation for introducing this option was this:

When your app is huge, it may take some time to boot. Then Capybara would timeout during booting the app up. This option was a solution (actually, a workaround) to this problem. I know that it's crappy solution, but it was quick to hack and it solved the problem I had. The tests are shit, though - they don't show the purpose of the feature properly.

Just wanted to clarify this. I don't use Capybara anymore, so don't care too much. Feel free to remove it if you think it's the right thing to do.

Collaborator

joliss commented Jan 3, 2012

Thanks for chipping in! I realize you probably don't have your app around anymore, but do you happen to know what specifically the huge application would have doing while the server booted?

I would expect that require'ing all the source files would have been done by this time, and all of it happens before the first HTTP request.

Another question: what value did you have to put for server_boot_timeout to make it work? I'm wondering if we could just set the timeout to 60 seconds (up from 10) and remove the option without making people unhappy.

Contributor

madadam commented Jan 3, 2012

It was a rails app with shitload of gems and plugins initializing stuff. Sorry I can't be more specific, I don't work on the app anymore so can't check. Neither do I remember the exact value we used. I'm almost sure it was less than 60.

I can imagine hardcoding a ridiculously high timeout value would work for most people. Or maybe removing the timeout completely and let Capybara wait until the app boots up, no matter how long it takes? In case it gets stuck, the user can always hit Ctrl-C or something. Not sure this is feasible/good idea though.

joliss closed this in c10d6a5 Jan 9, 2012

@jdelStrother jdelStrother pushed a commit to jdelStrother/capybara that referenced this issue Jan 9, 2012

@jnicklas jnicklas Merge pull request #601 from joliss/remove-server-boot-timeout
Remove Capybara.server_boot_timeout, hard-code 60 seconds. Closes #589
df95ebc
Collaborator

joliss commented Jan 9, 2012

Thanks Adam, that was really helpful!

(Closed by pull request #601: Remove Capybara.server_boot_timeout, hard-code 60 seconds.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment