Cleaning required capabilities out of tests, step 2: driver suppliers…

… should not use driver constructors that accept required capabilities
SeleniumHQ · Aug 29, 2017 · 05bc5bf · 05bc5bf
1 parent a38aec8
commit 05bc5bf
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 56 deletions.
diff --git a/java/client/test/org/openqa/selenium/testing/drivers/DefaultDriverSupplier.java b/java/client/test/org/openqa/selenium/testing/drivers/DefaultDriverSupplier.java
@@ -31,12 +31,9 @@ public class DefaultDriverSupplier implements Supplier<WebDriver> {
   private static final Logger log = Logger.getLogger(DefaultDriverSupplier.class.getName());
   private Class<? extends WebDriver> driverClass;
   private final Capabilities desiredCapabilities;
-  private final Capabilities requiredCapabilities;
 
-  public DefaultDriverSupplier(Capabilities desiredCapabilities,
-      Capabilities requiredCapabilities) {
+  public DefaultDriverSupplier(Capabilities desiredCapabilities) {
     this.desiredCapabilities = desiredCapabilities;
-    this.requiredCapabilities = requiredCapabilities;
 
     try {
       // Only support a default driver if we're actually in dev mode.
@@ -55,8 +52,7 @@ public WebDriver get() {
     log.info("Providing default driver instance");
 
     try {
-      return driverClass.getConstructor(Capabilities.class, Capabilities.class).
-          newInstance(desiredCapabilities, requiredCapabilities);
+      return driverClass.getConstructor(Capabilities.class).newInstance(desiredCapabilities);
     } catch (InvocationTargetException e) {
       throw new RuntimeException(e.getTargetException());
     } catch (ReflectiveOperationException e) {

diff --git a/java/client/test/org/openqa/selenium/testing/drivers/ExternalDriverSupplier.java b/java/client/test/org/openqa/selenium/testing/drivers/ExternalDriverSupplier.java
@@ -65,24 +65,21 @@ class ExternalDriverSupplier implements Supplier<WebDriver> {
   private static final String EXTERNAL_SERVER_URL_PROPERTY = "selenium.external.serverUrl";
 
   private final Capabilities desiredCapabilities;
-  private final Capabilities requiredCapabilities;
 
-  ExternalDriverSupplier(Capabilities desiredCapabilities, Capabilities requiredCapabilities) {
+  ExternalDriverSupplier(Capabilities desiredCapabilities) {
     this.desiredCapabilities = new DesiredCapabilities(desiredCapabilities);
-    this.requiredCapabilities = new DesiredCapabilities(requiredCapabilities);
   }
 
   @Override
   public WebDriver get() {
-    Optional<Supplier<WebDriver>> delegate = createDelegate(
-        desiredCapabilities, requiredCapabilities);
-    delegate = createForExternalServer(desiredCapabilities, requiredCapabilities, delegate);
+    Optional<Supplier<WebDriver>> delegate = createDelegate(desiredCapabilities);
+    delegate = createForExternalServer(desiredCapabilities, delegate);
 
     return delegate.orElse(Suppliers.ofInstance(null)).get();
   }
 
   private static Optional<Supplier<WebDriver>> createForExternalServer(
-      Capabilities desiredCapabilities, Capabilities requiredCapabilities,
+      Capabilities desiredCapabilities,
       Optional<Supplier<WebDriver>> delegate) {
     String externalUrl = System.getProperty(EXTERNAL_SERVER_URL_PROPERTY);
     if (externalUrl != null) {
@@ -93,27 +90,24 @@ private static Optional<Supplier<WebDriver>> createForExternalServer(
       } catch (MalformedURLException e) {
         throw new RuntimeException("Invalid server URL: " + externalUrl, e);
       }
-      Supplier<WebDriver> defaultSupplier = new DefaultRemoteSupplier(
-          url, desiredCapabilities, requiredCapabilities);
+      Supplier<WebDriver> defaultSupplier = new DefaultRemoteSupplier(url, desiredCapabilities);
       Supplier<WebDriver> supplier = new ExternalServerDriverSupplier(
           url, delegate.orElse(defaultSupplier));
       return Optional.of(supplier);
     }
     return delegate;
   }
 
-  private static Optional<Supplier<WebDriver>> createDelegate(
-      Capabilities desiredCapabilities, Capabilities requiredCapabilities) {
+  private static Optional<Supplier<WebDriver>> createDelegate(Capabilities desiredCapabilities) {
     Optional<Class<? extends Supplier>> supplierClass = getDelegateClass();
     if (supplierClass.isPresent()) {
       Class<? extends Supplier> clazz = supplierClass.get();
       logger.info("Using delegate supplier: " + clazz.getName());
       try {
         @SuppressWarnings("unchecked")
         Constructor<Supplier<WebDriver>> ctor =
-            (Constructor<Supplier<WebDriver>>) clazz.getConstructor(
-                Capabilities.class, Capabilities.class);
-        return Optional.of(ctor.newInstance(desiredCapabilities, requiredCapabilities));
+            (Constructor<Supplier<WebDriver>>) clazz.getConstructor(Capabilities.class);
+        return Optional.of(ctor.newInstance(desiredCapabilities));
       } catch (InvocationTargetException e) {
         throw new RuntimeException(e.getTargetException());
       } catch (Exception e) {
@@ -175,18 +169,15 @@ public WebDriver get() {
   private static class DefaultRemoteSupplier implements Supplier<WebDriver> {
     private final URL url;
     private final Capabilities desiredCapabilities;
-    private final Capabilities requiredCapabilities;
 
-    private DefaultRemoteSupplier(
-        URL url, Capabilities desiredCapabilities, Capabilities requiredCapabilities) {
+    private DefaultRemoteSupplier(URL url, Capabilities desiredCapabilities) {
       this.url = url;
       this.desiredCapabilities = desiredCapabilities;
-      this.requiredCapabilities = requiredCapabilities;
     }
 
     @Override
     public WebDriver get() {
-      RemoteWebDriver driver = new RemoteWebDriver(url, desiredCapabilities, requiredCapabilities);
+      RemoteWebDriver driver = new RemoteWebDriver(url, desiredCapabilities);
       driver.setFileDetector(new LocalFileDetector());
       return driver;
     }

diff --git a/java/client/test/org/openqa/selenium/testing/drivers/ReflectionBackedDriverSupplier.java b/java/client/test/org/openqa/selenium/testing/drivers/ReflectionBackedDriverSupplier.java
@@ -33,12 +33,9 @@ public class ReflectionBackedDriverSupplier implements Supplier<WebDriver> {
   private final static Logger log =
       Logger.getLogger(ReflectionBackedDriverSupplier.class.getName());
   private final Capabilities desiredCapabilities;
-  private final Capabilities requiredCapabilities;
 
-  public ReflectionBackedDriverSupplier(Capabilities desiredCapabilities,
-      Capabilities requiredCapabilities) {
+  public ReflectionBackedDriverSupplier(Capabilities desiredCapabilities) {
     this.desiredCapabilities = desiredCapabilities;
-    this.requiredCapabilities = requiredCapabilities;
   }
 
   public WebDriver get() {
@@ -50,13 +47,6 @@ public WebDriver get() {
         return null;
       }
 
-      try {
-          return driverClass.getConstructor(Capabilities.class,
-             Capabilities.class).newInstance(desiredCapsToUse, requiredCapabilities);
-      } catch (NoSuchMethodException e) {
-          // ignore
-      }
-
       return driverClass.getConstructor(Capabilities.class).newInstance(desiredCapsToUse);
     } catch (InvocationTargetException e) {
       throw new RuntimeException(e.getTargetException());

diff --git a/java/client/test/org/openqa/selenium/testing/drivers/RemoteSupplier.java b/java/client/test/org/openqa/selenium/testing/drivers/RemoteSupplier.java
@@ -30,12 +30,9 @@ public class RemoteSupplier implements Supplier<WebDriver> {
   private static OutOfProcessSeleniumServer server = new OutOfProcessSeleniumServer();
   private static volatile boolean started;
   private Capabilities desiredCapabilities;
-  private Capabilities requiredCapabilities;
 
-  public RemoteSupplier(Capabilities desiredCapabilities,
-      Capabilities requiredCapabilities) {
+  public RemoteSupplier(Capabilities desiredCapabilities) {
     this.desiredCapabilities = desiredCapabilities;
-   this.requiredCapabilities = requiredCapabilities;
   }
 
   public WebDriver get() {
@@ -47,8 +44,7 @@ public WebDriver get() {
       startServer();
     }
 
-    RemoteWebDriver driver = new RemoteWebDriver(
-        server.getWebDriverUrl(), desiredCapabilities, requiredCapabilities);
+    RemoteWebDriver driver = new RemoteWebDriver(server.getWebDriverUrl(), desiredCapabilities);
     driver.setFileDetector(new LocalFileDetector());
     return driver;
   }

diff --git a/java/client/test/org/openqa/selenium/testing/drivers/WebDriverBuilder.java b/java/client/test/org/openqa/selenium/testing/drivers/WebDriverBuilder.java
@@ -31,7 +31,6 @@
 
 public class WebDriverBuilder implements Supplier<WebDriver> {
   private Capabilities desiredCapabilities;
-  private Capabilities requiredCapabilities;
   private final Browser browser;
 
   public WebDriverBuilder() {
@@ -51,8 +50,7 @@ public WebDriver get() {
     Capabilities desiredCaps = new DesiredCapabilities(standardCapabilities,
         desiredCapabilities);
 
-    List<Supplier<WebDriver>> suppliers = getSuppliers(desiredCaps,
-        requiredCapabilities);
+    List<Supplier<WebDriver>> suppliers = getSuppliers(desiredCaps);
 
     for (Supplier<WebDriver> supplier : suppliers) {
       WebDriver driver = supplier.get();
@@ -81,17 +79,16 @@ private void modifyLogLevel(WebDriver driver) {
     }
   }
 
-  private List<Supplier<WebDriver>> getSuppliers(Capabilities desiredCaps,
-      Capabilities requiredCaps) {
+  private List<Supplier<WebDriver>> getSuppliers(Capabilities desiredCaps) {
     List<Supplier<WebDriver>> suppliers = Lists.newArrayList();
-    suppliers.add(new ExternalDriverSupplier(desiredCaps, requiredCaps));
+    suppliers.add(new ExternalDriverSupplier(desiredCaps));
     suppliers.add(new SauceBackedDriverSupplier(desiredCaps));
     suppliers.add(new GridSupplier(desiredCaps));
-    suppliers.add(new RemoteSupplier(desiredCaps, requiredCaps));
+    suppliers.add(new RemoteSupplier(desiredCaps));
     suppliers.add(new PhantomJSDriverSupplier(desiredCaps));
     suppliers.add(new TestInternetExplorerSupplier(desiredCaps));
-    suppliers.add(new ReflectionBackedDriverSupplier(desiredCaps, requiredCaps));
-    suppliers.add(new DefaultDriverSupplier(desiredCaps, requiredCaps));
+    suppliers.add(new ReflectionBackedDriverSupplier(desiredCaps));
+    suppliers.add(new DefaultDriverSupplier(desiredCaps));
     return suppliers;
   }
 
@@ -100,11 +97,6 @@ public WebDriverBuilder setDesiredCapabilities(Capabilities caps) {
     return this;
   }
 
-  public WebDriverBuilder setRequiredCapabilities(Capabilities caps) {
-    this.requiredCapabilities = caps;
-    return this;
-  }
-
   private enum LogLevel {
     OFF("OFF", Level.OFF),
     DEBUG("DEBUG", Level.FINE),