More exception cases for attempt_connect race condition #60

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@cgriego
Contributor
cgriego commented May 25, 2011

In attempt_connect, Errno::ECONNREFUSED is handled silently but I've run into additional exceptions that are temporary that should also be handled silently. Namely Errno::EAFNOSUPPORT and Errno::EACCES. Errno::EAFNOSUPPORT can be raised when the Xvfb process isn't finished booting yet on Gentoo thanks to a race condition with the headless gem. I haven't been able to consistently recreate Errno::EACCES.

@jferris
Member
jferris commented May 25, 2011

Thanks - I'll take a look at this when I can. I'd look at a pull request sooner.

@cgriego
Contributor
cgriego commented May 26, 2011

Pull request added.

@jferris
Member
jferris commented Jun 14, 2011

Can you add a unit test for this so that it's documented? It's not realistic to meet the race condition, so just a stub that tests browser (you can add spec/browser_spec.rb) would suffice.

@cgriego
Contributor
cgriego commented Jun 16, 2011

I started adding this spec, but I was quickly ended up mocking so much that I question the value of the spec. I had to stub start_server on any instance to avoid launching an instance of webkit_server per test. Then I had to pass in a mock for socket_class that stub raising the exception. Then I'd need to stub Capybara.timeout to avoid having the 3 tests add 15 seconds to the test suite. Any suggestions?

@jferris
Member
jferris commented Jun 29, 2011

Yeah, stubbing/mocking at the browser level might be tough. The other thing you could do is add a new class that's responsible for handling the connection retries/timeout, and pass in the socket class. You could then stub the socket class to raise errors. If you have the retry class accept the timeout strategy as well, you won't need to stub out Capybara.timeout. If you want to try that, that's cool. Otherwise, I'll take a stab at it when I can. I don't want to have untested edge cases, though, and if the code is difficult to test, it needs to be refactored.

@mike-burns
Member

Hi @cgriego, any update on these tests?

Thanks,
-Mike

@mike-burns
Member

Closing due to inactivity. Please re-open or submit a new pull request as needed.

Thanks,
-Mike

@mike-burns mike-burns closed this Aug 26, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment