changeset 1659:2418a81fcd27

Integration tests fail if mongod is not found in std PATH. Reviewed-by: omajid, neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-March/013020.html PR2270
author Severin Gehwolf <sgehwolf@redhat.com>
date Tue, 10 Mar 2015 09:44:47 +0100
parents 3e61adfe186a
children c6ae78b6f3ac
files integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/EnvironmentExecutor.java integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/EnvironmentExecutorTest.java integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/LocaleExecutor.java integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/PropertiesExecutor.java integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/PropertiesExecutorTest.java integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/SimpleExecutor.java
diffstat 7 files changed, 179 insertions(+), 69 deletions(-) [+]
line wrap: on
line diff
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/EnvironmentExecutor.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/EnvironmentExecutor.java	Tue Mar 10 09:44:47 2015 +0100
@@ -37,6 +37,11 @@
 package com.redhat.thermostat.itest;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import expectj.Executor;
 
@@ -47,40 +52,84 @@
  */
 class EnvironmentExecutor implements Executor {
 
-    private final String[] env;
-    private final String args;
+    private final Map<String, String> env;
+    private final String[] args;
     private final String script;
+    private final String binRoot;
 
     /**
      * 
      * @param script The script name (e.g. "thermostat")
-     * @param args The space separated list of arguments
-     * @param env List of environment variables in key=value pair format.
+     * @param args The arguments and options to the script
+     * @param env The environment variables in key value format
      */
-    public EnvironmentExecutor(String script, String args, String[] env) {
+    public EnvironmentExecutor(String script, String[] args, Map<String, String> env) {
+        this(IntegrationTest.getSystemBinRoot(), script, args, env);
+    }
+    
+    // Enable testing without invoking static initializer of IntegrationTest
+    EnvironmentExecutor(String binRoot, String script, String[] args, Map<String, String> env) {
         this.args = args;
         this.env = env;
         this.script = script;
+        this.binRoot = binRoot;
     }
 
     @Override
     public Process execute() throws IOException {
-        String command = buildCommand();
-        Process p = Runtime.getRuntime().exec(command, env);
-        return p;
+        List<String> commands = buildCommmands();
+        ProcessBuilder builder = new ProcessBuilder(commands);
+        updateEnvironment(getBuilderEnvironment(builder));
+        return startProcess(builder);
     }
 
-    @Override
-    public String toString() {
-        return script + " " + args;
+    List<String> buildCommmands() {
+        List<String> commands = new ArrayList<>(1 + args.length);
+        String command = buildScriptPath();
+        commands.add(command);
+        commands.addAll(Arrays.asList(args));
+        return commands;
     }
-    
-    private String buildCommand() {
-        return IntegrationTest.getSystemBinRoot() + "/" + script + " " + args;
+
+    private String buildScriptPath() {
+        return binRoot + "/" + script;
+    }
+
+    // for testing
+    protected void updateEnvironment(Map<String, String> toUpdate) {
+        for (Entry<String, String> entry : env.entrySet()) {
+            toUpdate.put(entry.getKey(), entry.getValue());
+        }
     }
     
     // for testing
-    String[] getEnv() {
+    protected Map<String, String> getBuilderEnvironment(ProcessBuilder builder) {
+        return builder.environment();
+    }
+    
+    // for testing
+    protected Process startProcess(ProcessBuilder builder) throws IOException {
+        return builder.start();
+    }
+    
+    
+    // for testing
+    Map<String, String> getEnv() {
         return env;
     }
+    
+    @Override
+    public String toString() {
+        return script + convertArgsToString(args);
+    }
+    
+    private static String convertArgsToString(String[] args) {
+        StringBuilder result = new StringBuilder();
+        if (args != null) {
+            for (String arg : args) {
+                result.append(" ").append(arg);
+            }
+        }
+        return result.toString();
+    }
 }
\ No newline at end of file
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/EnvironmentExecutorTest.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/EnvironmentExecutorTest.java	Tue Mar 10 09:44:47 2015 +0100
@@ -37,17 +37,87 @@
 package com.redhat.thermostat.itest;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.junit.Test;
 
 public class EnvironmentExecutorTest {
     
+    private static final String FAKE_BIN_ROOT = "";
+    private static final String[] ARGS = new String[] {
+        "bar", "baz"
+    };
+    private static final String SCRIPT = "thermostat";
+    
     @Test
     public void testExecutor() {
-        String[] env = new String[] { "env1=bar1" };
-        EnvironmentExecutor executor = new EnvironmentExecutor("thermostat", "bar baz", env);
+        Map<String, String> env = new HashMap<>();
+        env.put("env1", "bar1");
+        EnvironmentExecutor executor = new EnvironmentExecutor(FAKE_BIN_ROOT, SCRIPT, ARGS, env);
         assertEquals("thermostat bar baz", executor.toString());
-        assertEquals(1, executor.getEnv().length);
-        assertEquals("env1=bar1", executor.getEnv()[0]);
+        assertEquals(1, executor.getEnv().keySet().size());
+        assertEquals("bar1", executor.getEnv().get("env1"));
+    }
+    
+    @Test
+    public void executeUpdatesEnvironment() throws IOException {
+        String sharedEnvVar = "override-me-on-execute";
+        Map<String, String> overrideEnv = new HashMap<>();
+        final Map<String, String> processBuilderDefaultEnv = new HashMap<>();
+        
+        processBuilderDefaultEnv.put(sharedEnvVar, "old-value");
+        processBuilderDefaultEnv.put("LANG", "C");
+        overrideEnv.put(sharedEnvVar, "new-value");
+        
+        // Precondition
+        assertTrue("expected processbuilder env *and* override env to contain " 
+                   + sharedEnvVar,
+                       overrideEnv.containsKey(sharedEnvVar) &&
+                       processBuilderDefaultEnv.containsKey(sharedEnvVar));
+        
+        TestEnvironmentExecutor testExecutor = new TestEnvironmentExecutor(FAKE_BIN_ROOT, SCRIPT, ARGS, overrideEnv) {
+            
+            @Override
+            protected Map<String, String> getBuilderEnvironment(ProcessBuilder builder) {
+                return processBuilderDefaultEnv;
+            }
+        };
+        
+        // This should update the environment
+        testExecutor.execute();
+        assertTrue(testExecutor.executeCalled);
+        
+        Map<String, String> actualEnv = testExecutor.updatedEnvironment;
+        assertEquals("env var " + sharedEnvVar + " should have been overridden",
+                "new-value", actualEnv.get(sharedEnvVar));
+        assertEquals("env var we didn't override should stay untouched",
+                "C", actualEnv.get("LANG"));
+    }
+    
+    private static class TestEnvironmentExecutor extends EnvironmentExecutor {
+        
+        TestEnvironmentExecutor(String binRoot, String script, String[] args,
+                Map<String, String> env) {
+            super(binRoot, script, args, env);
+        }
+
+        private Map<String, String> updatedEnvironment;
+        private boolean executeCalled;
+        
+        @Override
+        protected Process startProcess(ProcessBuilder builder) {
+            executeCalled = true;
+            return null;
+        }
+        
+        @Override
+        protected void updateEnvironment(Map<String, String> toUpdate) {
+            super.updateEnvironment(toUpdate);
+            updatedEnvironment = toUpdate;
+        }
     }
 }
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java	Tue Mar 10 09:44:47 2015 +0100
@@ -67,6 +67,16 @@
  */
 public class IntegrationTest {
     
+    public static final String ITEST_USER_HOME_PROP = "com.redhat.thermostat.itest.thermostatUserHome";
+    public static final String ITEST_THERMOSTAT_HOME_PROP = "com.redhat.thermostat.itest.thermostatHome";
+    
+    private static final String AGENT_VERBOSE_MODE_PROP = "thermostat.agent.verbose";
+    private static final String THERMOSTAT_HOME = "THERMOSTAT_HOME";
+    private static final String USER_THERMOSTAT_HOME = "USER_THERMOSTAT_HOME";
+    
+    public static final Map<String, String> DEFAULT_ENVIRONMENT;
+    public static final Map<String, String> DEFAULT_ENV_WITH_LANG_C;
+    
     /**
      * Configure the log level to FINEST, and configure a file handler so as for
      * log messages to go to USER_THERMOSTAT_HOME/integration-tests.log rather
@@ -80,26 +90,15 @@
         File logFile = new File(getUserThermostatHome() + File.separator + "integration-tests.log");
         LogConfigurator configurator = new LogConfigurator(Level.FINEST, loggingProperties, logFile);
         configurator.writeConfiguration();
+        
+        // Set up environment maps.
+        DEFAULT_ENVIRONMENT = new HashMap<>();
+        DEFAULT_ENVIRONMENT.put(THERMOSTAT_HOME, getThermostatHome());
+        DEFAULT_ENVIRONMENT.put(USER_THERMOSTAT_HOME, getUserThermostatHome());
+        DEFAULT_ENV_WITH_LANG_C = new HashMap<>(DEFAULT_ENVIRONMENT);
+        DEFAULT_ENV_WITH_LANG_C.put("LANG", "C");
     }
     
-    public static final String ITEST_USER_HOME_PROP = "com.redhat.thermostat.itest.thermostatUserHome";
-    public static final String ITEST_THERMOSTAT_HOME_PROP = "com.redhat.thermostat.itest.thermostatHome";
-
-    private static final String AGENT_VERBOSE_MODE_PROP = "thermostat.agent.verbose";
-    private static final String THERMOSTAT_HOME = "THERMOSTAT_HOME";
-    private static final String USER_THERMOSTAT_HOME = "USER_THERMOSTAT_HOME";
-
-    public static final String[] DEFAULT_ENVIRONMENT = new String[] {
-        THERMOSTAT_HOME + "=" + IntegrationTest.getThermostatHome(),
-        USER_THERMOSTAT_HOME + "=" + IntegrationTest.getUserThermostatHome(),
-    };
-    
-    public static final String[] DEFAULT_ENV_WITH_LANG_C = new String [] {
-        THERMOSTAT_HOME + "=" + IntegrationTest.getThermostatHome(),
-        USER_THERMOSTAT_HOME + "=" + IntegrationTest.getUserThermostatHome(),
-        "LANG=C"
-    };
-    
     public static class SpawnResult {
         final Process process;
         final Spawn spawn;
@@ -289,25 +288,14 @@
     
     private static Spawn runScript(boolean localeDependent, String script, String[] args) throws IOException {
         ExpectJ expect = new ExpectJ(TIMEOUT_IN_SECONDS);
-        String toExecute = convertArgsToString(args);
         Executor exec = null;
         if (localeDependent) {
-            exec = new LocaleExecutor(script, toExecute);
+            exec = new LocaleExecutor(script, args);
         } else {
-            exec = new SimpleExecutor(script, toExecute);
+            exec = new SimpleExecutor(script, args);
         }
         return expect.spawn(exec);
     }
-    
-    private static String convertArgsToString(String[] args) {
-    	StringBuilder result = new StringBuilder();
-        if (args != null) {
-            for (String arg : args) {
-                result.append(" ").append(arg);
-            }
-        }
-        return result.toString();
-    }
 
     public static SpawnResult spawnThermostatAndGetProcess(String... args)
             throws IOException {
@@ -326,13 +314,11 @@
     }
 
     private static SpawnResult runCommandAndGetProcess(String script, String[] args, Map<String, String> props) throws IOException {
-        String toExecute = convertArgsToString(args);
-
         final Process[] process = new Process[1];
 
         ExpectJ expect = new ExpectJ(TIMEOUT_IN_SECONDS);
 
-        Spawn spawn = expect.spawn(new PropertiesExecutor(script, toExecute,
+        Spawn spawn = expect.spawn(new PropertiesExecutor(script, args,
                 props) {
             @Override
             public Process execute() throws IOException {
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/LocaleExecutor.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/LocaleExecutor.java	Tue Mar 10 09:44:47 2015 +0100
@@ -38,7 +38,7 @@
 
 class LocaleExecutor extends EnvironmentExecutor {
 
-    public LocaleExecutor(String script, String args) {
+    public LocaleExecutor(String script, String[] args) {
         super(script, args, IntegrationTest.DEFAULT_ENV_WITH_LANG_C);
     }
 
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/PropertiesExecutor.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/PropertiesExecutor.java	Tue Mar 10 09:44:47 2015 +0100
@@ -36,6 +36,8 @@
 
 package com.redhat.thermostat.itest;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 
 class PropertiesExecutor extends EnvironmentExecutor {
@@ -43,25 +45,25 @@
     private static final String ARG_TEMPLATE = "-J-D%s=%s";
     
     // main constructor
-    public PropertiesExecutor(String script, String args, Map<String, String> props) {
+    public PropertiesExecutor(String script, String []args, Map<String, String> props) {
         super(script, prependProperties(args, props), IntegrationTest.DEFAULT_ENVIRONMENT);
     }
     
     // Test-only
-    PropertiesExecutor(String script, String args, Map<String, String> props, String[] env) {
-        super(script, prependProperties(args, props), env);
+    PropertiesExecutor(String binRoot, String script, String[] args, Map<String, String> props, Map<String, String> env) {
+        super(binRoot, script, prependProperties(args, props), env);
     }
     
-    static private String prependProperties(String args, Map<String, String> props) {
-        String retval = "";
+    static private String[] prependProperties(String[] args, Map<String, String> props) {
+        List<String> newArgs = new ArrayList<>();
+        // prepend property args
         for (String key: props.keySet()) {
-            retval = retval + String.format(ARG_TEMPLATE, key, props.get(key)) + " ";
+            newArgs.add(String.format(ARG_TEMPLATE, key, props.get(key)));
         }
-        retval = retval.trim();
-        if (retval.length() == 0) {
-            return args;
-        } else {
-            return retval + " " + args;
+        // Append existing args.
+        for (String arg: args) {
+            newArgs.add(arg);
         }
+        return newArgs.toArray(new String[]{});
     }
 }
\ No newline at end of file
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/PropertiesExecutorTest.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/PropertiesExecutorTest.java	Tue Mar 10 09:44:47 2015 +0100
@@ -39,6 +39,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -46,6 +47,8 @@
 
 public class PropertiesExecutorTest {
     
+    private static final String FAKE_BIN_ROOT = "";
+    
     @Test
     public void testExecutorEmptyProps() {
         Map<String, String> props = new HashMap<>();
@@ -68,9 +71,9 @@
     }
     
     private PropertiesExecutor getPropertiesExecutor(Map<String, String> props) {
-        String[] emptyEnv = new String[] {};
-        PropertiesExecutor executor = new PropertiesExecutor("thermostat", "bar baz", props, emptyEnv);
-        assertEquals(0, executor.getEnv().length);
+        String[] args = new String[] { "bar",  "baz" };
+        PropertiesExecutor executor = new PropertiesExecutor(FAKE_BIN_ROOT, "thermostat", args, props, Collections.<String, String>emptyMap());
+        assertEquals(0, executor.getEnv().keySet().size());
         return executor;
     }
 }
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/SimpleExecutor.java	Tue Mar 03 16:32:21 2015 +0100
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/SimpleExecutor.java	Tue Mar 10 09:44:47 2015 +0100
@@ -38,7 +38,7 @@
 
 class SimpleExecutor extends EnvironmentExecutor {
 
-    public SimpleExecutor(String script, String args) {
+    public SimpleExecutor(String script, String[] args) {
         super(script, args, IntegrationTest.DEFAULT_ENVIRONMENT);
     }