[rb] allow passing devices into Action Builder constructor in more extensible way

Deprecates ordered parameters which will only be seen when not initializing via Driver class
Creates default keyboard and mouse input object instances in constructor instead of in Bridge

Author: titusfortner

rb/lib/selenium/webdriver/common/action_builder.rb

@@ -31,19 +31,42 @@ class ActionBuilder
       # the mouse is moving. Keep in mind that pauses must be added for other devices in order to line up the actions
       # correctly when using asynchronous.
       #
-      # @param [Selenium::WebDriver::Remote::Bridge] bridge the bridge for the current driver instance
-      # @param [Selenium::WebDriver::Interactions::PointerInput] mouse PointerInput for the mouse.
-      # @param [Selenium::WebDriver::Interactions::KeyInput] keyboard KeyInput for the keyboard.
-      # @param [Boolean] async Whether to perform the actions asynchronously per device. Defaults to false for
-      #   backwards compatibility.
+      # @param [Selenium::WebDriver::Remote::Bridge] bridge the bridge for the current driver instance.
+      # @param [Selenium::WebDriver::Interactions::PointerInput] deprecated_mouse PointerInput for the mouse.
+      # @param [Selenium::WebDriver::Interactions::KeyInput] deprecated_keyboard KeyInput for the keyboard.
+      # @param [Boolean] deprecated_async Whether to perform the actions asynchronously per device.
+      #   Defaults to false for backwards compatibility.
+      # @param [Array<Selenium::WebDriver::Interactions::InputDevices>] devices list of valid sources of input.
+      # @param [Boolean] async Whether to perform the actions asynchronously per device.
       # @return [ActionBuilder] A self reference.
       #
 
-      def initialize(bridge, mouse, keyboard, async = false)
-        # For backwards compatibility, automatically include mouse & keyboard
+      def initialize(bridge, deprecated_mouse = nil, deprecated_keyboard = nil, deprecated_async = nil,
+                     devices: [], async: false)
         @bridge = bridge
-        @devices = [mouse, keyboard]
-        @async = async
+
+        @async = if deprecated_async.nil?
+                   async
+                 else
+                   WebDriver.logger.deprecate('initializing ActionBuilder with async parameter',
+                                              ':async keyword',
+                                              id: :action_async)
+                   deprecated_async
+                 end
+
+        @devices = []
+        if deprecated_keyboard || deprecated_mouse
+          WebDriver.logger.deprecate "initializing ActionBuilder with keyboard and mouse parameters",
+                                     "devices keyword or, even better, Driver#action",
+                                     id: :action_devices
+          add_input(deprecated_mouse)
+          add_input(deprecated_keyboard)
+        elsif devices.empty?
+          add_pointer_input(:mouse, 'mouse')
+          add_key_input('keyboard')
+        else
+          devices.each { |device| add_input(device) }
+        end
       end
 
       #
@@ -202,12 +225,16 @@ def tick(*action_devices)
       #
 
       def add_input(device)
+        raise TypeError, "#{device.inspect} is not a valid InputDevice" unless device.is_a?(Interactions::InputDevice)
+
         unless @async
           max_device = @devices.max { |a, b| a.actions.length <=> b.actions.length }
-          pauses(device, max_device.actions.length)
+          pauses(device, max_device.actions.length) if max_device
         end
         @devices << device
       end
-    end # ActionBuilder
+    end
+
+    # ActionBuilder
   end # WebDriver
 end # Selenium

rb/lib/selenium/webdriver/remote/bridge.rb

@@ -367,11 +367,8 @@ def delete_all_cookies
         # actions
         #
 
-        def action(async = false)
-          ActionBuilder.new self,
-                            Interactions.pointer(:mouse, name: 'mouse'),
-                            Interactions.key('keyboard'),
-                            async
+        def action(deprecated_async = nil, async: false, devices: [])
+          ActionBuilder.new self, nil, nil, deprecated_async, async: async, devices: devices
         end
         alias_method :actions, :action
 

rb/spec/unit/selenium/webdriver/common/action_builder_spec.rb

@@ -25,8 +25,59 @@ module WebDriver
       let(:keyboard) { Interactions.key('key') }
       let(:mouse) { Interactions.pointer(:mouse, name: 'mouse') }
       let(:bridge) { instance_double('Bridge').as_null_object }
