-
Notifications
You must be signed in to change notification settings - Fork 110
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
Chromedriver now checks Selenium::WebDriver::Chrome#path. #39
Chromedriver now checks Selenium::WebDriver::Chrome#path. #39
Conversation
195a69f
to
b8185e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work on this, I have a few suggestions here.
lib/webdrivers/chromedriver.rb
Outdated
`google-chrome --version` | ||
if browser_defined? | ||
Webdrivers.logger.debug "Browser executable: '#{browser_exe}'" | ||
return `#{browser_exe} --product-version`.strip if browser_defined? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return `#{browser_exe} --product-version`.strip if browser_defined? | |
return `#{browser_exe} --product-version`.strip |
… Chrome, and get full chromedriver version instead of just the major.
b8185e4
to
928e699
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge it. I've asked for clarification from lead chromedriver dev. We can update the code when we hear back if we have different defaults.
Selenium::WebDriver::Chrome#path
before defaulting to Google Chrome. Addresses [3.7.0] Doesn't work with chromium on Linux #38.chromedriver
v2.46 if Chrome/Chromium version is less than 70.LATEST_FILE_*
only available for versions > 70.Chromedriver#current_version
now returns full version -73.0.3683.68
instead of73.0
.