Skip to content

Rescue LoadErrors during driver registration - #170

Merged
ronwsmith merged 2 commits into
oesmith:masterfrom
ahharvey:revert-register_drivers
Jul 29, 2016
Merged

ronwsmith merged 2 commits into
oesmith:masterfrom
ahharvey:revert-register_drivers

Conversation

@ahharvey

Copy link
Copy Markdown
Contributor

reverts lib/billy/browsers/capybara.rb to correctly rescue from LoadError

reverts lib/billy/browsers/capybara.rb to correctly rescue from LoadError
Comment thread lib/billy/browsers/capybara.rb Outdated
require driver rescue next
send("register_#{name}_driver")
begin
- require driver

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you left the - in here from the old diff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@AlanFoster

Copy link
Copy Markdown
Contributor

Confirmed that fix works locally for me 👍

@piotrb

piotrb commented Jul 28, 2016

Copy link
Copy Markdown

Came across this issue in my gem updates recently too, that looks like it does the original job once again since LoadError is not kind_of? RuntimeError which is what the new rescue seems to be doing. Please merge and push out a patch release with this change.

@AlanFoster

Copy link
Copy Markdown
Contributor

Is it possible to add a test for this scenario as a follow-up PR to ensure we have this code coverage? 👍

@piotrb

piotrb commented Jul 28, 2016

Copy link
Copy Markdown

I don't think so .. the failure is in missing a dependancy to begin with .. you'd literally need to test with some gems removed

@ronwsmith ronwsmith changed the title Update capybara.rb Rescue LoadErrors during driver registration Jul 29, 2016
@ronwsmith
ronwsmith merged commit 543b7b9 into oesmith:master Jul 29, 2016
@ronwsmith

Copy link
Copy Markdown
Collaborator

Releasing 0.9.1 right now. Thanks for the contribution.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants