Removed ExecutionServiceFactory, using ExecutionService SPI

spacelift-api/src/main/java/org/arquillian/spacelift/ImplementationLoader.java

@@ -16,7 +16,6 @@
  */
 package org.arquillian.spacelift;
 
-import org.arquillian.spacelift.execution.ExecutionServiceFactory;
 
 /**
  * Utility capable of loading interface implementations available on classpath indirectly.
@@ -39,7 +38,7 @@ private ImplementationLoader() {
     }
 
     /**
-     * Creates a new {@link ExecutionServiceFactory} instance of the specified type using the {@link Thread} Context
+     * Creates a new instance of the specified type using the {@link Thread} Context
      * {@link ClassLoader}. Will consult a configuration file visible to the {@link Thread} Context {@link ClassLoader} named
      * "META-INF/services/$fullyQualfiedClassName" which should contain a fully qualified name of the implementation.
      *

spacelift-api/src/main/java/org/arquillian/spacelift/Spacelift.java

@@ -4,7 +4,7 @@
 import java.util.logging.Logger;
 
 import org.arquillian.spacelift.Invokable.InvocationException;
-import org.arquillian.spacelift.execution.ExecutionServiceFactory;
+import org.arquillian.spacelift.execution.ExecutionService;
 import org.arquillian.spacelift.task.InjectTask;
 import org.arquillian.spacelift.task.InvalidTaskException;
 import org.arquillian.spacelift.task.Task;
@@ -61,24 +61,33 @@ public static <IN, OUT, TASK extends Task<? super IN, OUT>> TASK task(IN input,
         return task.passToNext(input).then(alias);
     }
 
+    public static TaskRegistry registry() {
+        return new SpaceliftInstance().registry();
+    }
+
+    public static ExecutionService service() {
+        return new SpaceliftInstance().service();
+    }
+
     /**
      * This class should not be used externally, will be replaced by dependency injection
      * @author kpiwko
      *
      */
     @SuppressWarnings({ "unchecked" })
