Fixed Webdrivers::VersionError with chrome version greater than 115#249
Fixed Webdrivers::VersionError with chrome version greater than 115#249sadahiro-ono wants to merge 16 commits intotitusfortner:mainfrom
Conversation
9ef214c to
de1dffc
Compare
|
Thanks for the PR. It looks like it isn't backwards compatible. Can you look at why the tests are failing? |
|
@titusfortner |
|
@titusfortner |
|
Just need to change from |
|
|
8438f32 to
4378b7d
Compare
|
StandardError is the superclass, it is the value of the String it failed on. Not sure if it is happenstance or common that MS doesn't do releases for all platforms for all patch versions, but not good either way. |
titusfortner
left a comment
There was a problem hiding this comment.
- Thanks for putting this together and adding tests
- I really hate looking at old code I've written.
- We probably need to exclude
chromedriver.rbfromMetrics/ClassLengthin.rubocop.yml.
| "#{msg} A network issue is preventing determination of latest chromedriver release." | ||
| end | ||
|
|
||
| url = if version >= normalize_version('115') |
There was a problem hiding this comment.
We can use https://chromedriver.chromium.org/downloads/version-selection for the error message
| current_build_version == browser_build_version) | ||
| end | ||
|
|
||
| def chrome_for_testing_base_url |
There was a problem hiding this comment.
Not sure if it matters, but this breaks https://github.com/titusfortner/webdrivers/wiki/Using-with-VCR-or-WebMock
There was a problem hiding this comment.
Do we need to support a version that we cannot get from this endpoint?
I believe we can change the base_url to the API endpoint if we don't need it.
There was a problem hiding this comment.
yes, we still need to support the older versions
| 'linux64' | ||
| elsif System.platform == 'mac' | ||
| apple_filename(driver_version) | ||
| if driver_version >= normalize_version('115') |
There was a problem hiding this comment.
If we move this conditional to #apple_filename we don't need to add #apple_filename_api.
| allow(chromedriver).to receive(:exists?).and_return(false) | ||
|
|
||
| msg = %r{Can not reach https://chromedriver.storage.googleapis.com} | ||
| msg = %r{Can not reach https://googlechromelabs.github.io/chrome-for-testing/latest-patch-versions-per-build.json} |
There was a problem hiding this comment.
How about considering the both cases for v115+ and v114 or earlier? The Gem is expected to print "Can not reach ...", and the target URL is not important here.
| msg = %r{Can not reach https://googlechromelabs.github.io/chrome-for-testing/latest-patch-versions-per-build.json} | |
| msg = %r{Can not reach (?:https://googlechromelabs.github.io|https://chromedriver.storage.googleapis.com)} |
| allow(Net::HTTP).to receive(:get_response).and_raise(SocketError) | ||
|
|
||
| msg = %r{Can not reach https://chromedriver.storage.googleapis.com/} | ||
| msg = %r{Can not reach https://googlechromelabs.github.io/chrome-for-testing/latest-patch-versions-per-build.json} |
There was a problem hiding this comment.
| msg = %r{Can not reach https://googlechromelabs.github.io/chrome-for-testing/latest-patch-versions-per-build.json} | |
| msg = %r{Can not reach (?:https://googlechromelabs.github.io|https://chromedriver.storage.googleapis.com)} |
| allow(Net::HTTP).to receive(:get_response).and_raise(SocketError) | ||
|
|
||
| msg = %r{^Can not reach https://chromedriver.storage.googleapis.com} | ||
| msg = %r{^Can not reach https://googlechromelabs.github.io/chrome-for-testing/latest-patch-versions-per-build.json} |
There was a problem hiding this comment.
| msg = %r{^Can not reach https://googlechromelabs.github.io/chrome-for-testing/latest-patch-versions-per-build.json} | |
| msg = %r{Can not reach (?:https://googlechromelabs.github.io|https://chromedriver.storage.googleapis.com)} |
|
Merged here — 97fb1e2 Thank you! |
similar Issue: #247
f the version is 115 or higher, the API can be used, so if the version is 115 or higher, the stable version is obtained and downloaded from the API.
refarence