Making BrowserLauncherFactory non-static, to avoid global configurati…

…on, for better testability and configurability (per object, not globally)
SeleniumHQ · Oct 15, 2015 · fa001b6 · fa001b6
1 parent 76363f3
commit fa001b6
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 12 deletions.
diff --git a/java/server/src/org/openqa/selenium/server/SeleniumDriverResourceHandler.java b/java/server/src/org/openqa/selenium/server/SeleniumDriverResourceHandler.java
@@ -33,7 +33,6 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.io.TemporaryFilesystem;
 import org.openqa.selenium.remote.SessionId;
-import org.openqa.selenium.remote.server.DriverSessions;
 import org.openqa.selenium.remote.server.log.LoggingManager;
 import org.openqa.selenium.remote.server.log.PerSessionLogHandler;
 import org.openqa.selenium.server.BrowserSessionFactory.BrowserSessionInfo;
@@ -98,8 +97,8 @@ public class SeleniumDriverResourceHandler extends ResourceHandler {
   private final BrowserSessionFactory browserSessionFactory;
 
   public SeleniumDriverResourceHandler(
-      SeleniumServer remoteControl, DriverSessions webdriverSessions) {
-    browserLauncherFactory = new BrowserLauncherFactory(webdriverSessions);
+      SeleniumServer remoteControl, BrowserLauncherFactory browserLauncherFactory) {
+    this.browserLauncherFactory = browserLauncherFactory;
     browserSessionFactory = new BrowserSessionFactory(browserLauncherFactory);
     this.remoteControl = remoteControl;
   }

diff --git a/java/server/src/org/openqa/selenium/server/SeleniumServer.java b/java/server/src/org/openqa/selenium/server/SeleniumServer.java
@@ -38,6 +38,7 @@
 import org.openqa.selenium.remote.server.log.LoggingManager;
 import org.openqa.selenium.remote.server.log.LoggingOptions;
 import org.openqa.selenium.server.BrowserSessionFactory.BrowserSessionInfo;
+import org.openqa.selenium.server.browserlaunchers.BrowserLauncherFactory;
 import org.openqa.selenium.server.browserlaunchers.Sleeper;
 import org.openqa.selenium.server.htmlrunner.HTMLLauncher;
 import org.openqa.selenium.server.htmlrunner.HTMLResultsListener;
@@ -191,6 +192,7 @@ public class SeleniumServer implements SslCertificateGenerator, IServer {
   private Log LOGGER;
 
   private Server server;
+  private BrowserLauncherFactory browserLauncherFactory;
   private SeleniumDriverResourceHandler driver;
   private SeleniumHTMLRunnerResultsHandler postResultsHandler;
   private StaticContentHandler staticContentHandler;
@@ -390,7 +392,8 @@ private HttpContext createDriverContextWithSeleniumDriverResourceHandler(
     // Associate the SeleniumDriverResourceHandler with the /selenium-server/driver context
     HttpContext driverContext = new HttpContext();
     driverContext.setContextPath("/selenium-server/driver");
-    driver = new SeleniumDriverResourceHandler(this, webdriverSessions);
+    browserLauncherFactory = new BrowserLauncherFactory(webdriverSessions);
+    driver = new SeleniumDriverResourceHandler(this, browserLauncherFactory);
     context.addHandler(driver);
     return driverContext;
   }
@@ -577,6 +580,10 @@ public RemoteControlConfiguration getConfiguration() {
     return configuration;
   }
 
+  public BrowserLauncherFactory getBrowserLauncherFactory() {
+    return browserLauncherFactory;
+  }
+
   public int getPort() {
     return configuration.getPort();
   }

diff --git a/java/server/src/org/openqa/selenium/server/browserlaunchers/BrowserLauncherFactory.java b/java/server/src/org/openqa/selenium/server/browserlaunchers/BrowserLauncherFactory.java
@@ -41,11 +41,11 @@ public class BrowserLauncherFactory {
 
   private static final Pattern CUSTOM_PATTERN = Pattern.compile("^\\*?custom( .*)?$");
 
-  private static final Map<String, Class<? extends BrowserLauncher>> supportedBrowsers =
+  private final Map<String, Class<? extends BrowserLauncher>> supportedBrowsers =
       Maps.newHashMap();
   private final DriverSessions webdriverSessions;
 
-  static {
+  {
     supportedBrowsers.put(BrowserType.FIREFOX_PROXY, FirefoxCustomProfileLauncher.class);
     supportedBrowsers.put(BrowserType.FIREFOX, FirefoxLauncher.class);
     supportedBrowsers.put(BrowserType.CHROME, FirefoxChromeLauncher.class);
@@ -122,15 +122,15 @@ public BrowserLauncher getBrowserLauncher(String browser, String sessionId,
     throw browserNotSupported(browser);
   }
 
-  public static Map<String, Class<? extends BrowserLauncher>> getSupportedLaunchers() {
+  public Map<String, Class<? extends BrowserLauncher>> getSupportedLaunchers() {
     return supportedBrowsers;
   }
 
-  public static void addBrowserLauncher(String browser, Class<? extends BrowserLauncher> clazz) {
+  public void addBrowserLauncher(String browser, Class<? extends BrowserLauncher> clazz) {
     supportedBrowsers.put(browser, clazz);
   }
 
-  public static boolean isBrowserSupported(String browser) {
+  public boolean isBrowserSupported(String browser) {
     for (String key : supportedBrowsers.keySet()) {
       final BrowserStringParser.Result result;
       result = new BrowserStringParser().parseBrowserStartCommand(key, browser);
@@ -145,7 +145,7 @@ public static boolean isBrowserSupported(String browser) {
     return false;
   }
 
-  public static String getSupportedBrowsersAsString() {
+  public String getSupportedBrowsersAsString() {
     StringBuffer str = new StringBuffer("");
     for (String name : supportedBrowsers.keySet()) {
       str.append("  *").append(name).append('\n');

diff --git a/...rver/test/org/openqa/selenium/server/browserlaunchers/BrowserLauncherFactoryUnitTest.java b/...rver/test/org/openqa/selenium/server/browserlaunchers/BrowserLauncherFactoryUnitTest.java
@@ -29,7 +29,8 @@ public class BrowserLauncherFactoryUnitTest {
 
   @Test
   public void testAllSupportedBrowsersDefineAppropriateConstructor() {
-    for (Class<? extends BrowserLauncher> c : BrowserLauncherFactory.getSupportedLaunchers()
+    BrowserLauncherFactory factory = new BrowserLauncherFactory();
+    for (Class<? extends BrowserLauncher> c : factory.getSupportedLaunchers()
         .values()) {
       try {
         c.getConstructor(Capabilities.class, RemoteControlConfiguration.class, String.class,

diff --git a/java/server/test/org/openqa/selenium/server/mock/MockPIFrameUnitTest.java b/java/server/test/org/openqa/selenium/server/mock/MockPIFrameUnitTest.java
@@ -63,8 +63,8 @@ public void setUp() throws Exception {
     configuration.setProxyInjectionModeArg(true);
 
     server = new SeleniumServer(false, configuration);
+    server.getBrowserLauncherFactory().addBrowserLauncher("dummy", DummyBrowserLauncher.class);
     server.start();
-    BrowserLauncherFactory.addBrowserLauncher("dummy", DummyBrowserLauncher.class);
     InjectionHelper.setFailOnError(false);
     LOGGER.info("Starting " + name.getMethodName());
   }