[rb] move driver related functionality to Service classes

Author: titusfortner

rb/lib/selenium/webdriver/chrome/bridge.rb

@@ -26,15 +26,25 @@ def initialize(opts = {})
           opts[:desired_capabilities] = create_capabilities(opts)
 
           unless opts.key?(:url)
+            driver_path = opts.delete(:driver_path) || Chrome.driver_path
             port = opts.delete(:port) || Service::DEFAULT_PORT
-            service_args = opts.delete(:service_args) || {}
 
-            if opts[:service_log_path]
-              service_args.merge!(service_log_path: opts.delete(:service_log_path))
+            opts[:driver_opts] ||= {}
+            if opts.key? :service_log_path
+              WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `:service_log_path` is deprecated. Use `driver_opts: {log_path: #{opts[:service_log_path]}}`
+              DEPRECATE
+              opts[:driver_opts][:log_path] = opts.delete :service_log_path
             end
 
-            driver_path = opts.delete(:driver_path) || Chrome.driver_path
-            @service = Service.new(driver_path, port, *extract_service_args(service_args))
+            if opts.key? :service_args
+              WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `:service_args` is deprecated. Pass switches using `driver_opts`
+              DEPRECATE
+              opts[:driver_opts][:args] = opts.delete(:service_args)
+            end
+
+            @service = Service.new(driver_path, port, opts.delete(:driver_opts))
             @service.start
             opts[:url] = @service.uri
           end
@@ -47,10 +57,8 @@ def browser
         end
 
         def driver_extensions
-          [
-            DriverExtensions::TakesScreenshot,
-            DriverExtensions::HasWebStorage
-          ]
+          [DriverExtensions::TakesScreenshot,
+           DriverExtensions::HasWebStorage]
         end
 
         def capabilities
@@ -76,34 +84,24 @@ def create_capabilities(opts)
             raise ArgumentError, ':args must be an Array of Strings'
           end
 
-          chrome_options['args'] = args.map(&:to_s)
+          args.map!(&:to_s)
           profile = opts.delete(:profile).as_json if opts.key?(:profile)
 
-          if profile && chrome_options['args'].none? { |arg| arg =~ /user-data-dir/}
-            chrome_options['args'] << "--user-data-dir=#{profile[:directory]}"
+          if profile && args.none? { |arg| arg =~ /user-data-dir/ }
+            args << "--user-data-dir=#{profile[:directory]}"
           end
+          chrome_options['args'] = args unless args.empty?
 
           chrome_options['extensions'] = profile[:extensions] if profile && profile[:extensions]
           chrome_options['detach'] = true if opts.delete(:detach)
           chrome_options['prefs'] = opts.delete(:prefs) if opts.key?(:prefs)
 
-          caps[:chrome_options] = chrome_options
+          caps[:chrome_options] = chrome_options unless chrome_options.empty?
           caps[:proxy] = opts.delete(:proxy) if opts.key?(:proxy)
           caps[:proxy] ||= opts.delete('proxy') if opts.key?('proxy')
 
           caps
         end
-
-        def extract_service_args(args)
-          service_args = []
-          service_args << "--log-path=#{args.delete(:service_log_path)}" if args.key?(:service_log_path)
-          service_args << "--url-base=#{args.delete(:url_base)}" if args.key?(:url_base)
-          service_args << "--port-server=#{args.delete(:port_server)}" if args.key?(:port_server)
-          service_args << "--whitelisted-ips=#{args.delete(:whitelisted_ips)}" if args.key?(:whitelisted_ips)
-          service_args << "--verbose=#{args.delete(:verbose)}" if args.key?(:verbose)
-          service_args << "--silent=#{args.delete(:silent)}" if args.key?(:silent)
-          service_args
-        end
       end # Bridge
     end # Chrome
   end # WebDriver

rb/lib/selenium/webdriver/chrome/service.rb

@@ -48,6 +48,17 @@ def start_process
         def cannot_connect_error_text
           "unable to connect to chromedriver #{@host}:#{@port}"
         end
