Actions: correctly fill sequences

Turns out we had shonky logic around figuring out
which input sources needed to be padded with extra
pauses.

Thanks to @codingphil for a fantastic bug report
with a test case to help clean this up. Corrected
the expectations, switched it to using the Mockito
infrastructure we already used, and grouped actions
by tick for clarity.

Closes #4538

java/client/src/org/openqa/selenium/interactions/Actions.java

@@ -23,6 +23,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
 
 import org.openqa.selenium.Keys;
 import org.openqa.selenium.UnsupportedCommandException;
@@ -62,7 +63,6 @@ public class Actions {
   private final Keyboard jsonKeyboard;
   private final Mouse jsonMouse;
   protected CompositeAction action = new CompositeAction();
-  private RuntimeException actionsException;
 
   public Actions(WebDriver driver) {
     this.driver = Preconditions.checkNotNull(driver);
@@ -547,19 +547,14 @@ public Actions tick(Interaction... actions) {
     for (Interaction action : actions) {
       Sequence sequence = getSequence(action.getSource());
       sequence.addAction(action);
-      seenSources.remove(action.getSource());
     }
 
     // And now pad the remaining sequences with a pause.
-    for (InputSource source : seenSources) {
+    Set<InputSource> unseen = Sets.difference(sequences.keySet(), seenSources);
+    for (InputSource source : unseen) {
       getSequence(source).addAction(new Pause(source, Duration.ZERO));
     }
 
-    if (isBuildingActions()) {
-      actionsException = new IllegalArgumentException(
-          "You may not use new style interactions with old style actions");
-    }
-
     return this;
   }
 

java/client/test/org/openqa/selenium/interactions/ActionsTest.java

@@ -17,6 +17,7 @@
 
 package org.openqa.selenium.interactions;
 
+import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -27,15 +28,22 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.internal.Coordinates;
 import org.openqa.selenium.internal.Locatable;
 
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * Tests the builder for advanced user interaction, the Actions class.
  */
@@ -146,6 +154,82 @@ public void creatingAllMouseActions() {
     order.verifyNoMoreInteractions();
   }
 
+  @SuppressWarnings("unchecked")
+  @Test
+  public void testCtrlClick() {
+    WebDriver driver = mock(WebDriver.class, withSettings().extraInterfaces(Interactive.class));
+    ArgumentCaptor<Collection<Sequence>> sequenceCaptor =
+        ArgumentCaptor.forClass((Class) Collection.class);
+    Mockito.doNothing().when((Interactive) driver).perform(sequenceCaptor.capture());
+
+    new Actions(driver)
+        .keyDown(Keys.CONTROL)
+        .click()
+        .keyUp(Keys.CONTROL)
+        .perform();
+
+    Collection<Sequence> sequence = sequenceCaptor.getValue();
+
+    assertEquals(2, sequence.size());
+
+    // get mouse and keyboard sequences
+    Map[] sequencesJson = sequence.stream().map(Sequence::toJson).toArray(HashMap[]::new);
+    Map mouseSequence = sequencesJson[0];
+    Map keyboardSequence;
+    if (!mouseSequence.get("type").equals("pointer")) {
+      mouseSequence = sequencesJson[1];
+      keyboardSequence = sequencesJson[0];
+    }
+    else {
+      keyboardSequence = sequencesJson[1];
+    }
+
+    assertEquals("pointer", mouseSequence.get("type"));
+    List<Map<?,?>> mouseActions = (List<Map<?,?>>) mouseSequence.get("actions");
+    assertEquals(4, mouseActions.size());
+
+    assertEquals("key", keyboardSequence.get("type"));
+    List<Map<?, ?>> keyboardActions = (List<Map<?, ?>>) keyboardSequence.get("actions");
+    assertEquals(4, keyboardActions.size());
+
+    // Mouse pauses as key goes down
+    Map<?, ?> pauseAction = mouseActions.get(0);
+    assertEquals("pause", pauseAction.get("type"));
+    assertEquals(0L, pauseAction.get("duration"));
+
+    Map<?, ?> keyDownAction = keyboardActions.get(0);
+    assertEquals("keyDown", keyDownAction.get("type"));
+    assertEquals(Keys.CONTROL.toString(), keyDownAction.get("value"));
+
+    // Mouse goes down, so keyboard pauses
+    Map<?, ?> pointerDownAction = mouseActions.get(1);
+    assertEquals("pointerDown", pointerDownAction.get("type"));
+    assertEquals(0, pointerDownAction.get("button"));
+
+    pauseAction = keyboardActions.get(1);
+    assertEquals("pause", pauseAction.get("type"));
+    assertEquals(0L, pauseAction.get("duration"));
+
+    // Mouse goes up, so keyboard pauses
+    Map<?, ?> pointerUpAction = mouseActions.get(2);
+    assertEquals("pointerUp", pointerUpAction.get("type"));
+    assertEquals(0, pointerUpAction.get("button"));
+
+    pauseAction = keyboardActions.get(2);
+    assertEquals("pause", pauseAction.get("type"));
+    assertEquals(0L, pauseAction.get("duration"));
+
+    // Mouse pauses as keyboard releases key
+    pauseAction = mouseActions.get(3);
+    assertEquals("pause", pauseAction.get("type"));
+    assertEquals(0L, pauseAction.get("duration"));
+
+    Map<?, ?> keyUpAction = keyboardActions.get(3);
+    assertEquals("keyUp", keyUpAction.get("type"));
+    assertEquals(Keys.CONTROL.toString(), keyUpAction.get("value"));
+  }
+
+
   private WebElement mockLocatableElementWithCoordinates(Coordinates coord) {
     WebElement element = mock(WebElement.class,
                               withSettings().extraInterfaces(Locatable.class));