This adds support for the EnvironmentOption to pax-exam-container-for…

…ked.

It is a slight variation on how the KarafJavaRunner interprets this option. It allows for:
* Passthrough of an externally defined variable: new EnvironmentOption("HOME")
* Setting the environment variable: new EnvironmentOption("VAR=value")
The latter can of course also be used to redefine an externally existing environment variable.

containers/pax-exam-container-forked/pom.xml

@@ -169,6 +169,21 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <environmentVariables>
+                        <!--
+                            The environment variable 'PROPAGATE' is needed for the following test cases:
+                            ForkedTestContainerFactoryTest.withEnvironmentPassthrough
+                            ForkedTestContainerFactoryTest.withEnvironmentOverride
+                            ForkedTestContainerFactoryTest.withoutEnvironmentPassthrough
+                         -->
+                        <PROPAGATE>test</PROPAGATE>
+                    </environmentVariables>
+                </configuration>
+            </plugin>
         </plugins>
     </build>
 </project>

containers/pax-exam-container-forked/src/main/java/org/ops4j/pax/exam/forked/ForkedFrameworkFactory.java

@@ -103,7 +103,7 @@ public void setFrameworkFactory(FrameworkFactory frameworkFactory) {
      */
     public RemoteFramework fork(List<String> vmArgs, Map<String, String> systemProperties,
         Map<String, Object> frameworkProperties, List<String> beforeFrameworkClasspath,
-        List<String> afterFrameworkClasspath) {
+        List<String> afterFrameworkClasspath, String[] env) {
         // TODO make port range configurable
         FreePort freePort = new FreePort(21000, 21099);
         port = freePort.getPort();
@@ -124,7 +124,7 @@ public RemoteFramework fork(List<String> vmArgs, Map<String, String> systemPrope
             String[] args = buildFrameworkProperties(frameworkProperties);
             javaRunner = new ExamJavaRunner(false);
             javaRunner.exec(vmOptions, buildClasspath(beforeFrameworkClasspath, afterFrameworkClasspath),
-                RemoteFrameworkImpl.class.getName(), args, getJavaHome(), null);
+                RemoteFrameworkImpl.class.getName(), args, getJavaHome(), null, env);
             return findRemoteFramework(port, rmiName);
         }
         catch (RemoteException | ExecutionException | URISyntaxException exc) {
@@ -147,7 +147,7 @@ public RemoteFramework fork(List<String> vmArgs, Map<String, String> systemPrope
      */
     public RemoteFramework fork(List<String> vmArgs, Map<String, String> systemProperties,
         Map<String, Object> frameworkProperties) {
-        return fork(vmArgs, systemProperties, frameworkProperties, null, null);
+        return fork(vmArgs, systemProperties, frameworkProperties, null, null, new String[0]);
     }
 
     private String[] buildSystemProperties(List<String> vmArgs, Map<String, String> systemProperties) {

containers/pax-exam-container-forked/src/main/java/org/ops4j/pax/exam/forked/ForkedTestContainer.java

@@ -55,6 +55,7 @@
 import org.ops4j.pax.exam.options.SystemPropertyOption;
 import org.ops4j.pax.exam.options.UrlReference;
 import org.ops4j.pax.exam.options.ValueOption;
+import org.ops4j.pax.exam.options.extra.EnvironmentOption;
 import org.ops4j.pax.exam.options.extra.RepositoryOption;
 import org.ops4j.pax.exam.options.extra.VMOption;
 import org.ops4j.pax.swissbox.framework.RemoteFramework;
@@ -159,8 +160,15 @@ public TestContainer start() {
                 }
             }
 
+            List<String> environment = new ArrayList<>();
+            EnvironmentOption[] environmentOptions = system.getOptions(EnvironmentOption.class);
+            for (EnvironmentOption environmentOption : environmentOptions) {
+                environment.add(environmentOption.getEnvironment());
+            }
+            String[] env = environment.toArray(new String[environment.size()]);
+
             remoteFramework = frameworkFactory.fork(vmArgs, systemProperties, frameworkProperties,
-                beforeFrameworkClasspath, afterFrameworkClasspath);
+                beforeFrameworkClasspath, afterFrameworkClasspath, env);
             remoteFramework.init();
             installAndStartBundles();
         }

containers/pax-exam-container-forked/src/test/java/org/ops4j/pax/exam/forked/ForkedFrameworkFactoryTest.java

@@ -111,7 +111,7 @@ public void forkWithBootClasspath() throws BundleException, IOException, Interru
             "org.kohsuke.metainf_services");
         RemoteFramework framework = forkedFactory.fork(Collections.<String> emptyList(),
             Collections.<String, String> emptyMap(), frameworkProperties, null,
-            bootClasspath);
+            bootClasspath, new String[0]);
         framework.start();
 
         File testBundle = generateBundle();
