[grid] Allowing webdriver executable to be configured for drivers.

Fixes #9592
SeleniumHQ · Aug 30, 2021 · 95bc5b5 · 95bc5b5
1 parent 79b7644
commit 95bc5b5
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 30 deletions.
diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeFlags.java b/java/src/org/openqa/selenium/grid/node/config/NodeFlags.java
@@ -17,6 +17,17 @@
 
 package org.openqa.selenium.grid.node.config;
 
+import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DETECT_DRIVERS;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_HEARTBEAT_PERIOD;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_MAX_SESSIONS;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_CYCLE;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_PERIOD;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_SESSION_TIMEOUT;
+import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_VNC_ENV_VAR;
+import static org.openqa.selenium.grid.node.config.NodeOptions.NODE_SECTION;
+import static org.openqa.selenium.grid.node.config.NodeOptions.OVERRIDE_MAX_SESSIONS;
+
 import com.google.auto.service.AutoService;
 
 import com.beust.jcommander.Parameter;
@@ -31,17 +42,6 @@
 import java.util.List;
 import java.util.Set;
 
-import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DETECT_DRIVERS;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_HEARTBEAT_PERIOD;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_MAX_SESSIONS;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_CYCLE;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_PERIOD;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_SESSION_TIMEOUT;
-import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_VNC_ENV_VAR;
-import static org.openqa.selenium.grid.node.config.NodeOptions.NODE_SECTION;
-import static org.openqa.selenium.grid.node.config.NodeOptions.OVERRIDE_MAX_SESSIONS;
-
 @SuppressWarnings("unused")
 @AutoService(HasRoles.class)
 public class NodeFlags implements HasRoles {
@@ -115,19 +115,21 @@ public class NodeFlags implements HasRoles {
     description = "List of configured drivers a Node supports. " +
                   "It is recommended to provide this type of configuration through a toml config " +
                   "file to improve readability. Command line example: " +
-                  "--drivers-configuration name=\"Firefox Nightly\" max-sessions=2 " +
-                  "stereotype='{\"browserName\": \"firefox\", \"browserVersion\": \"86\", " +
+                  "--drivers-configuration display-name=\"Firefox Nightly\" max-sessions=2 " +
+                  "webdriver-path=\"/usr/local/bin/geckodriver\" "
+                  + "stereotype='{\"browserName\": \"firefox\", \"browserVersion\": \"86\", " +
                   "\"moz:firefoxOptions\": " +
                   "{\"binary\":\"/Applications/Firefox Nightly.app/Contents/MacOS/firefox-bin\"}}'",
-    arity = 3,
+    arity = 4,
     variableArity = true,
     splitter = NonSplittingSplitter.class)
   @ConfigValue(
     section = NODE_SECTION,
     name = "driver-configuration",
     prefixed = true,
     example = "\n" +
-              "name = \"Firefox Nightly\"\n" +
+              "display-name = \"Firefox Nightly\"\n" +
+              "webdriver-executable = \"/usr/local/bin/chromedriver\"\n" +
               "max-sessions = 2\n" +
               "stereotype = \"{\"browserName\": \"firefox\", \"browserVersion\": \"86\", " +
               "\"moz:firefoxOptions\": " +

diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java
@@ -39,6 +39,7 @@
 import org.openqa.selenium.json.JsonOutput;
 import org.openqa.selenium.remote.service.DriverService;
 
+import java.io.File;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.net.URI;
@@ -246,10 +247,10 @@ private void addDriverConfigs(
     Multimap<WebDriverInfo, SessionFactory> driverConfigs = HashMultimap.create();
     config.getAll(NODE_SECTION, "driver-configuration").ifPresent(drivers -> {
       /*
-        The four accepted keys are: name, max-sessions, stereotype, webdriver-executable. The
-        mandatory keys are name and stereotype. When configs are read, they keys always come
-        alphabetically ordered. This means that we know a new config is present when we find
-        the "name" key again.
+        The four accepted keys are: display-name, max-sessions, stereotype, webdriver-executable.
+        The mandatory keys are display-name and stereotype. When configs are read, they keys always
+        come alphabetically ordered. This means that we know a new config is present when we find
+        the "display-name" key again.
        */
 
       if (drivers.size() == 0) {
@@ -266,22 +267,21 @@ private void addDriverConfigs(
                                     "required 'key=value' structure");
         });
 
-      // Find all indexes where "name" is present, as it marks the start of a config
+      // Find all indexes where "display-name" is present, as it marks the start of a config
       int[] configIndexes = IntStream.range(0, drivers.size())
-        .filter(index -> drivers.get(index).startsWith("name")).toArray();
+        .filter(index -> drivers.get(index).startsWith("display-name")).toArray();
 
       if (configIndexes.length == 0) {
-        throw new ConfigException("No 'name' keyword was found in the provided configs!");
+        throw new ConfigException("No 'display-name' keyword was found in the provided configs!");
       }
 
       List<Map<String, String>> driversMap = new ArrayList<>();
       for (int i = 0; i < configIndexes.length; i++) {
         int fromIndex = configIndexes[i];
         int toIndex = (i + 1) >= configIndexes.length ? drivers.size() : configIndexes[i + 1];
         Map<String, String> configMap = new HashMap<>();
-        drivers.subList(fromIndex, toIndex).forEach(keyValue -> {
-          configMap.put(keyValue.split("=")[0], keyValue.split("=")[1]);
-        });
+        drivers.subList(fromIndex, toIndex)
+          .forEach(keyValue -> configMap.put(keyValue.split("=")[0], keyValue.split("=")[1]));
         driversMap.add(configMap);
       }
 
@@ -295,18 +295,37 @@ private void addDriverConfigs(
         if (!configMap.containsKey("stereotype")) {
           throw new ConfigException("Driver config is missing stereotype value. " + configMap);
         }
-        Capabilities stereotype =
-          enhanceStereotype(JSON.toType(configMap.get("stereotype"), Capabilities.class));
-        String configName = configMap.getOrDefault("name", "Custom Slot Config");
-        int driverMaxSessions = Integer.parseInt(configMap.getOrDefault("max-sessions", "1"));
-        Require.positive("Driver max sessions", driverMaxSessions);
+
+        Capabilities confStereotype = JSON.toType(configMap.get("stereotype"), Capabilities.class);
+        if (configMap.containsKey("webdriver-executable")) {
+          String webDriverExecutablePath = configMap.getOrDefault("webdriver-executable", "");
+          File webDriverExecutable = new File(webDriverExecutablePath);
+          if (!webDriverExecutable.isFile()) {
+            LOG.warning(
+              "Driver executable does not seem to be a file! " + webDriverExecutablePath);
+          }
+          if (!webDriverExecutable.canExecute()) {
+            LOG.warning("Driver file exists but does not seem to be a executable! "
+                        + webDriverExecutablePath);
+          }
+          confStereotype = new PersistentCapabilities(confStereotype)
+            .setCapability("se:webDriverExecutable", webDriverExecutablePath);
+        }
+        Capabilities stereotype = enhanceStereotype(confStereotype);
+
+        String configName = configMap.getOrDefault("display-name", "Custom Slot Config");
 
         WebDriverInfo info = infos.stream()
           .filter(webDriverInfo -> webDriverInfo.isSupporting(stereotype))
           .findFirst()
           .orElseThrow(() ->
                          new ConfigException("Unable to find matching driver for %s", stereotype));
 
+        int driverMaxSessions = Integer
+          .parseInt(configMap.getOrDefault("max-sessions",
+                                           String.valueOf(info.getMaximumSimultaneousSessions())));
+        Require.positive("Driver max sessions", driverMaxSessions);
+
         WebDriverInfo driverInfoConfig = createConfiguredDriverInfo(info, stereotype, configName);
 
         builders.stream()

diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java b/java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java
@@ -37,6 +37,7 @@
 import org.openqa.selenium.remote.service.DriverService;
 import org.openqa.selenium.remote.tracing.Tracer;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -88,6 +89,8 @@ private static Collection<SessionFactory> createSessionFactory(
     Capabilities stereotype) {
     ImmutableList.Builder<SessionFactory> toReturn = ImmutableList.builder();
     SlotMatcher slotMatcher = new DefaultSlotMatcher();
+    String webDriverExecutablePath =
+      String.valueOf(stereotype.asMap().getOrDefault("se:webDriverExecutable", ""));
 
     builders.stream()
       .filter(builder -> builder.score(stereotype) > 0)
@@ -100,6 +103,10 @@ private static Collection<SessionFactory> createSessionFactory(
           // and the DriverService creation needs to be thread safe.
           Object driverBuilder = clazz.newInstance();
           driverServiceBuilder = ((DriverService.Builder<?, ?>) driverBuilder).usingAnyFreePort();
+          if (!webDriverExecutablePath.isEmpty()) {
+            driverServiceBuilder =
+              driverServiceBuilder.usingDriverExecutable(new File(webDriverExecutablePath));
+          }
         } catch (InstantiationException | IllegalAccessException e) {
           throw new IllegalArgumentException(String.format(
             "Class %s could not be found or instantiated", clazz));

diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java
@@ -303,6 +303,56 @@ public void driversCanBeConfigured() {
       .hasSize(2);
   }
 
+  @Test
+  public void driversCanBeConfiguredWithASpecificWebDriverBinary() {
+    String chLocation = "/Applications/Google Chrome Beta.app/Contents/MacOS/Google Chrome Beta";
+    String ffLocation = "/Applications/Firefox Nightly.app/Contents/MacOS/firefox-bin";
+    String chromeDriverLocation = "/path/to/chromedriver_beta/chromedriver";
+    String geckoDriverLocation = "/path/to/geckodriver_nightly/geckodriver";
+    ChromeOptions chromeOptions = new ChromeOptions();
+    chromeOptions.setBinary(chLocation);
+    FirefoxOptions firefoxOptions = new FirefoxOptions();
+    firefoxOptions.setBinary(ffLocation);
+    StringBuilder chromeCaps = new StringBuilder();
+    StringBuilder firefoxCaps = new StringBuilder();
+    new Json().newOutput(chromeCaps).setPrettyPrint(false).write(chromeOptions);
+    new Json().newOutput(firefoxCaps).setPrettyPrint(false).write(firefoxOptions);
+
+    String[] rawConfig = new String[]{
+      "[node]",
+      "detect-drivers = false",
+      "[[node.driver-configuration]]",
+      "display-name = \"Chrome Beta\"",
+      String.format("webdriver-executable = '%s'", chromeDriverLocation),
+      String.format("stereotype = \"%s\"", chromeCaps.toString().replace("\"", "\\\"")),
+      "[[node.driver-configuration]]",
+      "display-name = \"Firefox Nightly\"",
+      String.format("webdriver-executable = '%s'", geckoDriverLocation),
+      String.format("stereotype = \"%s\"", firefoxCaps.toString().replace("\"", "\\\""))
+    };
+    Config config = new TomlConfig(new StringReader(String.join("\n", rawConfig)));
+
+    List<Capabilities> reported = new ArrayList<>();
+    new NodeOptions(config).getSessionFactories(capabilities -> {
+      reported.add(capabilities);
+      return Collections.singleton(HelperFactory.create(config, capabilities));
+    });
+
+    assertThat(reported).is(supporting("chrome"));
+    assertThat(reported).is(supporting("firefox"));
+    assertThat(reported)
+      .filteredOn(capabilities -> capabilities.asMap().containsKey(ChromeOptions.CAPABILITY))
+      .allMatch(
+        capabilities ->
+          chromeDriverLocation.equals(capabilities.getCapability("se:webDriverExecutable")));
+
+    assertThat(reported)
+      .filteredOn(capabilities -> capabilities.asMap().containsKey(FirefoxOptions.FIREFOX_OPTIONS))
+      .anyMatch(
+        capabilities ->
+          geckoDriverLocation.equals(capabilities.getCapability("se:webDriverExecutable")));
+  }
+
   @Test
   public void driversConfigNeedsStereotypeField() {
     String[] rawConfig = new String[]{