-
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
Fixed Webdrivers::VersionError with chrome version greater than 115 #249
Changes from all commits
fa6c87e
e19d997
0ad4015
9b0cb30
928481e
184e9a1
4e62a4d
2d2525e
30c7bb1
0a35745
369e301
345006d
5bd1aa6
cd456f9
4378b7d
55ddd51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,11 @@ def base_url | |
private | ||
|
||
def latest_point_release(version) | ||
normalize_version(Network.get(URI.join(base_url, "LATEST_RELEASE_#{version}"))) | ||
if version < normalize_version('115') | ||
return normalize_version(Network.get(URI.join(base_url, "LATEST_RELEASE_#{version}"))) | ||
end | ||
|
||
latest_patch_version(version) | ||
rescue NetworkError | ||
msg = "Unable to find latest point release version for #{version}." | ||
msg = begin | ||
|
@@ -77,8 +81,14 @@ def latest_point_release(version) | |
"#{msg} A network issue is preventing determination of latest chromedriver release." | ||
end | ||
|
||
url = if version >= normalize_version('115') | ||
'https://googlechromelabs.github.io/chrome-for-testing' | ||
else | ||
'https://chromedriver.storage.googleapis.com/index.html' | ||
end | ||
|
||
msg = "#{msg} Please set `Webdrivers::Chromedriver.required_version = <desired driver version>` "\ | ||
'to a known chromedriver version: https://chromedriver.storage.googleapis.com/index.html' | ||
"to a known chromedriver version: #{url}" | ||
Webdrivers.logger.debug msg | ||
raise VersionError, msg | ||
end | ||
|
@@ -105,8 +115,17 @@ def apple_filename(driver_version) | |
end | ||
end | ||
|
||
def apple_filename_for_api(driver_version) | ||
if apple_m1_compatible?(driver_version) | ||
driver_version >= normalize_version('106.0.5249.61') ? 'mac-arm64' : 'mac64-m1' | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else | ||
'mac-x64' | ||
end | ||
end | ||
|
||
def direct_url(driver_version) | ||
"#{base_url}/#{driver_version}/chromedriver_#{driver_filename(driver_version)}.zip" | ||
direct_url_from_api(driver_version) || | ||
"#{base_url}/#{driver_version}/chromedriver_#{driver_filename(driver_version)}.zip" | ||
end | ||
|
||
def driver_filename(driver_version) | ||
|
@@ -115,7 +134,11 @@ def driver_filename(driver_version) | |
elsif System.platform == 'linux' | ||
'linux64' | ||
elsif System.platform == 'mac' | ||
apple_filename(driver_version) | ||
if driver_version >= normalize_version('115') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we move this conditional to |
||
apple_filename_for_api(driver_version) | ||
else | ||
apple_filename(driver_version) | ||
end | ||
else | ||
raise 'Failed to determine driver filename to download for your OS.' | ||
end | ||
|
@@ -149,6 +172,33 @@ def sufficient_binary? | |
super && current_version && (current_version < normalize_version('70.0.3538') || | ||
current_build_version == browser_build_version) | ||
end | ||
|
||
def chrome_for_testing_base_url | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to support a version that we cannot get from this endpoint? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, we still need to support the older versions |
||
'https://googlechromelabs.github.io' | ||
end | ||
|
||
def latest_patch_version(driver_version) | ||
latest_patch_version = URI.join(chrome_for_testing_base_url, '/chrome-for-testing/latest-patch-versions-per-build.json') | ||
.then { |url| Network.get(url) } | ||
.then { |res| JSON.parse(res, symbolize_names: true) } | ||
.then { |json| json.dig(:builds, :"#{driver_version}", :version) } | ||
.then { |version| version ? normalize_version(version) : nil } | ||
raise NetworkError unless latest_patch_version | ||
|
||
latest_patch_version | ||
end | ||
|
||
def direct_url_from_api(driver_version) | ||
return if normalize_version(driver_version) < normalize_version('115') | ||
|
||
URI.join(chrome_for_testing_base_url, '/chrome-for-testing/known-good-versions-with-downloads.json') | ||
.then { |url| Network.get(url) } | ||
.then { |res| JSON.parse(res, symbolize_names: true) } | ||
.then { |json| json[:versions].find { |e| e[:version] == driver_version.to_s } } | ||
.then { |json| json.dig(:downloads, :chromedriver) } | ||
.then { |json| json.find { |e| e[:platform] == driver_filename(driver_version) } } | ||
.then { |json| json&.dig(:url) } | ||
end | ||
end | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -66,7 +66,7 @@ | |||||
allow(Net::HTTP).to receive(:get_response).and_raise(SocketError) | ||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
|
||||||
expect { chromedriver.update }.to raise_error(Webdrivers::ConnectionError, msg) | ||||||
end | ||||||
end | ||||||
|
@@ -101,7 +101,7 @@ | |||||
it 'raises ConnectionError if offline' do | ||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
expect { chromedriver.update }.to raise_error(Webdrivers::ConnectionError, msg) | ||||||
end | ||||||
end | ||||||
|
@@ -168,6 +168,24 @@ | |||||
chromedriver.update | ||||||
expect(Webdrivers::System).to have_received(:download).with(end_with('_mac64_m1.zip'), anything) | ||||||
end | ||||||
|
||||||
it 'uses the correct chromedriver filename suffix for versions greater than 115 for Intel' do | ||||||
allow(Webdrivers::System).to receive(:apple_m1_architecture?).and_return(false) | ||||||
allow(chromedriver).to receive(:latest_version).and_return(Gem::Version.new('115.0.5790.102')) | ||||||
chromedriver.required_version = nil | ||||||
|
||||||
chromedriver.update | ||||||
expect(Webdrivers::System).to have_received(:download).with(end_with('-mac-x64.zip'), anything) | ||||||
end | ||||||
|
||||||
it 'uses the correct chromedriver filename suffix for versions greater than 115 for Silicon' do | ||||||
allow(Webdrivers::System).to receive(:apple_m1_architecture?).and_return(true) | ||||||
allow(chromedriver).to receive(:latest_version).and_return(Gem::Version.new('115.0.5790.102')) | ||||||
chromedriver.required_version = nil | ||||||
|
||||||
chromedriver.update | ||||||
expect(Webdrivers::System).to have_received(:download).with(end_with('-mac-arm64.zip'), anything) | ||||||
end | ||||||
end | ||||||
end | ||||||
|
||||||
|
@@ -206,7 +224,7 @@ | |||||
msg = 'Unable to find latest point release version for 999.0.0. '\ | ||||||
'You appear to be using a non-production version of Chrome. '\ | ||||||
'Please set `Webdrivers::Chromedriver.required_version = <desired driver version>` '\ | ||||||
'to a known chromedriver version: https://chromedriver.storage.googleapis.com/index.html' | ||||||
'to a known chromedriver version: https://googlechromelabs.github.io/chrome-for-testing' | ||||||
|
||||||
expect { chromedriver.latest_version }.to raise_exception(Webdrivers::VersionError, msg) | ||||||
end | ||||||
|
@@ -223,11 +241,12 @@ | |||||
it 'raises ConnectionError when offline' do | ||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
expect { chromedriver.latest_version }.to raise_error(Webdrivers::ConnectionError, msg) | ||||||
end | ||||||
|
||||||
it 'creates cached file' do | ||||||
allow(chromedriver).to receive(:latest_patch_version).and_return(nil) | ||||||
allow(Webdrivers::Network).to receive(:get).and_return('71.0.3578.137') | ||||||
|
||||||
chromedriver.latest_version | ||||||
|
@@ -258,6 +277,17 @@ | |||||
expect(Webdrivers::Network).to have_received(:get) | ||||||
expect(Webdrivers::System).to have_received(:valid_cache?) | ||||||
end | ||||||
|
||||||
it 'call chrome_for_testing if the browser version is greater than 115' do | ||||||
allow(chromedriver).to receive(:browser_version).and_return Gem::Version.new('115.0.5790.102') | ||||||
allow(Webdrivers::Network).to receive(:get).and_return( | ||||||
{"builds": {'115.0.5790': {"version": '115.0.5790.102'}}}.to_json | ||||||
) | ||||||
uri = URI.join('https://googlechromelabs.github.io', '/chrome-for-testing/latest-patch-versions-per-build.json') | ||||||
|
||||||
expect(chromedriver.latest_version).to eq Gem::Version.new('115.0.5790.102') | ||||||
expect(Webdrivers::Network).to have_received(:get).with(uri) | ||||||
end | ||||||
end | ||||||
|
||||||
describe '#required_version=' do | ||||||
|
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.
We can use https://chromedriver.chromium.org/downloads/version-selection for the error message