fixing grid launching with node role and including -browser

lukeis · lukeis · commit 55e987831d25 · 2016-07-13T14:18:36.000-07:00
JCommander Converters need to be in their own class file, since when running tests inside the IDE java will think they have a declared constructor even though they don't, simply because they are an inner class. created a new e2e test that mimics the real world example that everyone does Fixes #2355
diff --git a/.idea/libraries/jetty.xml b/.idea/libraries/jetty.xml
diff --git a/.idea/libraries/jetty_util_9_2_13_v20150730.xml b/.idea/libraries/jetty_util_9_2_13_v20150730.xml
diff --git a/.idea/libraries/websocket_9_2_15_v20160210.xml b/.idea/libraries/websocket_9_2_15_v20160210.xml
diff --git a/java/client/client.iml b/java/client/client.iml
@@ -45,6 +45,8 @@
     <orderEntry type="library" name="mockito-core-1.9.5-srcs" level="project" />
     <orderEntry type="library" name="objenesis-1.0" level="project" />
     <orderEntry type="library" name="commons-httpclient" level="project" />
+    <orderEntry type="library" scope="TEST" name="websocket-9.2.15.v20160210" level="project" />
+    <orderEntry type="library" scope="TEST" name="jetty-util-9.2.13.v20150730" level="project" />
   </component>
   <component name="sonarModuleSettings">
     <option name="alternativeWorkingDirPath" value="" />
@@ -53,4 +55,4 @@
     <option name="useAlternativeWorkingDir" value="false" />
     <option name="workingDirSelection" value="&lt;PROJECT&gt;" />
   </component>
-</module>
+</module>
diff --git a/java/server/server.iml b/java/server/server.iml
@@ -11,15 +11,12 @@
     <orderEntry type="inheritedJdk" />
     <orderEntry type="sourceFolder" forTests="false" />
     <orderEntry type="module" module-name="client" exported="" />
-    <orderEntry type="library" name="bouncycastle" level="project" />
     <orderEntry type="library" name="commons-io" level="project" />
     <orderEntry type="library" name="commons-logging" level="project" />
     <orderEntry type="library" name="jcip-annotations" level="project" />
     <orderEntry type="library" name="jcommander" level="project" />
     <orderEntry type="library" name="jetty" level="project" />
-    <orderEntry type="library" name="jetty-for-rc" level="project" />
     <orderEntry type="library" name="junit" level="project" />
-    <orderEntry type="library" name="mx4j" level="project" />
     <orderEntry type="library" name="servlet-api" level="project" />
     <orderEntry type="library" name="snakeyaml" level="project" />
     <orderEntry type="library" scope="TEST" name="hamcrest" level="project" />
