[rb] update SOC for driver finder and selenium manager classes

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

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

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 }
 

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

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

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

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

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

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,44 +46,46 @@ 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}}})
 
  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)

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,19 +105,15 @@ 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
  expect(described_class).to have_received(:new).with(no_args)
  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)