@@ -146,7 +146,7 @@ public void forkWithInvalidBootClasspath() throws BundleException, IOException,
             "org.kohsuke.metainf_services");
         forkedFactory.fork(Collections.<String> emptyList(),
             Collections.<String, String> emptyMap(), frameworkProperties, null,
-            bootClasspath);
+            bootClasspath, new String[0]);
     }
 
     private File generateBundle() throws IOException {

containers/pax-exam-container-forked/src/test/java/org/ops4j/pax/exam/forked/ForkedTestContainerFactoryTest.java

@@ -27,6 +27,9 @@
 import java.util.List;
 
 import org.apache.commons.io.FileUtils;
+import org.hamcrest.core.IsEqual;
+import org.hamcrest.core.IsInstanceOf;
+import org.hamcrest.core.StringStartsWith;
 import org.junit.Assert;
 import org.junit.Test;
 import org.ops4j.pax.exam.CoreOptions;
@@ -36,6 +39,7 @@
 import org.ops4j.pax.exam.TestContainerException;
 import org.ops4j.pax.exam.options.MavenArtifactUrlReference;
 import org.ops4j.pax.exam.options.UrlReference;
+import org.ops4j.pax.exam.options.extra.EnvironmentOption;
 import org.ops4j.pax.exam.spi.PaxExamRuntime;
 import org.ops4j.pax.tinybundles.TinyBundles;
 import org.osgi.framework.BundleActivator;
@@ -156,4 +160,127 @@ public void start(BundleContext bc) throws Exception {
         @Override
         public void stop(BundleContext bc) throws Exception {}
     }
+
+    @Test
+    public void withEnvironmentPassthrough() throws IOException {
+        // This test only works if the environment variable 'PROPAGATE' is set externally to 'test'
+        Assert.assertTrue(System.getenv().containsKey("PROPAGATE"));
+        Assert.assertThat(System.getenv().get("PROPAGATE"), IsEqual.equalTo("test"));
+
+        List<Option> options = new ArrayList<>();
+        // Configure the EnvironmentOption to passthrough the 'PROPAGATE' environment variable
+        options.add(new EnvironmentOption("PROPAGATE"));
+        options.add(CoreOptions.frameworkProperty("test.expected.variable").value("PROPAGATE"));
+        options.add(CoreOptions.frameworkProperty("test.expected.value").value("test"));
+
+        forkWithEnvironment(options);
+    }
+
+    @Test
+    public void withEnvironmentOverride() throws IOException {
+        // This test only works if the environment variable 'PROPAGATE' is set externally to 'test'
+        Assert.assertTrue(System.getenv().containsKey("PROPAGATE"));
+        Assert.assertThat(System.getenv().get("PROPAGATE"), IsEqual.equalTo("test"));
+
+        List<Option> options = new ArrayList<>();
+        // Configure the EnvironmentOption to override the value of environment variable PROPAGATE
+        options.add(new EnvironmentOption("PROPAGATE=testOverride"));
+        options.add(CoreOptions.frameworkProperty("test.expected.variable").value("PROPAGATE"));
+        options.add(CoreOptions.frameworkProperty("test.expected.value").value("testOverride"));
+
+        forkWithEnvironment(options);
+    }
+
+    @Test
+    public void withEnvironmentDefinition() throws IOException {
+        List<Option> options = new ArrayList<>();
+        // Configure the EnvironmentOption to define environment variable OTHER
+        options.add(new EnvironmentOption("OTHER=value"));
+        options.add(CoreOptions.frameworkProperty("test.expected.variable").value("OTHER"));
+        options.add(CoreOptions.frameworkProperty("test.expected.value").value("value"));
+
+        forkWithEnvironment(options);
+    }
+
+    @Test
+    public void withoutEnvironmentPassthrough() throws IOException {
+        // This test only works if the environment variable 'PROPAGATE' is set externally to 'test'
+        Assert.assertTrue(System.getenv().containsKey("PROPAGATE"));
+        Assert.assertThat(System.getenv().get("PROPAGATE"), IsEqual.equalTo("test"));
+
+        List<Option> options = new ArrayList<>();
+        // Do not configure a value or pass through of environment variable PROPAGATE
+        options.add(CoreOptions.frameworkProperty("test.expected.variable").value("PROPAGATE"));
+        options.add(CoreOptions.frameworkProperty("test.expected.value").value("test"));
+
+        // The fork should fail as the expected environment variable is not found
+        TestContainerException testContainerException = Assert.assertThrows(TestContainerException.class, () -> forkWithEnvironment(options));
+
+        Throwable bundleException = testContainerException.getCause();
+        Assert.assertThat(bundleException, IsInstanceOf.instanceOf(BundleException.class));
+
+        Throwable illegalStateException = bundleException.getCause();
+        Assert.assertThat(illegalStateException, IsInstanceOf.instanceOf(IllegalStateException.class));
+
+        // Verify the reason of the failure
+        Assert.assertThat(illegalStateException.getMessage(), StringStartsWith.startsWith("Unable to find environment variable 'PROPAGATE' with expected value 'test'"));
+    }
+
+    public void forkWithEnvironment(List<Option> options) throws IOException {
+        Option[] opts = options.toArray(new Option[options.size()]);
+
+        ExamSystem system = PaxExamRuntime.createServerSystem(opts);
+        ForkedTestContainerFactory factory = new ForkedTestContainerFactory();
+        TestContainer[] containers = factory.create(system);
+
+        Assert.assertNotNull(containers);
+        Assert.assertNotNull(containers[0]);
+
+        ForkedTestContainer container = (ForkedTestContainer) containers[0];
+        container.start();
+        try {
+            // Prepare a test bundle with EnvironmentTestActivator as activator
+            File testBundle = generateEnvironmentBundle();
+
+            // Install the test bundle, the start on EnvironmentTestActivator will fail if the configured environment
+            // variable does not have the expected value
+            container.install(new FileInputStream(testBundle));
+        } finally {
+            container.stop();
+        }
+    }
+
+    private File generateEnvironmentBundle() throws IOException {
+        InputStream stream = TinyBundles.bundle().addClass(EnvironmentTestActivator.class)
+                .setHeader(Constants.BUNDLE_MANIFESTVERSION, "2")
+                .setHeader(Constants.BUNDLE_SYMBOLICNAME, "environment.test.generated")
+                .setHeader(Constants.BUNDLE_ACTIVATOR, EnvironmentTestActivator.class.getName())
+                .setHeader(Constants.IMPORT_PACKAGE, "org.osgi.framework")
+                .build(rawBuilder());
+
+        File bundle = new File("target/bundles/environment-generated.jar");
+        FileUtils.copyInputStreamToFile(stream, bundle);
+        return bundle;
+    }
+
+    public static class EnvironmentTestActivator implements BundleActivator {
+
+        @Override
+        public void start(BundleContext bc) throws Exception {
+            final String variable = bc.getProperty("test.expected.variable");
+            final String expectedValue = bc.getProperty("test.expected.value");
+            if (variable == null || expectedValue == null) {
+                throw new IllegalStateException("Unable to retrieve value for framework property " +
+                        "'test.expected.variable' and/or 'test.expected.value'");
+            }
+            if (!System.getenv().containsKey(variable) || !expectedValue.equals(System.getenv().get(variable))) {
+                throw new IllegalStateException(
+                        "Unable to find environment variable '" + variable +
+                                "' with expected value '" + expectedValue + "' in: " + System.getenv());
+            }
+        }
+
+        @Override
+        public void stop(BundleContext bc) throws Exception {}
+    }
 }