diff --git a/java/server/src/org/openqa/grid/common/RegistrationRequest.java b/java/server/src/org/openqa/grid/common/RegistrationRequest.java
@@ -106,7 +106,7 @@ public JsonObject getAssociatedJSON() {
     res.addProperty("id", configuration.id);
     res.addProperty("name", name);
     res.addProperty("description", description);
-    res.add("configuration", new Gson().toJsonTree(configuration));
+    res.add("configuration", configuration.toJson());
     JsonArray caps = new JsonArray();
     for (DesiredCapabilities c : capabilities) {
       caps.add(new Gson().toJsonTree(c.asMap()));
@@ -137,9 +137,7 @@ public static RegistrationRequest getNewInstance(String json) throws JsonSyntaxE
     }
 
     JsonObject config = o.get("configuration").getAsJsonObject();
-    GridNodeConfiguration
-      configuration =
-      new Gson().fromJson(config, GridNodeConfiguration.class);
+    GridNodeConfiguration configuration = GridNodeConfiguration.loadFromJSON(config);
     request.setConfiguration(configuration);
 
     if (o.has("id")) {
@@ -185,6 +183,10 @@ public static RegistrationRequest build(GridNodeConfiguration configuration) {
     res.role = GridRole.get(configuration.role);
     res.addPlatformInfoToCapabilities();
 
+    if (configuration.browser.size() > 0) {
+      res.capabilities = configuration.browser;
+    }
+
     for (DesiredCapabilities cap : res.capabilities) {
       if (cap.getCapability(SELENIUM_PROTOCOL) == null) {
         cap.setCapability(SELENIUM_PROTOCOL, SeleniumProtocol.WebDriver.toString());
diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/GridConfiguration.java b/java/server/src/org/openqa/grid/internal/utils/configuration/GridConfiguration.java
@@ -20,6 +20,8 @@
 import com.beust.jcommander.IStringConverter;
 import com.beust.jcommander.Parameter;
 
+import org.openqa.grid.internal.utils.configuration.converters.CustomConverter;
+
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -56,19 +58,6 @@ public class GridConfiguration extends StandaloneConfiguration {
     description = "<String> : list of extra servlets this hub will display. Allows to present custom view of the hub for monitoring and management purposes. Specify multiple on the command line: -servlet tld.company.ServletA -servlet tld.company.ServletB. The servlet must exist in the path: /grid/admin/ServletA /grid/admin/ServletB"
   )
   public List<String> servlets;
-
-  private class CustomConverter implements IStringConverter<Map<String,String>> {
-    @Override
-    public Map<String,String> convert(String value) {
-      Map<String,String> custom = new HashMap<>();
-      for (String pair : value.split(",")) {
-        String[] pieces = pair.split("=");
-        custom.put(pieces[0], pieces[1]);
-      }
-      return custom;
-    }
-  }
-
   /**
    * replaces this instance of configuration value with the 'other' value if it's set.
    * @param other
diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java b/java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java
@@ -17,30 +17,45 @@
 
 package org.openqa.grid.internal.utils.configuration;
 
-import com.beust.jcommander.IStringConverter;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonObject;
+import com.google.gson.TypeAdapter;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
+
 import com.beust.jcommander.Parameter;
 
+import org.openqa.grid.common.exception.GridConfigurationException;
+import org.openqa.grid.internal.utils.configuration.converters.BrowserDesiredCapabilityConverter;
+import org.openqa.grid.internal.utils.configuration.converters.NoOpParameterSplitter;
+import org.openqa.selenium.remote.BeanToJsonConverter;
 import org.openqa.selenium.remote.DesiredCapabilities;
 
+import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 public class GridNodeConfiguration extends GridConfiguration {
 
+  // remoteHost is a generated value based on host / port specified, or read from JSON.
+  String remoteHost;
+
   @Parameter(
     names = "-id",
     description = "<String> : unique identifier for the node. Not required--by default, grid will use the url of the remoteHost"
   )
   public String id;
 
-  // remoteHost is a generated value based on host / port specified, or read from JSON.
-  String remoteHost;
-
   @Parameter(
     names = "-browser",
     description = "<String> : comma separated Capability values. Example: -browser browserName=firefox,platform=linux -browser browserName=chrome,platform=linux",
-    converter = DesiredCapabilityConverter.class
+    listConverter = BrowserDesiredCapabilityConverter.class,
+    converter = BrowserDesiredCapabilityConverter.class,
+    splitter = NoOpParameterSplitter.class
   )
   public List<DesiredCapabilities> browser;
 
@@ -153,18 +168,6 @@ private void parseHubUrl() {
     }
   }
 
-  private class DesiredCapabilityConverter implements IStringConverter<DesiredCapabilities> {
-    @Override
-    public DesiredCapabilities convert(String value) {
-      DesiredCapabilities capabilities = new DesiredCapabilities();
-      for (String cap : value.split(",")) {
-        String[] pieces = cap.split("=");
-        capabilities.setCapability(pieces[0], pieces[1]);
-      }
-      return capabilities;
-    }
-  }
-
   public void merge(GridNodeConfiguration other) {
     super.merge(other);
     if (other.browser != null) {
@@ -228,4 +231,44 @@ public String toString(String format) {
     sb.append(toString(format, "unregisterIfStillDownAfter", unregisterIfStillDownAfter));
     return sb.toString();
   }
+
+  /**
+   * @param json JsonObject to load configuration from
+   */
+  public static GridNodeConfiguration loadFromJSON(JsonObject json) {
+
+    try {
+      GsonBuilder builder = new GsonBuilder();
+      GridNodeConfiguration.staticAddJsonTypeAdapter(builder);
+      return builder.create().fromJson(json, GridNodeConfiguration.class);
+    } catch (Throwable e) {
+      throw new GridConfigurationException("Error with the JSON of the config : " + e.getMessage(),
+                                           e);
+    }
+  }
+
+  @Override
+  protected void addJsonTypeAdapter(GsonBuilder builder) {
+    super.addJsonTypeAdapter(builder);
+    GridNodeConfiguration.staticAddJsonTypeAdapter(builder);
+  }
+  protected static void staticAddJsonTypeAdapter(GsonBuilder builder) {
+    builder.registerTypeAdapter(DesiredCapabilities.class, new DesiredCapabilitiesAdapter().nullSafe());
+  }
+
+  protected static class DesiredCapabilitiesAdapter<T> extends TypeAdapter<DesiredCapabilities> {
+
+    @Override
+    public void write(JsonWriter jsonWriter, DesiredCapabilities t) throws IOException {
+      jsonWriter.value(String.format("{\"capabilities\":%s}", new BeanToJsonConverter().convert(t.asMap())));
+    }
+
+    @Override
+    public DesiredCapabilities read(JsonReader jsonReader) throws IOException {
+      Gson gson = new GsonBuilder().create();
+      Map<String, Map<String, Object>> capability = new HashMap<>();
+      capability = gson.fromJson(jsonReader.nextString(), capability.getClass());
+      return new DesiredCapabilities(capability.get("capabilities"));
+    }
+  }
 }
diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/converters/BrowserDesiredCapabilityConverter.java b/java/server/src/org/openqa/grid/internal/utils/configuration/converters/BrowserDesiredCapabilityConverter.java
@@ -0,0 +1,21 @@
+package org.openqa.grid.internal.utils.configuration.converters;
+
+import com.beust.jcommander.IStringConverter;
+
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+public class BrowserDesiredCapabilityConverter implements IStringConverter<DesiredCapabilities> {
+  @Override
+  public DesiredCapabilities convert(String value) {
+    DesiredCapabilities capabilities = new DesiredCapabilities();
+    for (String cap : value.split(",")) {
+      String[] pieces = cap.split("=");
+      if (pieces[0].equals("maxInstances")) {
+        capabilities.setCapability(pieces[0], Integer.parseInt(pieces[1], 10));
+      } else {
+        capabilities.setCapability(pieces[0], pieces[1]);
+      }
+    }
+    return capabilities;
+  }
+}
diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/converters/CustomConverter.java b/java/server/src/org/openqa/grid/internal/utils/configuration/converters/CustomConverter.java
@@ -0,0 +1,18 @@
+package org.openqa.grid.internal.utils.configuration.converters;
+
+import com.beust.jcommander.IStringConverter;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class CustomConverter implements IStringConverter<Map<String,String>> {
+  @Override
+  public Map<String,String> convert(String value) {
+    Map<String,String> custom = new HashMap<>();
+    for (String pair : value.split(",")) {
+      String[] pieces = pair.split("=");
+      custom.put(pieces[0], pieces[1]);
+    }
+    return custom;
+  }
+}
diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/converters/NoOpParameterSplitter.java b/java/server/src/org/openqa/grid/internal/utils/configuration/converters/NoOpParameterSplitter.java
@@ -0,0 +1,13 @@
+package org.openqa.grid.internal.utils.configuration.converters;
+
+import com.beust.jcommander.converters.IParameterSplitter;
+
+import java.util.Arrays;
+import java.util.List;
+
+public class NoOpParameterSplitter implements IParameterSplitter {
+  @Override
+  public List<String> split(String value) {
+    return Arrays.asList(value);
+  }
+}
diff --git a/java/server/src/org/openqa/grid/selenium/GridLauncherV3.java b/java/server/src/org/openqa/grid/selenium/GridLauncherV3.java
@@ -230,6 +230,9 @@ public void setConfiguration(String[] args) {
             configuration = new GridNodeConfiguration();
             new JCommander(configuration, args);
             helpRequested = configuration.help;
+            if (configuration.port == null) {
+              configuration.port = 5555;
+            }
           }
 
           public void launch() throws Exception {
diff --git a/java/server/test/org/openqa/grid/e2e/GridE2ETests.java b/java/server/test/org/openqa/grid/e2e/GridE2ETests.java
@@ -27,6 +27,7 @@
 import org.openqa.grid.e2e.misc.ConfigInheritanceTest;
 import org.openqa.grid.e2e.misc.Grid1HeartbeatTest;
 import org.openqa.grid.e2e.misc.GridSerializeExceptionTest;
+import org.openqa.grid.e2e.misc.GridViaCommandLineTest;
 import org.openqa.grid.e2e.misc.HubRestart;
 import org.openqa.grid.e2e.misc.HubRestartNeg;
 import org.openqa.grid.e2e.misc.WebDriverPriorityDemo;
@@ -58,6 +59,7 @@
     NodeTimeOutTest.class,
     SmokeTest.class, // slow
     ExtraServletUtilTest.class,
+    GridViaCommandLineTest.class,
 })
 public class GridE2ETests {
 }
diff --git a/java/server/test/org/openqa/grid/e2e/misc/GridViaCommandLineTest.java b/java/server/test/org/openqa/grid/e2e/misc/GridViaCommandLineTest.java
@@ -0,0 +1,63 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.grid.e2e.misc;
+
+import junit.framework.Assert;
+
+import org.junit.Test;
+import org.openqa.grid.selenium.GridLauncherV3;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.net.UrlChecker;
+import org.openqa.selenium.remote.DesiredCapabilities;
+import org.openqa.selenium.remote.RemoteWebDriver;
+
+import java.net.URL;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Ensure that launching the hub / node in most common ways simulating command line args works
+ */
+public class GridViaCommandLineTest {
+
+  @Test
+  public void testRegisterNodeToHub() throws Exception {
+    String[] hubArgs = {"-role", "hub"};
+    GridLauncherV3.main(hubArgs);
+    UrlChecker urlChecker = new UrlChecker();
+    urlChecker.waitUntilAvailable(10, TimeUnit.SECONDS, new URL("http://localhost:4444/grid/console"));
+
+    String[] nodeArgs = {"-role", "node", "-hub", "http://localhost:4444", "-browser", "browserName=chrome,maxInstances=1"};
+    GridLauncherV3.main(nodeArgs);
+    urlChecker.waitUntilAvailable(100, TimeUnit.SECONDS, new URL("http://localhost:5555/wd/hub/status"));
+
+    WebDriver driver = new RemoteWebDriver(new URL("http://localhost:4444/wd/hub"),
+                                                   DesiredCapabilities.chrome());
+
+    try {
+      driver.get("http://localhost:4444/grid/console");
+      Assert.assertEquals("Should only have one chrome registered to the hub", 1, driver.findElements(By.cssSelector("img[src$='chrome.png']")).size());
+    } finally {
+      try {
+        driver.quit();
+      } catch (Exception e) {}
+    }
+
+
+  }
+}
diff --git a/java/server/test/org/openqa/grid/e2e/node/SmokeTest.java b/java/server/test/org/openqa/grid/e2e/node/SmokeTest.java
diff --git a/java/server/test/org/openqa/grid/e2e/utils/GridTestHelper.java b/java/server/test/org/openqa/grid/e2e/utils/GridTestHelper.java
