Propagate webdriver_firefox_port preference to FirefoxDriver, was being ignored. Fixes issue 5172

Signed-off-by: Alexei Barantsev <[email protected]>

Author: Stephen Kuenzli

java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java

@@ -266,7 +266,7 @@ private static FirefoxProfile getProfile(FirefoxProfile profile) {
 
   protected ExtensionConnection connectTo(FirefoxBinary binary, FirefoxProfile profile,
       String host) {
-    Lock lock = obtainLock();
+    Lock lock = obtainLock(profile);
     try {
       FirefoxBinary bin = binary == null ? new FirefoxBinary() : binary;
 
@@ -283,8 +283,10 @@ protected ExtensionConnection connectTo(FirefoxBinary binary, FirefoxProfile pro
     }
   }
 
-  protected Lock obtainLock() {
-    return new SocketLock();
+  protected static Lock obtainLock(FirefoxProfile profile) {
+    int preferredPort =
+        profile.getIntegerPreference(FirefoxProfile.PORT_PREFERENCE, SocketLock.DEFAULT_PORT);
+    return new SocketLock(preferredPort);
   }
 
   @Override

java/client/src/org/openqa/selenium/firefox/FirefoxProfile.java

@@ -143,6 +143,14 @@ private boolean getBooleanPreference(Preferences prefs, String key, boolean defa
     throw new WebDriverException("Expected boolean value is not a boolean. It is: " + value);
   }
 
+  public int getIntegerPreference(String key, int defaultValue) {
+    Object preference=additionalPrefs.getPreference(key);
+    if(preference!=null && preference instanceof Integer){
+      return (Integer)preference;
+    }
+    return defaultValue;
+  }
+
   private void verifyModel(File model) {
     if (model == null) {
       return;

java/client/src/org/openqa/selenium/firefox/internal/NewProfileExtensionConnection.java

@@ -83,7 +83,7 @@ public void start() throws IOException {
 
     lock.lock(connectTimeout);
     try {
-      port = determineNextFreePort(DEFAULT_PORT);
+      port = determineNextFreePort(profile.getIntegerPreference(PORT_PREFERENCE, DEFAULT_PORT));
       profile.setPreference(PORT_PREFERENCE, port);
 
       profileDir = profile.layoutOnDisk();

java/client/src/org/openqa/selenium/internal/SocketLock.java

@@ -129,4 +129,12 @@ private boolean isLockFree(InetSocketAddress address) throws IOException {
       return false;
     }
   }
+
+  /**
+   * Gets the port number that is being-locked.
+   * @return
+   */
+  public int getLockPort(){
+    return this.address.getPort();
+  }
 }

java/client/test/org/openqa/selenium/firefox/FirefoxDriverUtilitiesTest.java

@@ -0,0 +1,41 @@
+package org.openqa.selenium.firefox;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.Test;
+import org.openqa.selenium.internal.Lock;
+import org.openqa.selenium.internal.SocketLock;
+
+/**
+ * FirefoxDriverUtilitiesTest is responsible for tests of FirefoxDriver
+ * utilities that do not require a browser.
+ */
+public class FirefoxDriverUtilitiesTest {
+
+  @Test
+  public void shouldObtainSocketLockForDefaultPortWhenNotSpecifiedInProfile(){
+    Lock lock = FirefoxDriver.obtainLock(new FirefoxProfile());
+
+    assertTrue("expected lock to be a SocketLock", lock instanceof SocketLock);
+
+    assertEquals(SocketLock.DEFAULT_PORT, ((SocketLock) lock).getLockPort());
+  }
+
+  @Test
+  public void shouldObtainSocketLockForPortSpecifiedInProfile(){
+    FirefoxProfile mockProfile = mock(FirefoxProfile.class);
+    int preferredPort = 2400;
+    when(mockProfile.getIntegerPreference(FirefoxProfile.PORT_PREFERENCE, SocketLock.DEFAULT_PORT)).thenReturn(
+        preferredPort);
+
+    Lock lock = FirefoxDriver.obtainLock(mockProfile);
+
+    assertTrue("expected lock to be a SocketLock", lock instanceof SocketLock);
+
+    assertEquals(preferredPort, ((SocketLock) lock).getLockPort());
+  }
+
+}

java/client/test/org/openqa/selenium/firefox/FirefoxProfileTest.java

@@ -78,6 +78,24 @@ public void shouldSetIntegerPreferences() throws Exception {
     assertTrue("Did not see integer value being set correctly", seenCheese);
   }
 
+  @Test
+  public void getIntegerPreferenceShouldReturnUserSuppliedValueWhenSet() throws Exception {
+    String key = "cheese";
+    int value = 1234;
+    profile.setPreference(key, value);
+
+    int defaultValue = -42;
+    assertEquals(1234, profile.getIntegerPreference(key, defaultValue));
+  }
+
+  @Test
+  public void getIntegerPreferenceShouldReturnDefaultValueWhenSet() throws Exception {
+    String key = "cheese";
+
+    int defaultValue = 42;
+    assertEquals(defaultValue, profile.getIntegerPreference(key, defaultValue));
+  }
+
   @Test
   public void shouldSetBooleanPreferences() throws Exception {
     profile.setPreference("cheese", false);

java/client/test/org/openqa/selenium/firefox/FirefoxSpecificTests.java

@@ -20,6 +20,7 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Suite;
 import org.openqa.selenium.firefox.internal.ExecutableTest;
+import org.openqa.selenium.firefox.internal.NewProfileExtensionConnectionTest;
 import org.openqa.selenium.firefox.internal.SocketLockTest;
 import org.openqa.selenium.firefox.internal.StreamsTest;
 
@@ -29,8 +30,10 @@
     ExecutableTest.class,
     FirefoxCapabilitiesTest.class,
     FirefoxDriverTest.class,
+    FirefoxDriverUtilitiesTest.class,
     FirefoxProfileTest.class,
     NativeEventsTest.class,
+    NewProfileExtensionConnectionTest.class,
     PreferencesTest.class,
     SocketLockTest.class,
     StreamsTest.class

java/client/test/org/openqa/selenium/firefox/internal/NewProfileExtensionConnectionTest.java

@@ -0,0 +1,46 @@
+package org.openqa.selenium.firefox.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.openqa.selenium.WebDriverException;
+import org.openqa.selenium.firefox.FirefoxBinary;
+import org.openqa.selenium.firefox.FirefoxProfile;
+import org.openqa.selenium.internal.SocketLock;
+
+public class NewProfileExtensionConnectionTest {
+
+  @Test
+  public void canBeConstructed() throws Exception {
+    new NewProfileExtensionConnection
+        (makeLock(), new FirefoxBinary(), new FirefoxProfile(), "my-host");
+  }
+
+  @Test
+  public void shouldDefaultToPortSpecifiedInProfileWhenDeterminingNextFreePort() throws Exception {
+    int expectedPort = 2400;
+
+    FirefoxProfile profile = new FirefoxProfile();
+    profile.setPreference(FirefoxProfile.PORT_PREFERENCE, expectedPort);
+
+    NewProfileExtensionConnection connection = new NewProfileExtensionConnection
+        (makeLock(), new FirefoxBinary(), profile, "my-host");
+
+    try {
+
+      connection.start();
+
+      fail("there was an unexpected server listening on " + expectedPort + "; expected connection to fail");
+    } catch (WebDriverException e) {
+      int PORT_PREFERENCE_NOT_PROPAGATED = -1;
+      assertEquals(expectedPort,
+                   profile.getIntegerPreference(FirefoxProfile.PORT_PREFERENCE, PORT_PREFERENCE_NOT_PROPAGATED));
+    }
+
+  }
+
+  private SocketLock makeLock() {
+    return new SocketLock(4200);
+  }
+}