core/pax-exam/src/main/java/org/ops4j/pax/exam/ExamJavaRunner.java

@@ -20,8 +20,10 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.ops4j.exec.CommandLineBuilder;
 import org.ops4j.exec.ExecutionException;
@@ -134,13 +136,59 @@ public synchronized void exec(final String[] vmOptions,
     }
 
     private void setEnv(final String[] envOptions, ProcessBuilder pb) {
-        HashSet<String> envSet = new HashSet<String>(Arrays.asList(envOptions));
-        Map<String, String> env = pb.environment();
-        HashSet<String> toRemove = new HashSet<String>(env.keySet());
-        toRemove.removeAll(envSet);
-        for (String key : toRemove) {
-            env.remove(key);
+        final Map<String, String> envMap = toKeyValueMap(envOptions);
+
+        final Map<String, String> env = pb.environment();
+
+        // Remove all items from the ProcessBuilder's environment,
+        // that are not explicitly referenced in the envMap's keyset
+        final Set<String> toRemove = new HashSet<String>(env.keySet());
+        toRemove.removeAll(envMap.keySet());
+        env.keySet().removeAll(toRemove);
+
+        // Any supplied values should be applied, possibly overriding an initial value from the ProcessBuilder.
+        envMap.entrySet().stream()
+                .filter(entry -> entry.getValue() != null)
+                .forEach(entry -> env.put(entry.getKey(), entry.getValue()));
+    }
+
+    /**
+     * Converts an array of strings in the format "key=value" into a map of key-value pairs.
+     * If a string does not contain an '=' character, the entire string is treated as the key,
+     * and the corresponding value is set to {@code null}.
+     *
+     * @param envOptions an array of strings, each representing an environment option in the
+     *                   format "key=value". Strings without an '=' will have {@code null} as the value.
+     * @return a {@code Map<String, String>} where each key-value pair is derived from the input array.
+     *         Keys are extracted from the portion of the string before the first '=' character,
+     *         and values are extracted from the portion after the '=' character. If no '=' is
+     *         present, the value is set to {@code null}.
+     */
+    private static Map<String, String> toKeyValueMap(String[] envOptions) {
+        final Map<String, String> envMap = new HashMap<>();
+        for (String envOption : envOptions) {
+            if (envOption == null || envOption.isEmpty()) {
+                throw new IllegalArgumentException("Null or empty entry in envOptions");
+            }
+            final int equalsPosition = envOption.indexOf('=');
+            final String key;
+            final String value;
+            if (equalsPosition < 0) {
+                key = envOption.trim();
+                value = null;
+            } else {
+                key = envOption.substring(0, equalsPosition).trim();
+                value = envOption.substring(equalsPosition + 1).trim();
+            }
+            if (key.isEmpty()) {
+                throw new IllegalArgumentException("Input " + envOption + " resulted in an empty key");
+            }
+            if (envMap.containsKey(key)) {
+                throw new IllegalArgumentException("Duplicate key found: " + key);
+            }
+            envMap.put(key, value);
         }
+        return envMap;
     }
 
     /**