Use ProcessBuilder, not deprecated CommandLine (#278)

Author: sbabcoc

src/main/java/com/nordstrom/automation/selenium/examples/ExamplePage.java

@@ -1,13 +1,13 @@
 package com.nordstrom.automation.selenium.examples;
 
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
-import org.apache.http.client.utils.URIBuilder;
 import org.openqa.selenium.By;
 import org.openqa.selenium.SearchContext;
 import org.openqa.selenium.WebDriver;
@@ -20,6 +20,7 @@
 import com.nordstrom.automation.selenium.model.Page;
 import com.nordstrom.automation.selenium.model.RobustWebElement;
 import com.nordstrom.automation.selenium.servlet.ExamplePageLauncher;
+import com.nordstrom.common.uri.UriUtils;
 
 @PageUrl("/grid/admin/ExamplePageServlet")
 public class ExamplePage extends Page {
@@ -265,15 +266,11 @@ public static URI setHubAsTarget() {
             } else {
                 // get URL of Selenium Grid hub server
                 URL hubUrl = config.getSeleniumGrid().getHubServer().getUrl();
-                // assemble hub base URI
-                targetUri = new URIBuilder()
-                        .setScheme(hubUrl.getProtocol())
-                        .setHost(hubUrl.getHost())
-                        .setPort(hubUrl.getPort())
-                        .build().normalize();
+                // get base URI of grid hub server
+                targetUri = UriUtils.uriForPath(hubUrl);
             }
             config.setTargetUri(targetUri);
-        } catch (URISyntaxException eaten) {
+        } catch (URISyntaxException | IOException eaten) {
             // nothing to do here
         }
         return targetUri;

src/main/java/com/nordstrom/automation/selenium/exceptions/GridServerLaunchFailedException.java

@@ -1,7 +1,5 @@
 package com.nordstrom.automation.selenium.exceptions;
 
-import java.io.IOException;
-
 /**
  * Thrown if a Grid component process failed to start.
  */
@@ -16,7 +14,7 @@ public class GridServerLaunchFailedException extends RuntimeException {
      * @param role Grid server role specifier ({@code hub} or {@code node})
      * @param cause the cause of this exception
      */
-    public GridServerLaunchFailedException(final String role, final IOException cause) {
+    public GridServerLaunchFailedException(final String role, final Throwable cause) {
         super(getMessage(role), cause);
     }
 

src/main/java/com/nordstrom/automation/selenium/servlet/ExamplePageLauncher.java

@@ -1,45 +1,46 @@
 package com.nordstrom.automation.selenium.servlet;
 
 import java.io.File;
+import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
-import org.openqa.selenium.os.CommandLine;
 
 import com.nordstrom.automation.selenium.core.GridUtility;
 import com.nordstrom.automation.selenium.examples.ServletContainer;
 import com.nordstrom.common.file.PathUtils;
 import com.nordstrom.common.jar.JarUtils;
 
-@SuppressWarnings("deprecation")
 public class ExamplePageLauncher {
 
     public enum Container {
         INSTANCE;
 
-        private final CommandLine process;
+        private final ProcessBuilder builder;
+        private Process process;
         private boolean hasStarted = false;
         private boolean isActive = false;
 
         private Container() {
             List<String> argsList = new ArrayList<>();
+            argsList.add(System.getProperty("java.home") + File.separator + "bin" + File.separator + "java");
             argsList.add("-cp");
             argsList.add(JarUtils.getContextPaths(ServletContainer.getDependencyContexts()).get(0));
             argsList.add(ServletContainer.class.getName());
 
-            String executable = System.getProperty("java.home") + File.separator + "bin" + File.separator + "java";
-            process = new CommandLine(executable, argsList.toArray(new String[0]));
-            process.setEnvironmentVariable("PATH", PathUtils.getSystemPath());
+            builder = new ProcessBuilder(argsList);
+            builder.environment().put("PATH", PathUtils.getSystemPath());
         }
 
         /**
          * Start the process associated with the example page servlet.
+         * @throws IOException if an I/O error occurs
          */
-        public void start() {
+        public void start() throws IOException {
             if (!hasStarted) {
-                process.executeAsync();
+                process = builder.start();
                 hasStarted = true;
             }
         }
@@ -58,12 +59,14 @@ public boolean isActive() {
 
         /**
          * Stop the example page servlet.
+         * @throws InterruptedException if interrupted while awaiting shutdown
          */
-        public void shutdown() {
+        public void shutdown() throws InterruptedException {
             if (isActive()) {
                 process.destroy();
                 hasStarted = false;
                 isActive = false;
+                process.waitFor();
             }
         }
 

src/main/java/com/nordstrom/automation/selenium/plugins/AbstractAppiumPlugin.java renamed to src/selenium3/java/com/nordstrom/automation/selenium/plugins/AbstractAppiumPlugin.java

@@ -1,17 +1,13 @@
 package com.nordstrom.automation.selenium.plugins;
 
-import static org.openqa.selenium.json.Json.MAP_TYPE;
-
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.Reader;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.URL;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
@@ -25,8 +21,6 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
-import org.openqa.selenium.grid.config.ConfigException;
-import org.openqa.selenium.json.Json;
 import org.openqa.selenium.net.PortProber;
 import org.openqa.selenium.os.CommandLine;
 import org.openqa.selenium.remote.RemoteWebDriver;
@@ -71,7 +65,6 @@
  *  </exclusions>
  *&lt;/dependency&gt;</pre>
  */
-@SuppressWarnings("deprecation")
 public abstract class AbstractAppiumPlugin implements DriverPlugin {
 
     private static final String[] DEPENDENCY_CONTEXTS = {};
@@ -119,7 +112,6 @@ public String[] getPropertyNames(String capabilities) {
      * {@inheritDoc}
      */
     @Override
-    @SuppressWarnings("unchecked")
     public LocalGridServer create(SeleniumConfig config, String launcherClassName, String[] dependencyContexts,
             URL hubUrl, Path workingPath, Path outputPath) throws IOException {
 
@@ -184,35 +176,17 @@ public LocalGridServer create(SeleniumConfig config, String launcherClassName, S
             }
         }
 
-        // if target is Selenium 3
-        if (config.getVersion() == 3) {
-            // get 'localhost' and free port
-            address = GridUtility.getLocalHost();
-            portNum = PortProber.findFreePort();
-            
-            // add 'base-path' argument
-            argsList.add("--base-path");
-            argsList.add("/wd/hub");
-            
-            // add 'nodeconfig' path
-            argsList.add("--nodeconfig");
-            argsList.add(nodeConfigPath.toString());
-        // otherwise (target is Selenium 4+)
-        } else {
-            // extract address and port from relay configuration
-            try (Reader reader = Files.newBufferedReader(nodeConfigPath)) {
-                Map<String, Object> nodeConfig = new Json().toType(reader, MAP_TYPE);
-                Map<String, Object> relayOptions = (Map<String, Object>) nodeConfig.get("relay");
-                address = (String) relayOptions.get("host");
-                portNum = ((Long) relayOptions.get("port")).intValue();
-            } catch (IOException e) {
-                throw new ConfigException("Failed reading node configuration.", e);
-            }
-            
-            // add driver specification
-            argsList.add("--use-drivers");
-            argsList.add(getBrowserName().toLowerCase());
-        }
+        // get 'localhost' and free port
+        address = GridUtility.getLocalHost();
+        portNum = PortProber.findFreePort();
+        
+        // add 'base-path' argument
+        argsList.add("--base-path");
+        argsList.add("/wd/hub");
+        
+        // add 'nodeconfig' path
+        argsList.add("--nodeconfig");
+        argsList.add(nodeConfigPath.toString());
 
         // specify server port
         argsList.add(0, portNum.toString());
@@ -266,8 +240,6 @@ public LocalGridServer create(SeleniumConfig config, String launcherClassName, S
             process = new CommandLine(findNodeBinary().getAbsolutePath(), argsList.toArray(new String[0]));
         }
 
-        // if target is Selenium 4+, store path to relay configuration in Appium process environment
-        if (config.getVersion() > 3) process.setEnvironmentVariable("nodeConfigPath", nodeConfigPath.toString());
         return new AppiumGridServer(address, portNum, false, process, workingPath, outputPath);
     }
 
@@ -453,17 +425,6 @@ public boolean shutdown(final boolean localOnly) throws InterruptedException {
             return true;
         }
 
-        /**
-         * Get stored relay node configuration path. <br>
-         * <b>NOTE</b>: A relay node is needed to connect the Appium server to a Selenium 4+ Grid hub.
-         * 
-         * @return path to relay node configuration; may be {@code null}
-         */
-        public Path getNodeConfigPath() {
-            String nodeConfigPath = getEnvironment().get("nodeConfigPath");
-            return (nodeConfigPath != null) ? Paths.get(nodeConfigPath) : null;
-        }
-        
         /**
          * Get process environment from this AppiumGridServer object.
          *  
@@ -541,7 +502,5 @@ public static boolean isAppiumWithPM2(URL nodeUrl) {
             }
             return appiumWithPM2;
         }
-        
     }
-    
 }

src/selenium4/java/com/nordstrom/automation/selenium/core/LocalSeleniumGrid.java

@@ -1,8 +1,6 @@
 package com.nordstrom.automation.selenium.core;
 
 import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -16,7 +14,6 @@
 import java.util.ServiceConfigurationError;
 import java.util.concurrent.TimeoutException;
 import org.openqa.selenium.net.PortProber;
-import org.openqa.selenium.os.CommandLine;
 
 import com.nordstrom.automation.selenium.AbstractSeleniumConfig.SeleniumSettings;
 import com.nordstrom.automation.selenium.DriverPlugin;
@@ -39,7 +36,6 @@
  * <b>NOTE</b>: If no test context is specified, the log file will be stored in the "current" directory of the parent
  * Java process.  
  */
-@SuppressWarnings("deprecation")
 public class LocalSeleniumGrid extends SeleniumGrid {
 
     private static final String OPT_HOST = "--host";
@@ -178,7 +174,6 @@ public static SeleniumGrid create(SeleniumConfig config, final Path hubConfigPat
      * @param workingPath {@link Path} of working directory for server process; {@code null} for default
      * @param outputPath {@link Path} to output log file; {@code null} to decline log-to-file
      * @param propertyNames optional array of property names to propagate to server process
-     * 
      * @return {@link LocalGridServer} object for managing the server process
      * @throws GridServerLaunchFailedException If a Grid component process failed to start
      * @see #activate()
@@ -253,18 +248,19 @@ public static LocalGridServer create(final SeleniumConfig config, final String l
             argsList.add(0, "-agentlib:jdwp=transport=dt_socket,server=y,address=" + address);
         }
 
-        String executable = System.getProperty("java.home") + File.separator + "bin" + File.separator + "java";
-        CommandLine process = new CommandLine(executable, argsList.toArray(new String[0]));
-        process.setEnvironmentVariable("PATH", PathUtils.getSystemPath());
-        return new LocalGridServer(hostUrl, portNum, isHub, process, workingPath, outputPath);
+        argsList.add(0, System.getProperty("java.home") + File.separator + "bin" + File.separator + "java");
+        ProcessBuilder builder = new ProcessBuilder(argsList);
+        builder.environment().put("PATH", PathUtils.getSystemPath());
+        return new LocalGridServer(hostUrl, portNum, isHub, builder, workingPath, outputPath);
     }
 
     /**
      * This class represents a single Selenium server (hub or node) belonging to a local Grid collection.
      */
     public static class LocalGridServer extends GridServer {
 
-        private final CommandLine process;
+        private final ProcessBuilder builder;
+        private Process process = null;
         private boolean hasStarted = false;
         private boolean isActive = false;
         private final Map<String, String> personalities = new HashMap<>();
@@ -275,34 +271,39 @@ public static class LocalGridServer extends GridServer {
          * @param host IP address of local Grid server
          * @param port port of local Grid server
          * @param isHub role of Grid server being started ({@code true} = hub; {@code false} = node)
-         * @param process {@link Process} of local Grid server
+         * @param builder {@link ProcessBuilder} of local Grid server
          * @param workingPath {@link Path} of working directory for server process; {@code null} for default
          * @param outputPath {@link Path} to output log file; {@code null} to decline log-to-file
          */
-        public LocalGridServer(String host, Integer port, boolean isHub, CommandLine process, Path workingPath, Path outputPath) {
+        public LocalGridServer(String host, Integer port, boolean isHub, ProcessBuilder builder, Path workingPath, Path outputPath) {
             super(getServerUrl(host, port), isHub);
 
             if (workingPath != null) {
-                process.setWorkingDirectory(workingPath.toString());
+                builder.directory(workingPath.toFile());
             }
 
             if (outputPath != null) {
-                try {
-                    process.copyOutputTo(new FileOutputStream(outputPath.toFile()));
-                } catch (FileNotFoundException e) {
-                    throw new GridServerLaunchFailedException(isHub ? "hub" : "node", e);
-                }
+                builder.redirectOutput(outputPath.toFile());
             }
 
-            this.process = process;
+            this.builder = builder;
+        }
+        
+        /**
+         * Get process environment of this local grid server.
+         *  
+         * @return map of process environment variables
+         */
+        public Map<String, String> getEnvironment() {
+            return builder.environment();
         }
 
         /**
          * Get process for this local Grid server.
          * 
          * @return {@link Process} object
          */
-        public CommandLine getProcess() {
+        public Process getProcess() {
             return process;
         }
 
@@ -327,7 +328,7 @@ public Map<String, String> getPersonalities() {
          */
         public void start() throws IOException, InterruptedException, TimeoutException {
             if (!hasStarted) {
-                process.executeAsync();
+                process = builder.start();
                 hasStarted = true;
             }
         }
@@ -352,6 +353,7 @@ public boolean shutdown(final boolean localOnly) throws InterruptedException {
                 getProcess().destroy();
                 hasStarted = false;
                 isActive = false;
+                getProcess().waitFor();
             }
 
             return true;