Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix access to specializations when Selenium::Driver is subclassed #2185

Conversation

floehopper
Copy link
Contributor

This problem was introduced in this commit (i.e. since the v3.17.0 release) and is causing NoMethodError exceptions in all the JS-related feature specs in an app I'm working on.

The specializations are stored in a class instance variable. However, they are accessed from the #specialize_driver method via self.class.

Previously if this method was called within an instance of a subclass of Selenium::Driver, then it accessed the class instance variable associated with the subclass (which is nil by default) and not the class instance variable associated with the Selenium::Driver class as intended resulting in an exception like this:

 NoMethodError:
   private method `select' called for nil:NilClass
   ./lib/capybara/selenium/driver.rb:417:in `specialize_driver'
   ./lib/capybara/selenium/driver.rb:49:in `browser'
   ./lib/capybara/selenium/driver.rb:66:in `visit'
   ./lib/capybara/session.rb:277:in `visit'
   ./spec/selenium_spec_chrome.rb:118:in `block (3 levels) in <top (required)>'

This commit adds a spec using a subclass of Selenium::Driver with Chrome to demonstrate the problem and fixes it by changing the #specialize_driver method to access the specializations via the explicitly specified class name rather than via self.class.

An alternative fix would be to use a class variable for specializations and do away with the class attribute reader altogether.

The specializations are stored in a class instance variable. However,
they are accessed from the specialize_driver method via `self.class`.

Previously if this method was called on an instance of a subclass of
Selenium::Driver, then it accessed the class instance variable
associated with the subclass (which is nil by default) and not the class
instance variable associated with the Selenium::Driver class as intended
resulting in an exception like this:

     NoMethodError:
       private method `select' called for nil:NilClass
       ./lib/capybara/selenium/driver.rb:417:in `specialize_driver'
       ./lib/capybara/selenium/driver.rb:49:in `browser'
       ./lib/capybara/selenium/driver.rb:66:in `visit'
       ./lib/capybara/session.rb:277:in `visit'
       ./spec/selenium_spec_chrome.rb:118:in `block (3 levels) in <top (required)>'

This commit adds a spec using a subclass of Selenium::Driver with Chrome
to demonstrate the problem and fixes it by changing the
specialize_driver method to access the specializations via the
explicitly specified class name.

An alternative fix would be to use a class variable for specializations
and do away with the class attribute reader altogether.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6049

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.008%) to 98.668%

Files with Coverage Reduction New Missed Lines %
lib/capybara/window.rb 1 93.18%
Totals Coverage Status
Change from base Build 6044: -0.008%
Covered Lines: 12888
Relevant Lines: 13062

💛 - Coveralls

@twalpole
Copy link
Member

twalpole commented May 11, 2019

Thanks for this! Going forward I think I'd like this to be implemented in a way that allows for subclasses to override specializations but the current implementation fixes the current issue without really losing any current functionality so I'll merge it and release a 3.19.1

@twalpole twalpole merged commit 60c3ac1 into teamcapybara:master May 11, 2019
@floehopper
Copy link
Contributor Author

Thanks! 👍

@floehopper floehopper deleted the fix-access-to-specializations-when-selenium-driver-is-subclassed branch May 11, 2019 20:00
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.

None yet

3 participants