+
+        def extract_service_args(driver_opts)
+          driver_args = super
+          driver_args << "--log-path=#{driver_opts.delete(:log_path)}" if driver_opts.key?(:log_path)
+          driver_args << "--url-base=#{driver_opts.delete(:url_base)}" if driver_opts.key?(:url_base)
+          driver_args << "--port-server=#{driver_opts.delete(:port_server)}" if driver_opts.key?(:port_server)
+          driver_args << "--whitelisted-ips=#{driver_opts.delete(:whitelisted_ips)}" if driver_opts.key?(:whitelisted_ips)
+          driver_args << "--verbose" if driver_opts.key?(:verbose)
+          driver_args << "--silent" if driver_opts.key?(:silent)
+          driver_args
+        end
       end # Service
     end # Chrome
   end # WebDriver

rb/lib/selenium/webdriver/common/service.rb

@@ -45,11 +45,11 @@ class << self
 
       attr_accessor :host
 
-      def initialize(executable_path, port, *extra_args)
+      def initialize(executable_path, port, driver_opts)
         @executable_path = binary_path(executable_path)
         @host            = Platform.localhost
         @port            = Integer(port)
-        @extra_args      = extra_args
+        @extra_args      = extract_service_args(driver_opts)
 
         raise Error::WebDriverError, "invalid port: #{@port}" if @port < 1
       end
@@ -137,6 +137,13 @@ def cannot_connect_error_text
       def socket_lock
         @socket_lock ||= SocketLock.new(@port - 1, SOCKET_LOCK_TIMEOUT)
       end
+
+      protected
+
+      def extract_service_args(driver_opts)
+        driver_opts.key?(:args) ? driver_opts.delete(:args) :  []
+      end
+
     end # Service
   end # WebDriver
 end # Selenium

rb/lib/selenium/webdriver/edge/bridge.rb

@@ -26,19 +26,26 @@ module Edge
 
       class Bridge < Remote::W3CBridge
         def initialize(opts = {})
-          port = opts.delete(:port) || Service::DEFAULT_PORT
-          service_args = opts.delete(:service_args) || {}
+          opts[:desired_capabilities] ||= Remote::W3CCapabilities.edge
 
           unless opts.key?(:url)
             driver_path = opts.delete(:driver_path) || Edge.driver_path
-            @service = Service.new(driver_path, port, *extract_service_args(service_args))
+            port = opts.delete(:port) || Service::DEFAULT_PORT
+
+            opts[:driver_opts] ||= {}
+            if opts.key? :service_args
+              WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `:service_args` is deprecated. Pass switches using `driver_opts`
+              DEPRECATE
+              opts[:driver_opts][:args] = opts.delete(:service_args)
+            end
+
+            @service = Service.new(driver_path, port, opts.delete(:driver_opts))
             @service.host = 'localhost' if @service.host == '127.0.0.1'
             @service.start
             opts[:url] = @service.uri
           end
 
-          opts[:desired_capabilities] ||= Remote::W3CCapabilities.edge
-
           super(opts)
         end
 
@@ -108,16 +115,6 @@ def reposition_window(x, y, handle = :current)
         def maximize_window(handle = :current)
           execute :maximize_window, window_handle: handle
         end
-
-        private
-
-        def extract_service_args(args = {})
-          service_args = []
-          service_args << "–host=#{args[:host]}" if args.key? :host
-          service_args << "–package=#{args[:package]}" if args.key? :package
-          service_args << "-verbose" if args[:verbose] == true
-          service_args
-        end
       end # Bridge
     end # Edge
   end # WebDriver

rb/lib/selenium/webdriver/edge/service.rb

@@ -47,6 +47,14 @@ def start_process
         def cannot_connect_error_text
           "unable to connect to MicrosoftWebDriver #{@host}:#{@port}"
         end
+
+        def extract_service_args(driver_opts)
+          driver_args = super
+          driver_args << "–host=#{driver_opts[:host]}" if driver_opts.key? :host
+          driver_args << "–package=#{driver_opts[:package]}" if driver_opts.key? :package
+          driver_args << "-verbose" if driver_opts[:verbose] == true
+          driver_args
+        end
       end # Service
     end # Edge
   end # WebDriver

