Refactoring constructor that accepts both desired capabilities and pr…

…ofile.
SeleniumHQ · Mar 2, 2017 · 59f8fef · 59f8fef
1 parent 5ff67fc
commit 59f8fef
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 16 deletions.
diff --git a/java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java b/java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java
@@ -185,6 +185,11 @@ public FirefoxDriver(Capabilities desiredCapabilities, Capabilities requiredCapa
     this(getBinary(desiredCapabilities), null, desiredCapabilities, requiredCapabilities);
   }
 
+  public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile, Capabilities capabilities) {
+    this(getFirefoxOptions(capabilities).setBinary(binary).setProfile(profile)
+             .addDesiredCapabilities(capabilities));
+  }
+
   private static FirefoxProfile prepareProfile(FirefoxProfile profile,
                                                Capabilities desiredCapabilities,
                                                Capabilities requiredCapabilities) {
@@ -292,10 +297,6 @@ private static FirefoxBinary getBinary(Capabilities capabilities) {
     return new FirefoxBinary();
   }
 
-  public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile, Capabilities capabilities) {
-    this(binary, profile, capabilities, null);
-  }
-
   public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile,
       Capabilities desiredCapabilities, Capabilities requiredCapabilities) {
     this(createCommandExecutor(desiredCapabilities, requiredCapabilities, binary, profile),

diff --git a/java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java b/java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java
@@ -172,14 +172,14 @@ public FirefoxOptions setProfile(FirefoxProfile profile) {
   }
 
   public FirefoxProfile getProfile() {
-    if (profile !=  null) {
-      return profile;
-    }
+    final FirefoxProfile toReturn = Optional.ofNullable(profile).orElse(
+        Optional.ofNullable(extractProfile()).orElse(new FirefoxProfile()));
+
+    booleanPrefs.entrySet().forEach(pref -> toReturn.setPreference(pref.getKey(), pref.getValue()));
+    intPrefs.entrySet().forEach(pref -> toReturn.setPreference(pref.getKey(), pref.getValue()));
+    stringPrefs.entrySet().forEach(pref -> toReturn.setPreference(pref.getKey(), pref.getValue()));
 
-//    if (requiredCapabilities != null && requiredCapabilities.getCapability(PROFILE) != null) {
-//      raw = requiredCapabilities.getCapability(PROFILE);
-//    }
-    return extractProfile(desiredCapabilities);
+    return toReturn;
   }
 
   // Confusing API. Keeping package visible only
@@ -223,7 +223,7 @@ public FirefoxOptions setLogLevel(Level logLevel) {
   public FirefoxOptions addDesiredCapabilities(Capabilities desiredCapabilities) {
     this.desiredCapabilities.merge(desiredCapabilities);
 
-    FirefoxProfile suggestedProfile = extractProfile(desiredCapabilities);
+    FirefoxProfile suggestedProfile = extractProfile();
     if (suggestedProfile !=  null) {
       if (!booleanPrefs.isEmpty() || !intPrefs.isEmpty() || !stringPrefs.isEmpty()) {
         throw new IllegalStateException(
@@ -241,7 +241,7 @@ public FirefoxOptions addDesiredCapabilities(Capabilities desiredCapabilities) {
     return this;
   }
 
-  private FirefoxProfile extractProfile(Capabilities desiredCapabilities) {
+  private FirefoxProfile extractProfile() {
     if (desiredCapabilities == null) {
       return null;
     }

diff --git a/java/client/test/org/openqa/selenium/firefox/FirefoxDriverTest.java b/java/client/test/org/openqa/selenium/firefox/FirefoxDriverTest.java
@@ -43,7 +43,6 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.Dimension;
 import org.openqa.selenium.JavascriptExecutor;
-import org.openqa.selenium.UnexpectedAlertBehaviour;
 import org.openqa.selenium.testing.NeedsFreshDriver;
 import org.openqa.selenium.ParallelTestRunner;
 import org.openqa.selenium.ParallelTestRunner.Worker;
@@ -92,7 +91,9 @@ public void canStartDriverWithNoParameters() {
   @Test
   public void canStartDriverWithSpecifiedBinary() throws IOException {
     FirefoxBinary binary = spy(new FirefoxBinary());
+
     localDriver = new FirefoxDriver(binary);
+
     verifyItIsLegacy(localDriver);
     verify(binary).startFirefoxProcess(any());
   }
@@ -102,19 +103,24 @@ public void canStartDriverWithSpecifiedProfile() {
     FirefoxProfile profile = new FirefoxProfile();
     profile.setPreference("browser.startup.page", 1);
     profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
     localDriver = new FirefoxDriver(profile);
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
     verifyItIsLegacy(localDriver);
   }
 
   @Test
   public void canStartDriverWithSpecifiedBinaryAndProfile() throws IOException {
     FirefoxBinary binary = spy(new FirefoxBinary());
+
     FirefoxProfile profile = new FirefoxProfile();
     profile.setPreference("browser.startup.page", 1);
     profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
     localDriver = new FirefoxDriver(binary, profile);
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
     verifyItIsLegacy(localDriver);
     verify(binary).startFirefoxProcess(any());
   }
@@ -123,8 +129,73 @@ public void canStartDriverWithSpecifiedBinaryAndProfile() throws IOException {
   public void canPassCapabilities() {
     DesiredCapabilities capabilities = new DesiredCapabilities();
     capabilities.setCapability(CapabilityType.PAGE_LOAD_STRATEGY, "none");
+
     localDriver = new FirefoxDriver(capabilities);
+
+    verifyItIsLegacy(localDriver);
+    assertEquals(
+        localDriver.getCapabilities().getCapability(CapabilityType.PAGE_LOAD_STRATEGY), "none");
+  }
+
+  @Test
+  public void canSetPreferencesInFirefoxOptions() {
+    DesiredCapabilities caps = new FirefoxOptions()
+        .addPreference("browser.startup.page", 1)
+        .addPreference("browser.startup.homepage", pages.xhtmlTestPage)
+        .addTo(DesiredCapabilities.firefox());
+
+    localDriver = new FirefoxDriver(caps);
+    wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsLegacy(localDriver);
+  }
+
+  @Test
+  public void canSetProfileInFirefoxOptions() {
+    FirefoxProfile profile = new FirefoxProfile();
+    profile.setPreference("browser.startup.page", 1);
+    profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
+    DesiredCapabilities caps = new FirefoxOptions().setProfile(profile)
+        .addTo(DesiredCapabilities.firefox());
+
+    localDriver = new FirefoxDriver(caps);
+    wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsLegacy(localDriver);
+  }
+
+  @Test
+  public void canSetProfileInCapabilities() {
+    FirefoxProfile profile = new FirefoxProfile();
+    profile.setPreference("browser.startup.page", 1);
+    profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
+    DesiredCapabilities caps = new DesiredCapabilities();
+    caps.setCapability(FirefoxDriver.PROFILE, profile);
+
+    localDriver = new FirefoxDriver(caps);
+    wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsLegacy(localDriver);
+  }
+
+  @Test
+  public void canPassCapabilitiesBinaryAndProfileSeparately() throws IOException {
+    FirefoxBinary binary = spy(new FirefoxBinary());
+
+    FirefoxProfile profile = new FirefoxProfile();
+    profile.setPreference("browser.startup.page", 1);
+    profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
+    DesiredCapabilities capabilities = new DesiredCapabilities();
+    capabilities.setCapability(CapabilityType.PAGE_LOAD_STRATEGY, "none");
+
+    localDriver = new FirefoxDriver(binary, profile, capabilities);
+    wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
     verifyItIsLegacy(localDriver);
+    verify(binary).startFirefoxProcess(any());
     assertEquals(
         localDriver.getCapabilities().getCapability(CapabilityType.PAGE_LOAD_STRATEGY), "none");
   }

diff --git a/java/client/test/org/openqa/selenium/firefox/MarionetteTest.java b/java/client/test/org/openqa/selenium/firefox/MarionetteTest.java
@@ -25,6 +25,7 @@
 import static org.mockito.Mockito.verify;
 import static org.openqa.selenium.testing.Driver.FIREFOX;
 import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.fail;
 
 import org.junit.After;
 import org.junit.Test;
@@ -63,7 +64,9 @@ public void canStartDriverWithNoParameters() {
   @Test
   public void canStartDriverWithSpecifiedBinary() throws IOException {
     FirefoxBinary binary = spy(new FirefoxBinary());
+
     localDriver = new FirefoxDriver(binary);
+
     verifyItIsMarionette(localDriver);
     verify(binary, atLeastOnce()).getPath();
     verify(binary, never()).startFirefoxProcess(any());
@@ -74,19 +77,24 @@ public void canStartDriverWithSpecifiedProfile() {
     FirefoxProfile profile = new FirefoxProfile();
     profile.setPreference("browser.startup.page", 1);
     profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
     localDriver = new FirefoxDriver(profile);
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
     verifyItIsMarionette(localDriver);
   }
 
   @Test
   public void canStartDriverWithSpecifiedBinaryAndProfile() throws IOException {
     FirefoxBinary binary = spy(new FirefoxBinary());
+
     FirefoxProfile profile = new FirefoxProfile();
     profile.setPreference("browser.startup.page", 1);
     profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
     localDriver = new FirefoxDriver(binary, profile);
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
     verifyItIsMarionette(localDriver);
     verify(binary, atLeastOnce()).getPath();
     verify(binary, never()).startFirefoxProcess(any());
@@ -96,7 +104,9 @@ public void canStartDriverWithSpecifiedBinaryAndProfile() throws IOException {
   public void canPassCapabilities() {
     DesiredCapabilities capabilities = new DesiredCapabilities();
     capabilities.setCapability(CapabilityType.PAGE_LOAD_STRATEGY, "none");
+
     localDriver = new FirefoxDriver(capabilities);
+
     verifyItIsMarionette(localDriver);
     assertEquals(
         localDriver.getCapabilities().getCapability(CapabilityType.PAGE_LOAD_STRATEGY), "none");
@@ -110,8 +120,9 @@ public void canSetPreferencesInFirefoxOptions() {
       .addTo(DesiredCapabilities.firefox());
 
     localDriver = new FirefoxDriver(caps);
-
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsMarionette(localDriver);
   }
 
   @Test
@@ -124,8 +135,9 @@ public void canSetProfileInFirefoxOptions() {
         .addTo(DesiredCapabilities.firefox());
 
     localDriver = new FirefoxDriver(caps);
-
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsMarionette(localDriver);
   }
 
   @Test
@@ -138,8 +150,58 @@ public void canSetProfileInCapabilities() {
     caps.setCapability(FirefoxDriver.PROFILE, profile);
 
     localDriver = new FirefoxDriver(caps);
+    wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsMarionette(localDriver);
+  }
+
+  @Test
+  public void canUseSameProfileInCapabilitiesAndDirectly() {
+    FirefoxProfile profile = new FirefoxProfile();
+    profile.setPreference("browser.startup.page", 1);
+    profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
+    DesiredCapabilities caps = new DesiredCapabilities();
+    caps.setCapability(FirefoxDriver.PROFILE, profile);
 
+    localDriver = new FirefoxDriver(new FirefoxBinary(), profile, caps);
     wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsMarionette(localDriver);
+  }
+
+  @Test
+  public void cannotUseDifferentProfilesInCapabilitiesAndDirectly() {
+    DesiredCapabilities caps = new DesiredCapabilities();
+    caps.setCapability(FirefoxDriver.PROFILE, new FirefoxProfile());
+
+    try {
+      localDriver = new FirefoxDriver(new FirefoxBinary(), new FirefoxProfile(), caps);
+      fail("Exception expected");
+    } catch (IllegalStateException ex) {
+      // expected
+    }
+  }
+
+  @Test
+  public void canPassCapabilitiesBinaryAndProfileSeparately() throws IOException {
+    FirefoxBinary binary = spy(new FirefoxBinary());
+
+    FirefoxProfile profile = new FirefoxProfile();
+    profile.setPreference("browser.startup.page", 1);
+    profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
+
+    DesiredCapabilities capabilities = new DesiredCapabilities();
+    capabilities.setCapability(CapabilityType.PAGE_LOAD_STRATEGY, "none");
+
+    localDriver = new FirefoxDriver(binary, profile, capabilities);
+    wait.until($ -> "XHTML Test Page".equals(localDriver.getTitle()));
+
+    verifyItIsMarionette(localDriver);
+    verify(binary, atLeastOnce()).getPath();
+    verify(binary, never()).startFirefoxProcess(any());
+    assertEquals(
+        localDriver.getCapabilities().getCapability(CapabilityType.PAGE_LOAD_STRATEGY), "none");
   }
 
   private void verifyItIsMarionette(FirefoxDriver driver) {