From 9f5123661fc66815054b344d9ead76b05e94dd1d Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Mon, 15 Apr 2024 03:14:20 -0500 Subject: [PATCH] [rb] update SOC for driver finder and selenium manager classes (#13386) [rb] move driver finding logic out of selenium manager class Co-authored-by: Diego Molina --- .../webdriver/common/driver_finder.rb | 67 +++-- .../selenium/webdriver/common/local_driver.rb | 9 +- rb/lib/selenium/webdriver/common/platform.rb | 4 +- .../webdriver/common/selenium_manager.rb | 109 +++----- rb/lib/selenium/webdriver/common/service.rb | 2 +- .../selenium/webdriver/chrome/service_spec.rb | 2 +- .../selenium/webdriver/edge/service_spec.rb | 2 +- .../webdriver/firefox/service_spec.rb | 2 +- .../selenium/webdriver/chrome/driver_spec.rb | 25 +- .../selenium/webdriver/chrome/service_spec.rb | 9 +- .../webdriver/common/driver_finder_spec.rb | 234 ++++-------------- .../webdriver/common/selenium_manager_spec.rb | 126 +++------- .../selenium/webdriver/edge/driver_spec.rb | 21 +- .../selenium/webdriver/edge/service_spec.rb | 9 +- .../selenium/webdriver/firefox/driver_spec.rb | 17 +- .../webdriver/firefox/service_spec.rb | 9 +- .../unit/selenium/webdriver/ie/driver_spec.rb | 17 +- .../selenium/webdriver/ie/service_spec.rb | 9 +- .../selenium/webdriver/safari/driver_spec.rb | 17 +- .../selenium/webdriver/safari/service_spec.rb | 9 +- 20 files changed, 248 insertions(+), 451 deletions(-) diff --git a/rb/lib/selenium/webdriver/common/driver_finder.rb b/rb/lib/selenium/webdriver/common/driver_finder.rb index 71477bac995f8..3f5aa4a1af648 100644 --- a/rb/lib/selenium/webdriver/common/driver_finder.rb +++ b/rb/lib/selenium/webdriver/common/driver_finder.rb @@ -20,25 +20,64 @@ module Selenium module WebDriver class DriverFinder - def self.path(options, klass) - path = klass.driver_path - path = path.call if path.is_a?(Proc) + def initialize(options, service) + @options = options + @service = service + end + + def browser_path + paths[:browser_path] + end + + def driver_path + paths[:driver_path] + end + + def browser_path? + !browser_path.nil? && !browser_path.empty? + end + + private - path ||= begin - SeleniumManager.driver_path(options) unless options.is_a?(Remote::Capabilities) + def paths + @paths ||= begin + path = @service.class.driver_path + path = path.call if path.is_a?(Proc) + exe = @service.class::EXECUTABLE + if path + WebDriver.logger.debug("Skipping Selenium Manager; path to #{exe} specified in service class: #{path}") + Platform.assert_executable(path) + {driver_path: path} + else + output = SeleniumManager.binary_paths(*to_args) + formatted = {driver_path: Platform.cygwin_path(output['driver_path'], only_cygwin: true), + browser_path: Platform.cygwin_path(output['browser_path'], only_cygwin: true)} + Platform.assert_executable(formatted[:driver_path]) + Platform.assert_executable(formatted[:browser_path]) + formatted + end rescue StandardError => e - raise Error::NoSuchDriverError, "Unable to obtain #{klass::EXECUTABLE} using Selenium Manager; #{e.message}" + WebDriver.logger.error("Exception occurred: #{e.message}") + WebDriver.logger.error("Backtrace:\n\t#{e.backtrace&.join("\n\t")}") + raise Error::NoSuchDriverError, "Unable to obtain #{exe}" end + end - begin - Platform.assert_executable(path) - rescue TypeError - raise Error::NoSuchDriverError, "Unable to locate or obtain #{klass::EXECUTABLE}" - rescue Error::WebDriverError => e - raise Error::NoSuchDriverError, "#{klass::EXECUTABLE} located, but: #{e.message}" + def to_args + args = ['--browser', @options.browser_name] + if @options.browser_version + args << '--browser-version' + args << @options.browser_version end - - path + if @options.respond_to?(:binary) && !@options.binary.nil? + args << '--browser-path' + args << @options.binary.gsub('\\', '\\\\\\') + end + if @options.proxy + args << '--proxy' + args << (@options.proxy.ssl || @options.proxy.http) + end + args end end end diff --git a/rb/lib/selenium/webdriver/common/local_driver.rb b/rb/lib/selenium/webdriver/common/local_driver.rb index f2f48d3c5aba8..0e20ca01d8ecc 100644 --- a/rb/lib/selenium/webdriver/common/local_driver.rb +++ b/rb/lib/selenium/webdriver/common/local_driver.rb @@ -38,7 +38,14 @@ def process_options(options, service) raise ArgumentError, ":options must be an instance of #{default_options.class}" end - service.executable_path ||= WebDriver::DriverFinder.path(options, service.class) + service.executable_path ||= begin + finder = WebDriver::DriverFinder.new(options, service) + if options.respond_to?(:binary) && finder.browser_path? + options.binary = finder.browser_path + options.browser_version = nil + end + finder.driver_path + end options.as_json end end diff --git a/rb/lib/selenium/webdriver/common/platform.rb b/rb/lib/selenium/webdriver/common/platform.rb index 65f498e8b5c0b..4d326b9fee5ce 100644 --- a/rb/lib/selenium/webdriver/common/platform.rb +++ b/rb/lib/selenium/webdriver/common/platform.rb @@ -111,7 +111,9 @@ def wrap_in_quotes_if_necessary(str) windows? && !cygwin? ? %("#{str}") : str end - def cygwin_path(path, **opts) + def cygwin_path(path, only_cygwin: false, **opts) + return path if only_cygwin && !cygwin? + flags = [] opts.each { |k, v| flags << "--#{k}" if v } diff --git a/rb/lib/selenium/webdriver/common/selenium_manager.rb b/rb/lib/selenium/webdriver/common/selenium_manager.rb index d2d217aebb6f4..f14f12c15b1c9 100644 --- a/rb/lib/selenium/webdriver/common/selenium_manager.rb +++ b/rb/lib/selenium/webdriver/common/selenium_manager.rb @@ -34,88 +34,33 @@ def bin_path @bin_path ||= '../../../../../bin' end - # @param [Options] options browser options. - # @return [String] the path to the correct driver. - def driver_path(options) - command = generate_command(binary, options) - - output = run(*command) - - browser_path = Platform.cygwin? ? Platform.cygwin_path(output['browser_path']) : output['browser_path'] - driver_path = Platform.cygwin? ? Platform.cygwin_path(output['driver_path']) : output['driver_path'] - Platform.assert_executable driver_path - - if options.respond_to?(:binary) && browser_path && !browser_path.empty? - options.binary = browser_path - options.browser_version = nil - end - - driver_path + # @param [Array] arguments what gets sent to to Selenium Manager binary. + # @return [Hash] paths to the requested assets. + def binary_paths(*arguments) + arguments += %w[--language-binding ruby] + arguments += %w[--output json] + arguments << '--debug' if WebDriver.logger.debug? + + run(binary, *arguments) end private - def generate_command(binary, options) - command = [binary, '--browser', options.browser_name] - if options.browser_version - command << '--browser-version' - command << options.browser_version - end - if options.respond_to?(:binary) && !options.binary.nil? - command << '--browser-path' - command << options.binary.squeeze('\\').gsub('\\', '\\\\\\') - end - if options.proxy - command << '--proxy' - command << (options.proxy.ssl || options.proxy.http) - end - command - end - # @return [String] the path to the correct selenium manager def binary @binary ||= begin - location = ENV.fetch('SE_MANAGER_PATH', begin - directory = File.expand_path(bin_path, __FILE__) - if Platform.windows? - "#{directory}/windows/selenium-manager.exe" - elsif Platform.mac? - "#{directory}/macos/selenium-manager" - elsif Platform.linux? - "#{directory}/linux/selenium-manager" - elsif Platform.unix? - WebDriver.logger.warn('Selenium Manager binary may not be compatible with Unix; verify settings', - id: %i[selenium_manager unix_binary]) - "#{directory}/linux/selenium-manager" - end - rescue Error::WebDriverError => e - raise Error::WebDriverError, "Unable to obtain Selenium Manager binary for #{e.message}" - end) + if (location = ENV.fetch('SE_MANAGER_PATH', nil)) + WebDriver.logger.debug("Selenium Manager set by ENV['SE_MANAGER_PATH']: #{location}") + end + location ||= platform_location - validate_location(location) - location - end - end - - def validate_location(location) - begin - Platform.assert_file(location) Platform.assert_executable(location) - rescue TypeError - raise Error::WebDriverError, - "Unable to locate or obtain Selenium Manager binary; #{location} is not a valid file object" - rescue Error::WebDriverError => e - raise Error::WebDriverError, "Selenium Manager binary located, but #{e.message}" + WebDriver.logger.debug("Selenium Manager binary found at #{location}", id: :selenium_manager) + location end - - WebDriver.logger.debug("Selenium Manager binary found at #{location}", id: :selenium_manager) end def run(*command) - command += %w[--language-binding ruby] - command += %w[--output json] - command << '--debug' if WebDriver.logger.debug? - WebDriver.logger.debug("Executing Process #{command}", id: :selenium_manager) begin @@ -124,16 +69,34 @@ def run(*command) raise Error::WebDriverError, "Unsuccessful command executed: #{command}; #{e.message}" end - json_output = stdout.empty? ? {} : JSON.parse(stdout) - (json_output['logs'] || []).each do |log| + json_output = stdout.empty? ? {'logs' => [], 'result' => {}} : JSON.parse(stdout) + json_output['logs'].each do |log| level = log['level'].casecmp('info').zero? ? 'debug' : log['level'].downcase WebDriver.logger.send(level, log['message'], id: :selenium_manager) end result = json_output['result'] - return result unless status.exitstatus.positive? + return result unless status.exitstatus.positive? || result.nil? - raise Error::WebDriverError, "Unsuccessful command executed: #{command}\n#{result}#{stderr}" + raise Error::WebDriverError, + "Unsuccessful command executed: #{command} - Code #{status.exitstatus}\n#{result}\n#{stderr}" + end + + def platform_location + directory = File.expand_path(bin_path, __FILE__) + if Platform.windows? + "#{directory}/windows/selenium-manager.exe" + elsif Platform.mac? + "#{directory}/macos/selenium-manager" + elsif Platform.linux? + "#{directory}/linux/selenium-manager" + elsif Platform.unix? + WebDriver.logger.warn('Selenium Manager binary may not be compatible with Unix', + id: %i[selenium_manager unix_binary]) + "#{directory}/linux/selenium-manager" + else + raise Error::WebDriverError, "unsupported platform: #{Platform.os}" + end end end end # SeleniumManager diff --git a/rb/lib/selenium/webdriver/common/service.rb b/rb/lib/selenium/webdriver/common/service.rb index e1266a7350ea4..e20f7569db28c 100644 --- a/rb/lib/selenium/webdriver/common/service.rb +++ b/rb/lib/selenium/webdriver/common/service.rb @@ -89,7 +89,7 @@ def initialize(path: nil, port: nil, log: nil, args: nil) def launch @executable_path ||= begin default_options = WebDriver.const_get("#{self.class.name&.split('::')&.[](2)}::Options").new - DriverFinder.path(default_options, self.class) + DriverFinder.new(default_options, self).driver_path end ServiceManager.new(self).tap(&:start) end diff --git a/rb/spec/integration/selenium/webdriver/chrome/service_spec.rb b/rb/spec/integration/selenium/webdriver/chrome/service_spec.rb index 2815dc5f4ce8e..fe594c1132eb4 100644 --- a/rb/spec/integration/selenium/webdriver/chrome/service_spec.rb +++ b/rb/spec/integration/selenium/webdriver/chrome/service_spec.rb @@ -29,7 +29,7 @@ module Chrome after { service_manager.stop } it 'auto uses chromedriver' do - service.executable_path = DriverFinder.path(Options.new, described_class) + service.executable_path = DriverFinder.new(Options.new, described_class.new).driver_path expect(service_manager.uri).to be_a(URI) end diff --git a/rb/spec/integration/selenium/webdriver/edge/service_spec.rb b/rb/spec/integration/selenium/webdriver/edge/service_spec.rb index 457c89d2f096c..cb13efaf4d904 100644 --- a/rb/spec/integration/selenium/webdriver/edge/service_spec.rb +++ b/rb/spec/integration/selenium/webdriver/edge/service_spec.rb @@ -29,7 +29,7 @@ module Edge after { service_manager.stop } it 'auto uses edgedriver' do - service.executable_path = DriverFinder.path(Options.new, described_class) + service.executable_path = DriverFinder.new(Options.new, described_class.new).driver_path expect(service_manager.uri).to be_a(URI) end diff --git a/rb/spec/integration/selenium/webdriver/firefox/service_spec.rb b/rb/spec/integration/selenium/webdriver/firefox/service_spec.rb index cca6388d92e1d..74fac7f65e3af 100644 --- a/rb/spec/integration/selenium/webdriver/firefox/service_spec.rb +++ b/rb/spec/integration/selenium/webdriver/firefox/service_spec.rb @@ -29,7 +29,7 @@ module Firefox after { service_manager.stop } it 'auto uses geckodriver' do - service.executable_path = DriverFinder.path(Options.new, described_class) + service.executable_path = DriverFinder.new(Options.new, described_class.new).driver_path expect(service_manager.uri).to be_a(URI) end diff --git a/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb b/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb index d81dc0080bcc3..03d1b0d69f907 100644 --- a/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb @@ -33,6 +33,7 @@ module Chrome body: {value: {sessionId: 0, capabilities: {browserName: 'chrome'}}}.to_json, headers: {content_type: 'application/json'}} end + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } def expect_request(body: nil, endpoint: nil) body = (body || {capabilities: {alwaysMatch: {browserName: 'chrome', 'goog:chromeOptions': {}}}}).to_json @@ -45,37 +46,32 @@ def expect_request(body: nil, endpoint: nil) end it 'uses DriverFinder when provided Service without path' do - allow(DriverFinder).to receive(:path) + allow(DriverFinder).to receive(:new).and_return(finder) expect_request options = Options.new described_class.new(service: service, options: options) - expect(DriverFinder).to have_received(:path).with(options, service.class) + expect(finder).to have_received(:driver_path) end it 'does not use DriverFinder when provided Service with path' do expect_request allow(service).to receive(:executable_path).and_return('path') - allow(DriverFinder).to receive(:path) + allow(DriverFinder).to receive(:new).and_return(finder) described_class.new(service: service) - expect(DriverFinder).not_to have_received(:path) + expect(finder).not_to have_received(:driver_path) end it 'does not require any parameters' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - + allow(DriverFinder).to receive(:new).and_return(finder) expect_request expect { described_class.new }.not_to raise_exception end it 'accepts provided Options as sole parameter' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) opts = {args: ['-f']} expect_request(body: {capabilities: {alwaysMatch: {browserName: 'chrome', 'goog:chromeOptions': opts}}}) @@ -83,6 +79,13 @@ def expect_request(body: nil, endpoint: nil) expect { described_class.new(options: Options.new(**opts)) }.not_to raise_exception end + it 'raises an ArgumentError if parameter is not recognized' do + allow(DriverFinder).to receive(:new).and_return(finder) + + msg = 'unknown keyword: :invalid' + expect { described_class.new(invalid: 'foo') }.to raise_error(ArgumentError, msg) + end + it 'does not accept Options of the wrong class' do expect { described_class.new(options: Options.firefox) diff --git a/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb b/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb index 7ad3581a0563d..c2f2ebf241841 100644 --- a/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb @@ -91,6 +91,7 @@ module Chrome end let(:service_manager) { instance_double(ServiceManager, uri: 'http://example.com') } let(:bridge) { instance_double(Remote::Bridge, quit: nil, create_session: {}) } + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } before do allow(Remote::Bridge).to receive(:new).and_return(bridge) @@ -104,9 +105,7 @@ module Chrome end it 'is created when :url is not provided' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new).and_return(service) driver.new @@ -114,9 +113,7 @@ module Chrome end it 'accepts :service without creating a new instance' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new) driver.new(service: service) diff --git a/rb/spec/unit/selenium/webdriver/common/driver_finder_spec.rb b/rb/spec/unit/selenium/webdriver/common/driver_finder_spec.rb index 74405d24b981e..fafa0803f61f1 100644 --- a/rb/spec/unit/selenium/webdriver/common/driver_finder_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/driver_finder_spec.rb @@ -22,209 +22,69 @@ module Selenium module WebDriver describe DriverFinder do - context 'when Chrome' do - let(:options) { Options.chrome } - let(:service) { Chrome::Service } - let(:driver) { 'chromedriver' } + it 'class path accepts a String without calling Selenium Manager' do + allow(Chrome::Service).to receive(:driver_path).and_return('path') + allow(SeleniumManager).to receive(:binary_paths) + allow(Platform).to receive(:assert_executable).with('path').and_return(true) - after { Chrome::Service.driver_path = nil } + described_class.new(Options.chrome, Service.chrome).driver_path - it 'accepts path set on class as String' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = 'path' - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path').exactly(2).times - end - - it 'accepts path set on class as proc' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = proc { 'path' } - - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path') - end - - it 'gives original error if not found by Selenium Manager' do - allow(SeleniumManager).to receive(:driver_path).and_raise(Error::WebDriverError) - - expect { - described_class.path(options, service) - }.to raise_error(WebDriver::Error::NoSuchDriverError, %r{errors/driver_location}) - end + expect(SeleniumManager).not_to have_received(:binary_paths) + expect(Platform).to have_received(:assert_executable).with('path') end - context 'when Edge' do - let(:options) { Options.edge } - let(:service) { Edge::Service } - let(:driver) { 'msedgedriver' } - - after { service.driver_path = nil } - - it 'accepts path set on class as String' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = 'path' - described_class.path(options, service) + it 'class path accepts a proc without calling Selenium Manager' do + allow(Chrome::Service).to receive(:driver_path).and_return(proc { 'path' }) + allow(SeleniumManager).to receive(:binary_paths) + allow(Platform).to receive(:assert_executable).with('path').and_return(true) - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path').exactly(2).times - end + described_class.new(Options.chrome, Service.chrome).driver_path - it 'accepts path set on class as proc' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = proc { 'path' } - - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path') - end - - it 'gives original error if not found by Selenium Manager' do - allow(SeleniumManager).to receive(:driver_path).and_raise(Error::WebDriverError) - - expect { - described_class.path(options, service) - }.to raise_error(WebDriver::Error::NoSuchDriverError, %r{errors/driver_location}) - end + expect(SeleniumManager).not_to have_received(:binary_paths) + expect(Platform).to have_received(:assert_executable).with('path') end - context 'when Firefox' do - let(:options) { Options.firefox } - let(:service) { Firefox::Service } - let(:driver) { 'geckodriver' } - - after { service.driver_path = nil } - - it 'accepts path set on class as String' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = 'path' - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path').exactly(2).times - end - - it 'accepts path set on class as proc' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = proc { 'path' } + it 'validates all returned files' do + allow(SeleniumManager).to receive(:binary_paths).and_return({'browser_path' => '/path/to/browser', + 'driver_path' => '/path/to/driver'}) + allow(Platform).to receive(:assert_executable).with('/path/to/browser').and_return(true) + allow(Platform).to receive(:assert_executable).with('/path/to/driver').and_return(true) - described_class.path(options, service) + described_class.new(Options.chrome, Service.chrome).driver_path - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path') - end - - it 'gives original error if not found by Selenium Manager' do - allow(SeleniumManager).to receive(:driver_path).and_raise(Error::WebDriverError) - - expect { - described_class.path(options, service) - }.to raise_error(WebDriver::Error::NoSuchDriverError, %r{errors/driver_location}) - end + expect(Platform).to have_received(:assert_executable).with('/path/to/browser') + expect(Platform).to have_received(:assert_executable).with('/path/to/driver') end - context 'when IE' do - let(:options) { Options.ie } - let(:service) { IE::Service } - let(:driver) { 'IEDriverServer' } - - after { service.driver_path = nil } - - it 'accepts path set on class as String' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = 'path' - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path').exactly(2).times - end - - it 'accepts path set on class as proc' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = proc { 'path' } - - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path') - end - - it 'gives original error if not found by Selenium Manager' do - allow(SeleniumManager).to receive(:driver_path).and_raise(Error::WebDriverError) + it 'wraps error with NoSuchDriverError' do + allow(SeleniumManager).to receive(:binary_paths).and_raise(Error::WebDriverError, 'this error') + expect { expect { - described_class.path(options, service) - }.to raise_error(WebDriver::Error::NoSuchDriverError, %r{errors/driver_location}) - end + described_class.new(Options.chrome, Service.chrome).driver_path + }.to output(/Exception occurred: this error/).to_stderr_from_any_process + }.to raise_error(WebDriver::Error::NoSuchDriverError, + /Unable to obtain chromedriver; For documentation on this error/) end - context 'when Safari' do - let(:options) { Options.safari } - let(:service) { Safari::Service } - let(:driver) { 'safaridriver' } - - after { service.driver_path = nil } - - it 'accepts path set on class as String' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = 'path' - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path').exactly(2).times - end - - it 'accepts path set on class as proc' do - allow(SeleniumManager).to receive(:driver_path) - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) - - service.driver_path = proc { 'path' } - - described_class.path(options, service) - - expect(SeleniumManager).not_to have_received(:driver_path) - expect(Platform).to have_received(:assert_executable).with('path') - end - - it 'gives original error if not found by Selenium Manager' do - allow(SeleniumManager).to receive(:driver_path).and_raise(Error::WebDriverError) - - expect { - described_class.path(options, service) - }.to raise_error(WebDriver::Error::NoSuchDriverError, %r{errors/driver_location}) - end + it 'creates arguments' do + allow(SeleniumManager).to receive(:binary_paths).and_return({'browser_path' => '/path/to/browser', + 'driver_path' => '/path/to/driver'}) + proxy = instance_double(Proxy, ssl: 'proxy') + options = Options.chrome(browser_version: 'stable', proxy: proxy, binary: 'path/to/browser') + allow(Platform).to receive(:assert_executable).with('/path/to/browser').and_return(true) + allow(Platform).to receive(:assert_executable).with('/path/to/driver').and_return(true) + + described_class.new(options, Service.chrome).driver_path + + expect(SeleniumManager).to have_received(:binary_paths).with('--browser', + options.browser_name, + '--browser-version', + options.browser_version, + '--browser-path', + options.binary, + '--proxy', + options.proxy.ssl) end end end diff --git a/rb/spec/unit/selenium/webdriver/common/selenium_manager_spec.rb b/rb/spec/unit/selenium/webdriver/common/selenium_manager_spec.rb index c115c0ab9715f..d6816c14ac93b 100644 --- a/rb/spec/unit/selenium/webdriver/common/selenium_manager_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/selenium_manager_spec.rb @@ -23,34 +23,36 @@ module Selenium module WebDriver describe SeleniumManager do describe '.binary' do - def stub_binary(binary) - allow(File).to receive(:exist?).with(a_string_ending_with(binary)).and_return(true) - allow(File).to receive(:executable?).with(a_string_ending_with(binary)).and_return(true) - end - before do described_class.instance_variable_set(:@binary, nil) end + it 'uses environment variable' do + allow(Platform).to receive(:assert_executable).with('/path/to/selenium-manager').and_return(true) + allow(ENV).to receive(:fetch).with('SE_MANAGER_PATH', nil).and_return('/path/to/selenium-manager') + + expect(described_class.send(:binary)).to match(%r{/path/to/selenium-manager}) + end + it 'detects Windows' do - stub_binary('/windows/selenium-manager.exe') - allow(Platform).to receive(:assert_file) + allow(Platform).to receive(:assert_executable).with(a_string_ending_with('/windows/selenium-manager.exe')) + .and_return(true) allow(Platform).to receive(:windows?).and_return(true) expect(described_class.send(:binary)).to match(%r{/windows/selenium-manager\.exe$}) end it 'detects Mac' do - stub_binary('/macos/selenium-manager') - allow(Platform).to receive(:assert_file) + allow(Platform).to receive(:assert_executable).with(a_string_ending_with('/macos/selenium-manager')) + .and_return(true) allow(Platform).to receive_messages(windows?: false, mac?: true) expect(described_class.send(:binary)).to match(%r{/macos/selenium-manager$}) end it 'detects Linux' do - stub_binary('/linux/selenium-manager') - allow(Platform).to receive(:assert_file) + allow(Platform).to receive(:assert_executable).with(a_string_ending_with('/linux/selenium-manager')) + .and_return(true) allow(Platform).to receive_messages(windows?: false, mac?: false, linux?: true) expect(described_class.send(:binary)).to match(%r{/linux/selenium-manager$}) @@ -59,94 +61,44 @@ def stub_binary(binary) it 'errors if cannot find' do allow(File).to receive(:exist?).with(a_string_including('selenium-manager')).and_return(false) - expect { - described_class.send(:binary) - }.to raise_error(Error::WebDriverError, /Selenium Manager binary located, but not a file/) + expect { described_class.send(:binary) }.to raise_error(Error::WebDriverError, /not a file/) end end - describe 'self.run' do - it 'errors if a problem with command' do - expect { - described_class.send(:run, 'anything') - }.to raise_error(Error::WebDriverError, /Unsuccessful command executed: /) - end - end - - describe 'self.driver_path' do - it 'determines browser name by default' do - allow(described_class).to receive_messages(run: {'browser_path' => '', 'driver_path' => ''}, - binary: 'selenium-manager') - allow(Platform).to receive(:assert_executable) - - described_class.driver_path(Options.chrome) - - expect(described_class).to have_received(:run) - .with('selenium-manager', '--browser', 'chrome') - end - - it 'uses browser version if specified' do - allow(described_class).to receive_messages(run: {'browser_path' => '', 'driver_path' => ''}, - binary: 'selenium-manager') - allow(Platform).to receive(:assert_executable) - options = Options.chrome(browser_version: 1) - - described_class.driver_path(options) - - expect(described_class).to have_received(:run) - .with('selenium-manager', - '--browser', 'chrome', - '--browser-version', 1) - end - - it 'uses proxy if specified' do - proxy = Selenium::WebDriver::Proxy.new(ssl: 'proxy') - allow(described_class).to receive_messages(run: {'browser_path' => '', 'driver_path' => ''}, - binary: 'selenium-manager') - allow(Platform).to receive(:assert_executable) - options = Options.chrome(proxy: proxy) - - described_class.driver_path(options) + describe '.run' do + it 'returns result if positive exit status' do + status = instance_double(Process::Status, exitstatus: 0) + stdout = '{"result": "value", "logs": []}' + allow(Open3).to receive(:capture3).and_return([stdout, 'stderr', status]) - expect(described_class).to have_received(:run) - .with('selenium-manager', - '--browser', 'chrome', - '--proxy', 'proxy') + expect(described_class.send(:run, 'anything')).to eq 'value' end - it 'uses browser location if specified' do - allow(described_class).to receive_messages(run: {'browser_path' => '', 'driver_path' => ''}, - binary: 'selenium-manager') - allow(Platform).to receive(:assert_executable) - options = Options.chrome(binary: '/path/to/browser') - - described_class.driver_path(options) + it 'errors if a problem with command' do + allow(Open3).to receive(:capture3).and_raise(StandardError) - expect(described_class).to have_received(:run) - .with('selenium-manager', '--browser', 'chrome', '--browser-path', '/path/to/browser') + expect { + described_class.send(:run, 'anything') + }.to raise_error(Error::WebDriverError, /Unsuccessful command executed: \["anything"\]/) end - it 'properly escapes plain spaces in browser location' do - allow(described_class).to receive_messages(run: {'browser_path' => 'a', 'driver_path' => ''}, - binary: 'selenium-manager') - allow(Platform).to receive(:assert_executable) - options = Options.chrome(binary: '/path to/the/browser') + it 'errors if exit status greater than 0' do + status = instance_double(Process::Status, exitstatus: 1) + stdout = '{"result": "value", "logs": []}' + allow(Open3).to receive(:capture3).and_return([stdout, 'stderr', status]) - described_class.driver_path(options) - - expect(described_class).to have_received(:run) - .with('selenium-manager', '--browser', 'chrome', - '--browser-path', '/path to/the/browser') + msg = /Unsuccessful command executed: \["anything"\] - Code 1\nvalue\nstderr/ + expect { + described_class.send(:run, 'anything') + }.to raise_error(Error::WebDriverError, msg) end + end - it 'sets binary location on options' do - allow(described_class).to receive_messages(run: {'browser_path' => 'foo', 'driver_path' => ''}, - binary: 'selenium-manager') - allow(Platform).to receive(:assert_executable) - options = Options.chrome - - described_class.driver_path(options) - expect(options.binary).to eq 'foo' + describe '.binary_paths' do + it 'returns exact output from #run' do + return_map = {} + allow(described_class).to receive_messages(binary: 'binary', run: return_map) + expect(described_class.binary_paths('something')).to eq(return_map) end end end diff --git a/rb/spec/unit/selenium/webdriver/edge/driver_spec.rb b/rb/spec/unit/selenium/webdriver/edge/driver_spec.rb index bbf3fd7219a17..bfc15d8833406 100644 --- a/rb/spec/unit/selenium/webdriver/edge/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/edge/driver_spec.rb @@ -33,6 +33,7 @@ module Edge body: {value: {sessionId: 0, capabilities: {browserName: 'MicrosoftEdge'}}}.to_json, headers: {content_type: 'application/json'}} end + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } def expect_request(body: nil, endpoint: nil) body = (body || {capabilities: {alwaysMatch: {browserName: 'MicrosoftEdge', 'ms:edgeOptions': {}}}}).to_json @@ -45,36 +46,32 @@ def expect_request(body: nil, endpoint: nil) end it 'uses DriverFinder when provided Service without path' do + allow(DriverFinder).to receive(:new).and_return(finder) expect_request - allow(DriverFinder).to receive(:path) options = Options.new described_class.new(service: service, options: options) - expect(DriverFinder).to have_received(:path).with(options, service.class) + expect(finder).to have_received(:driver_path) end it 'does not use DriverFinder when provided Service with path' do expect_request allow(service).to receive(:executable_path).and_return('path') - allow(DriverFinder).to receive(:path) + allow(DriverFinder).to receive(:new).and_return(finder) described_class.new(service: service) - expect(DriverFinder).not_to have_received(:path) + expect(finder).not_to have_received(:driver_path) end it 'does not require any parameters' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) expect_request expect { described_class.new }.not_to raise_exception end it 'accepts provided Options as sole parameter' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) opts = {args: ['-f']} expect_request(body: {capabilities: {alwaysMatch: {browserName: 'MicrosoftEdge', 'ms:edgeOptions': opts}}}) @@ -82,9 +79,7 @@ def expect_request(body: nil, endpoint: nil) end it 'raises an ArgumentError if parameter is not recognized' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) msg = 'unknown keyword: :invalid' expect { described_class.new(invalid: 'foo') }.to raise_error(ArgumentError, msg) end diff --git a/rb/spec/unit/selenium/webdriver/edge/service_spec.rb b/rb/spec/unit/selenium/webdriver/edge/service_spec.rb index 9f45e5c58c5ff..8f398ca999693 100644 --- a/rb/spec/unit/selenium/webdriver/edge/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/edge/service_spec.rb @@ -84,6 +84,7 @@ module Edge context 'when initializing driver' do let(:driver) { Edge::Driver } + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } let(:service) do instance_double(described_class, launch: service_manager, executable_path: nil, 'executable_path=': nil, class: described_class) @@ -107,9 +108,7 @@ module Edge end it 'is created when :url is not provided' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new).and_return(service) driver.new @@ -117,9 +116,7 @@ module Edge end it 'accepts :service without creating a new instance' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new) driver.new(service: service) diff --git a/rb/spec/unit/selenium/webdriver/firefox/driver_spec.rb b/rb/spec/unit/selenium/webdriver/firefox/driver_spec.rb index e04a0de6ab72f..1d9a56b6e7f14 100644 --- a/rb/spec/unit/selenium/webdriver/firefox/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/firefox/driver_spec.rb @@ -33,6 +33,7 @@ module Firefox body: {value: {sessionId: 0, capabilities: {browserName: 'firefox'}}}.to_json, headers: {content_type: 'application/json'}} end + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } def expect_request(body: nil, endpoint: nil) body = (body || {capabilities: {alwaysMatch: {acceptInsecureCerts: true, @@ -48,36 +49,32 @@ def expect_request(body: nil, endpoint: nil) end it 'uses DriverFinder when provided Service without path' do + allow(DriverFinder).to receive(:new).and_return(finder) expect_request - allow(DriverFinder).to receive(:path) options = Options.new described_class.new(service: service, options: options) - expect(DriverFinder).to have_received(:path).with(options, service.class) + expect(finder).to have_received(:driver_path) end it 'does not use DriverFinder when provided Service with path' do expect_request + allow(DriverFinder).to receive(:new).and_return(finder) allow(service).to receive(:executable_path).and_return('path') - allow(DriverFinder).to receive(:path) described_class.new(service: service) - expect(DriverFinder).not_to have_received(:path) + expect(finder).not_to have_received(:driver_path) end it 'does not require any parameters' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) expect_request expect { described_class.new }.not_to raise_exception end it 'accepts provided Options as sole parameter' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) opts = {args: ['-f']} expect_request(body: {capabilities: {alwaysMatch: {acceptInsecureCerts: true, diff --git a/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb b/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb index 3fd004bf20bd5..60268fcd02eb4 100644 --- a/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb @@ -87,6 +87,7 @@ module Firefox end let(:service_manager) { instance_double(ServiceManager, uri: 'http://example.com') } let(:bridge) { instance_double(Remote::Bridge, quit: nil, create_session: {}) } + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } before do allow(Remote::Bridge).to receive(:new).and_return(bridge) @@ -100,9 +101,7 @@ module Firefox end it 'is created when :url is not provided' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new).and_return(service) driver.new @@ -111,9 +110,7 @@ module Firefox end it 'accepts :service without creating a new instance' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new) driver.new(service: service) diff --git a/rb/spec/unit/selenium/webdriver/ie/driver_spec.rb b/rb/spec/unit/selenium/webdriver/ie/driver_spec.rb index d3698eb13bc26..f23fc5f0bedc1 100644 --- a/rb/spec/unit/selenium/webdriver/ie/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/ie/driver_spec.rb @@ -33,6 +33,7 @@ module IE body: {value: {sessionId: 0, capabilities: {browserName: 'internet explorer'}}}.to_json, headers: {content_type: 'application/json'}} end + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } def expect_request(body: nil, endpoint: nil) body = (body || {capabilities: {alwaysMatch: {browserName: 'internet explorer', @@ -47,35 +48,31 @@ def expect_request(body: nil, endpoint: nil) it 'uses DriverFinder when provided Service without path' do expect_request - allow(DriverFinder).to receive(:path) + allow(DriverFinder).to receive(:new).and_return(finder) options = Options.new described_class.new(service: service, options: options) - expect(DriverFinder).to have_received(:path).with(options, service.class) + expect(finder).to have_received(:driver_path) end it 'does not use DriverFinder when provided Service with path' do expect_request allow(service).to receive(:executable_path).and_return('path') - allow(DriverFinder).to receive(:path) + allow(DriverFinder).to receive(:new).and_return(finder) described_class.new(service: service) - expect(DriverFinder).not_to have_received(:path) + expect(finder).not_to have_received(:driver_path) end it 'does not require any parameters' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) expect_request expect { described_class.new }.not_to raise_exception end it 'accepts provided Options as sole parameter' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) opts = {args: ['-f']} expect_request(body: {capabilities: {alwaysMatch: {browserName: 'internet explorer', 'se:ieOptions': {nativeEvents: true, diff --git a/rb/spec/unit/selenium/webdriver/ie/service_spec.rb b/rb/spec/unit/selenium/webdriver/ie/service_spec.rb index f27dbdbbaeb01..34f102ca94f75 100644 --- a/rb/spec/unit/selenium/webdriver/ie/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/ie/service_spec.rb @@ -88,6 +88,7 @@ module IE end let(:service_manager) { instance_double(ServiceManager, uri: 'http://example.com') } let(:bridge) { instance_double(Remote::Bridge, quit: nil, create_session: {}) } + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } before do allow(Remote::Bridge).to receive(:new).and_return(bridge) @@ -102,9 +103,7 @@ module IE end it 'is created when :url is not provided' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new).and_return(service) driver.new @@ -113,9 +112,7 @@ module IE end it 'accepts :service without creating a new instance' do - allow(DriverFinder).to receive(:path).and_return('path') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new) driver.new(service: service) diff --git a/rb/spec/unit/selenium/webdriver/safari/driver_spec.rb b/rb/spec/unit/selenium/webdriver/safari/driver_spec.rb index f88f8cacdf483..725c90d5e9b67 100644 --- a/rb/spec/unit/selenium/webdriver/safari/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/safari/driver_spec.rb @@ -33,6 +33,7 @@ module Safari body: {value: {sessionId: 0, capabilities: {browserName: 'safari'}}}.to_json, headers: {content_type: 'application/json'}} end + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } def expect_request(body: nil, endpoint: nil) body = (body || {capabilities: {alwaysMatch: {browserName: 'safari'}}}).to_json @@ -45,36 +46,32 @@ def expect_request(body: nil, endpoint: nil) end it 'uses DriverFinder when provided Service without path' do + allow(DriverFinder).to receive(:new).and_return(finder) expect_request - allow(DriverFinder).to receive(:path) options = Options.new described_class.new(service: service, options: options) - expect(DriverFinder).to have_received(:path).with(options, service.class) + expect(finder).to have_received(:driver_path) end it 'does not use DriverFinder when provided Service with path' do expect_request allow(service).to receive(:executable_path).and_return('path') - allow(DriverFinder).to receive(:path) + allow(DriverFinder).to receive(:new).and_return(finder) described_class.new(service: service) - expect(DriverFinder).not_to have_received(:path) + expect(finder).not_to have_received(:driver_path) end it 'does not require any parameters' do - allow(DriverFinder).to receive(:path).and_return('/path/to/safaridriver') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) expect_request expect { described_class.new }.not_to raise_exception end it 'accepts provided Options as sole parameter' do - allow(DriverFinder).to receive(:path).and_return('/path/to/safaridriver') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) opts = {automatic_inspection: true} expect_request(body: {capabilities: {alwaysMatch: {browserName: 'safari', 'safari:automaticInspection': true}}}) diff --git a/rb/spec/unit/selenium/webdriver/safari/service_spec.rb b/rb/spec/unit/selenium/webdriver/safari/service_spec.rb index df79bffdac671..9ae84dba3be74 100644 --- a/rb/spec/unit/selenium/webdriver/safari/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/safari/service_spec.rb @@ -83,6 +83,7 @@ module Safari end let(:service_manager) { instance_double(ServiceManager, uri: 'http://example.com') } let(:bridge) { instance_double(Remote::Bridge, quit: nil, create_session: {}) } + let(:finder) { instance_double(DriverFinder, browser_path?: false, driver_path: '/path/to/driver') } before do allow(Remote::Bridge).to receive(:new).and_return(bridge) @@ -97,9 +98,7 @@ module Safari end it 'is created when :url is not provided' do - allow(DriverFinder).to receive(:path).and_return('/path/to/safaridriver') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new).and_return(service) driver.new @@ -108,9 +107,7 @@ module Safari end it 'accepts :service without creating a new instance' do - allow(DriverFinder).to receive(:path).and_return('/path/to/safaridriver') - allow(Platform).to receive(:assert_file) - allow(Platform).to receive(:assert_executable) + allow(DriverFinder).to receive(:new).and_return(finder) allow(described_class).to receive(:new) driver.new(service: service)