rb/lib/selenium/webdriver/firefox/bridge.rb

@@ -24,14 +24,20 @@ module Firefox
       class Bridge < Remote::Bridge
         def initialize(opts = {})
           opts[:desired_capabilities] ||= Remote::Capabilities.firefox
-          opts[:desired_capabilities].proxy = opts.delete(:proxy) if opts.key?(:proxy)
+
+          if opts.key? :proxy
+            WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `:proxy` is deprecated. Pass in as capability: `Remote::Capabilities.firefox(proxy: #{opts[:proxy]})`
+            DEPRECATE
+            opts[:desired_capabilities].proxy = opts.delete(:proxy)
+          end
 
           unless opts.key?(:url)
             port = opts.delete(:port) || DEFAULT_PORT
             profile = opts.delete(:profile)
 
             Binary.path = opts[:desired_capabilities][:firefox_binary] if opts[:desired_capabilities][:firefox_binary]
-            @launcher = create_launcher(port, profile)
+            @launcher = Launcher.new Binary.new, port, profile
             @launcher.launch
             opts[:url] = @launcher.url
           end
@@ -58,12 +64,6 @@ def quit
         ensure
           @launcher.quit
         end
-
-        private
-
-        def create_launcher(port, profile)
-          Launcher.new Binary.new, port, profile
-        end
       end # Bridge
     end # Firefox
   end # WebDriver

rb/lib/selenium/webdriver/firefox/service.rb

@@ -51,6 +51,15 @@ def start_process
         def cannot_connect_error_text
           "unable to connect to Mozilla geckodriver #{@host}:#{@port}"
         end
+
+        def extract_service_args(driver_opts)
+          driver_args = super
+          driver_args << "--binary=#{driver_opts[:binary]}" if driver_opts.key?(:binary)
+          driver_args << "–-log=#{driver_opts[:log]}" if driver_opts.key?(:log)
+          driver_args << "–-marionette-port=#{driver_opts[:marionette_port]}" if driver_opts.key?(:marionette_port)
+          driver_args << "–-host=#{driver_opts[:host]}" if driver_opts.key?(:host)
+          driver_args
+        end
       end # Service
     end # Firefox
   end # WebDriver

rb/lib/selenium/webdriver/firefox/w3c_bridge.rb

@@ -26,11 +26,18 @@ def initialize(opts = {})
           opts[:desired_capabilities] = create_capabilities(opts)
 
           unless opts.key?(:url)
+            driver_path = opts.delete(:driver_path) || Firefox.driver_path
             port = opts.delete(:port) || Service::DEFAULT_PORT
-            service_args = opts.delete(:service_args) || {}
 
-            driver_path = opts.delete(:driver_path) || Firefox.driver_path
-            @service = Service.new(driver_path, port, *extract_service_args(service_args))
+            opts[:driver_opts] ||= {}
+            if opts.key? :service_args
+              WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `:service_args` is deprecated. Pass switches using `driver_opts`
+              DEPRECATE
+              opts[:driver_opts][:args] = opts.delete(:service_args)
+            end
+
+            @service = Service.new(driver_path, port, opts.delete(:driver_opts))
             @service.start
             opts[:url] = @service.uri
           end
@@ -44,7 +51,7 @@ def browser
 
         def driver_extensions
           [DriverExtensions::TakesScreenshot,
-            DriverExtensions::HasWebStorage]
+           DriverExtensions::HasWebStorage]
         end
 
         def quit
@@ -71,17 +78,6 @@ def create_capabilities(opts)
           Binary.path = caps[:firefox_options][:binary] if caps[:firefox_options].key?(:binary)
           caps
         end