-      let(:builder) { ActionBuilder.new(bridge, mouse, keyboard) }
-      let(:async_builder) { ActionBuilder.new(bridge, mouse, keyboard, true) }
+      let(:builder) { ActionBuilder.new(bridge, devices: [mouse, keyboard]) }
+      let(:async_builder) { ActionBuilder.new(bridge, devices: [mouse, keyboard], async: true) }
+
+      describe '#initialize' do
+        it 'creates default mouse and keyboard when none are provided' do
+          action_builder = ActionBuilder.new(bridge)
+          expect(action_builder.devices).to include(a_kind_of(Interactions::KeyInput),
+                                                    a_kind_of(Interactions::PointerInput))
+        end
+
+        it 'deprecates using mouse and keyboard directly' do
+          expect {
+            action_builder = ActionBuilder.new(bridge, mouse, keyboard)
+            expect(action_builder.devices).to eq([mouse, keyboard])
+          }.to have_deprecated(:action_devices)
+        end
+
+        it 'deprecates using async parameter' do
+          expect {
+            action_builder = ActionBuilder.new(bridge, nil, nil, true)
+            expect(action_builder.devices).to include(a_kind_of(Interactions::PointerInput))
+            expect(action_builder.devices).to include(a_kind_of(Interactions::KeyInput))
+          }.to have_deprecated(:action_async)
+        end
+
+        it 'accepts mouse and keyboard with devices keyword' do
+          action_builder = ActionBuilder.new(bridge, devices: [mouse, keyboard])
+          expect(action_builder.devices).to eq([mouse, keyboard])
+        end
+
+        it 'accepts multiple devices with devices keyword' do
+          none = Interactions.none('none')
+          touch = Interactions.pointer(:touch, name: 'touch')
+          action_builder = ActionBuilder.new(bridge, devices: [mouse, keyboard, none, touch])
+
+          expect(action_builder.devices).to eq([mouse, keyboard, none, touch])
+        end
+
+        it 'does not accept additional devices if deprecated parameters are used' do
+          none = Interactions.none('none')
+          touch = Interactions.pointer(:touch, name: 'touch')
+          expect {
+            action_builder = ActionBuilder.new(bridge, mouse, keyboard, devices: [none, touch])
+            expect(action_builder.devices).to eq([mouse, keyboard])
+          }.to have_deprecated(:action_devices)
+        end
+
+        it 'raises a TypeError if a non InputDevice is passed into devices' do
+          expect {
+            ActionBuilder.new(bridge, devices: [mouse, keyboard, "banana"])
+          }.to raise_error(TypeError)
+        end
+      end
 
       describe '#devices' do
         it 'returns Array of devices' do
@@ -36,7 +87,7 @@ module WebDriver
       end
 
       describe '#add_pointer_input' do
-        let(:device) { instance_double Interactions::PointerInput, actions: [] }
+        let(:device) { Interactions.pointer :mouse }
 
         it 'creates pointer and adds to devices' do
           builder
@@ -73,7 +124,7 @@ module WebDriver
       end
 
       describe '#add_key_input' do
-        let(:device) { instance_double Interactions::KeyInput, actions: [] }
+        let(:device) { Interactions.key }
 
         it 'creates keyboard and adds to devices' do
           builder

rb/spec/unit/selenium/webdriver/common/interactions/interaction_spec.rb

@@ -53,11 +53,6 @@ def encode
       end
 
       class SubActionBuilder < ActionBuilder
-        def initialize(bridge, mouse, keyboard, async = nil)
-          super
-          @devices << NewDevice.new('new')
-        end
-
         def special_action(special, device: nil)
           special_input(device).create_special(special)
           self
@@ -78,7 +73,7 @@ def special_input(device = nil)
         it 'can create subclass' do
           bridge = instance_double(Remote::Bridge)
           allow(bridge).to receive(:send_actions)
-          sub_action_builder = SubActionBuilder.new(bridge, Interactions.pointer(:mouse), Interactions.key('key'))
+          sub_action_builder = SubActionBuilder.new(bridge, devices: [NewDevice.new('new')])
           sub_action_builder.special_action('special').perform
 
           expect(bridge).to have_received(:send_actions).with([{type: :special,

rb/spec/unit/selenium/webdriver/common/interactions/key_actions_spec.rb

@@ -25,7 +25,7 @@ module WebDriver
       let(:keyboard) { Interactions::KeyInput.new('keyboard') }
       let(:mouse) { Interactions::PointerInput.new(:mouse, name: 'pointer') }
       let(:bridge) { instance_double('Bridge').as_null_object }
-      let(:builder) { Selenium::WebDriver::ActionBuilder.new(bridge, mouse, keyboard) }
+      let(:builder) { Selenium::WebDriver::ActionBuilder.new(bridge, devices: [mouse, keyboard]) }
       let(:element) { Selenium::WebDriver::Element.new(bridge, 'element') }
       let(:key) { 'a' }
       let(:keys) { 'abc' }

rb/spec/unit/selenium/webdriver/common/interactions/pointer_actions_spec.rb

@@ -25,7 +25,7 @@ module WebDriver
       let(:keyboard) { Interactions::KeyInput.new('keyboard') }
       let(:mouse) { Interactions::PointerInput.new(:mouse, name: 'pointer') }
       let(:bridge) { instance_double('Bridge').as_null_object }
-      let(:builder) { ActionBuilder.new(bridge, mouse, keyboard) }
+      let(:builder) { ActionBuilder.new(bridge, devices: [mouse, keyboard]) }
       let(:element) { Element.new(bridge, 'element') }
       let(:element2) { Element.new(bridge, 'element2') }
       let(:duration) { builder.default_move_duration }