-    public static class SpaceliftInstance {
+    private static class SpaceliftInstance {
         private static final Logger log = Logger.getLogger(Spacelift.class.getName());
 
-        private static ExecutionServiceFactory lastFactory;
+        private static ExecutionService service;
         private static TaskRegistry registry;
         static {
             try {
-                lastFactory = ImplementationLoader.implementationOf(ExecutionServiceFactory.class);
+                service = ImplementationLoader.implementationOf(ExecutionService.class);
             } catch (InvocationException e) {
+                e.printStackTrace();
                 log.log(Level.SEVERE,
                     "Unable to find default implemenation of {0} on classpath, make sure that you set one programatically.",
-                    ExecutionServiceFactory.class.getName());
+                    ExecutionService.class.getName());
             }
             try {
                 registry = ImplementationLoader.implementationOf(TaskRegistry.class);
@@ -95,8 +104,8 @@ public TaskRegistry registry() {
             return registry;
         }
 
-        public ExecutionServiceFactory service() {
-            return lastFactory;
+        public ExecutionService service() {
+            return service;
         }
     }
 

spacelift-api/src/main/java/org/arquillian/spacelift/task/TaskFactory.java

@@ -32,7 +32,7 @@ public interface TaskFactory<IN, OUT, TASK extends Task<? super IN, OUT>> {
 
     public static class ExecutionServiceInjector {
         <IN, OUT, TASK extends Task<? super IN, OUT>> TASK inject(TASK task) {
-            task.setExecutionService(new Spacelift.SpaceliftInstance().service().getExecutionServiceInstance());
+            task.setExecutionService(Spacelift.service());
             return task;
         }
     }

spacelift-api/src/test/java/org/arquillian/spacelift/task/TestExecutionService.java

@@ -14,7 +14,7 @@
 import org.arquillian.spacelift.execution.ExecutionService;
 import org.arquillian.spacelift.execution.TimeoutExecutionException;
 
-class TestExecutionService implements ExecutionService {
+public class TestExecutionService implements ExecutionService {
 
     @Override
     public <T> Execution<T> execute(final Callable<T> task) throws ExecutionException {

spacelift-api/src/test/java/org/arquillian/spacelift/task/ToolTest.java

@@ -60,7 +60,7 @@ public void getJavaToolFromRegistryByAlias() {
 
     @Test
     public void getJavaToolFromSpaceliftByAlias() {
-        new Spacelift.SpaceliftInstance().registry().register(TestJavaTool.class, "java");
+        Spacelift.registry().register(TestJavaTool.class, "java");
 
         Task<?, ?> task = Spacelift.task("java");
         assertThat(task, notNullValue());

spacelift-api/src/test/resources/META-INF/services/org.arquillian.spacelift.execution.ExecutionService

@@ -0,0 +1,2 @@
+# this class is used only in API tests
+org.arquillian.spacelift.task.TestExecutionService

spacelift-impl/src/main/java/org/arquillian/spacelift/execution/impl/ExecutionServiceImpl.java

@@ -41,9 +41,7 @@ public class ExecutionServiceImpl implements ExecutionService {
     private final ScheduledExecutorService scheduledService;
 
     public ExecutionServiceImpl() {
-
         final ThreadFactory threadFactory = new SpaceliftThreadFactory();
-
         this.service = Executors.newCachedThreadPool(threadFactory);
         this.scheduledService = Executors.newScheduledThreadPool(1, threadFactory);
     }

spacelift-impl/src/main/java/org/arquillian/spacelift/task/os/CommandTool.java

@@ -231,9 +231,14 @@ public CommandTool workingDir(String workingDirectory) throws IllegalArgumentExc
      * @return
      * @throws IllegalArgumentException
      */
-    public CommandTool addEnvironment(Map<String, String> envVariables) throws IllegalArgumentException {
+    public CommandTool addEnvironment(Map<? extends CharSequence, ? extends CharSequence> envVariables) throws IllegalArgumentException {
         Validate.notNull(envVariables, "Environment variables must not be null");
-        this.environment.putAll(envVariables);
+
+        for(Map.Entry<? extends CharSequence, ? extends CharSequence> entry: envVariables.entrySet()) {
+            Validate.notNull(entry.getKey(), "Environment variable name must not be null nor empty");
+            CharSequence value = entry.getValue();
+            environment.put(entry.getKey().toString(), value != null ? value.toString() : null);
+        }
         return this;
     }
 

spacelift-impl/src/main/java/org/arquillian/spacelift/task/os/SpawnProcessTask.java

@@ -57,7 +57,7 @@ public SpawnProcessTask workingDirectory(File workingDirectory) {
     }
 
     public SpawnProcessTask addEnvironment(Map<String, String> environment) {
-        this.environment = environment;
+        this.environment.putAll(environment);
         return this;
     }
 
@@ -101,6 +101,7 @@ protected Process process(Object input) throws Exception {
 
         ProcessBuilder builder = new ProcessBuilder(command.getFullCommand());
         builder.directory(workingDirectory);
+
         builder.environment().putAll(environment);
         builder.redirectErrorStream(redirectErrorStream);
         return builder.start();

spacelift-impl/src/main/resources/META-INF/services/org.arquillian.spacelift.execution.ExecutionService

@@ -0,0 +1 @@
+org.arquillian.spacelift.execution.impl.ExecutionServiceImpl

spacelift-impl/src/test/java/org/arquillian/spacelift/task/os/CommandToolTest.java

@@ -38,10 +38,10 @@ public void runJavaCommandAfterRegistration() {
         // run only on linux
         Assume.assumeThat(SystemUtils.IS_OS_LINUX, is(true));
 
-        new Spacelift.SpaceliftInstance().registry().register(CommandTool.class);
+        Spacelift.registry().register(CommandTool.class);
 
         // previous call caused this task to register
-        // NOTE, this is not really usefull for Java, but rather for dynamic languages
+        // NOTE, this is not really useful for Java, but rather for dynamic languages
         ((CommandTool) Spacelift.task("CommandTool")).programName("java").parameters("-help")
         .execute().await();
     }