-
-        def extract_service_args(args = {})
-          service_args = []
-          service_args << "--binary=#{args[:binary]}" if args.key?(:binary)
-          service_args << "–-log=#{args[:log]}" if args.key?(:log)
-          service_args << "–-marionette-port=#{args[:marionette_port]}" if args.key?(:marionette_port)
-          service_args << "–-host=#{args[:host]}" if args.key?(:host)
-          service_args << "–-port=#{args[:port]}" if args.key?(:port)
-          service_args
-        end
-
       end # W3CBridge
     end # Firefox
   end # WebDriver

rb/lib/selenium/webdriver/ie/bridge.rb

@@ -29,11 +29,26 @@ def initialize(opts = {})
           opts[:desired_capabilities] ||= Remote::Capabilities.internet_explorer
 
           unless opts.key?(:url)
-            port = opts.delete(:port) || Service::DEFAULT_PORT
-            service_args = opts.delete(:service_args) || {}
-            service_args = match_legacy(opts, service_args)
             driver_path = opts.delete(:driver_path) || IE.driver_path
-            @service = Service.new(driver_path, port, *extract_service_args(service_args))
+            port = opts.delete(:port) || Service::DEFAULT_PORT
+
+            opts[:driver_opts] ||= {}
+            if opts.key? :service_args
+              WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `:service_args` is deprecated. Pass switches using `driver_opts`
+              DEPRECATE
+              opts[:driver_opts][:args] = opts.delete(:service_args)
+            end
+
+            %i[log_level log_file implementation].each do |method|
+              next unless opts.key? method
+              WebDriver.logger.warn <<-DEPRECATE.gsub(/\n +| {2,}/, ' ').freeze
+            [DEPRECATION] `#{method}` is deprecated. Pass switches using `driver_opts`
+              DEPRECATE
+              opts[:driver_opts][method] = opts.delete(method)
+            end
+
+            @service = Service.new(driver_path, port, opts.delete(:driver_opts))
             @service.start
             opts[:url] = @service.uri
           end
@@ -59,26 +74,6 @@ def quit
         ensure
           @service.stop if @service
         end
-
-        private
-
-        def match_legacy(opts, args)
-          args[:log_level] = opts.delete(:log_level) if opts.key?(:log_level)
-          args[:log_file] = opts.delete(:log_file) if opts.key?(:log_file)
-          args[:implementation] = opts.delete(:implementation) if opts.key?(:implementation)
-          args
-        end
-
-        def extract_service_args(args)
-          service_args = []
-          service_args << "--log-level=#{args.delete(:log_level).to_s.upcase}" if args.key?(:log_level)
-          service_args << "--log-file=#{args.delete(:log_file)}" if args.key?(:log_file)
-          service_args << "--implementation=#{args.delete(:implementation).to_s.upcase}" if args.key?(:implementation)
-          service_args << "--host=#{args.delete(:host)}" if args.key?(:host)
-          service_args << "--extract_path=#{args.delete(:extract_path)}" if args.key?(:extract_path)
-          service_args << "--silent" if args[:silent] == true
-          service_args
-        end
       end # Bridge
     end # IE
   end # WebDriver

rb/lib/selenium/webdriver/ie/service.rb

@@ -47,6 +47,17 @@ def start_process
         def cannot_connect_error_text
           "unable to connect to IE server #{@host}:#{@port}"
         end
+
+        def extract_service_args(driver_opts)
+          driver_args = super
+          driver_args << "--log-level=#{driver_opts.delete(:log_level).to_s.upcase}" if driver_opts.key?(:log_level)
+          driver_args << "--log-file=#{driver_opts.delete(:log_file)}" if driver_opts.key?(:log_file)
+          driver_args << "--implementation=#{driver_opts.delete(:implementation).to_s.upcase}" if driver_opts.key?(:implementation)
+          driver_args << "--host=#{driver_opts.delete(:host)}" if driver_opts.key?(:host)
+          driver_args << "--extract_path=#{driver_opts.delete(:extract_path)}" if driver_opts.key?(:extract_path)
+          driver_args << "--silent" if driver_opts[:silent] == true
+          driver_args
+        end
       end # Server
     end # IE
   end # WebDriver