Polish CLI tester code

Reduce duplication by extracting FileOptions class to be shared by
both the RunCommand and the TestCommand.

Also applied minor code convention tweaks.

spring-boot-cli/src/main/groovy/testers/junit.groovy

@@ -22,6 +22,7 @@ import org.springframework.boot.cli.command.tester.TestResults
 import java.lang.annotation.Annotation
 import java.lang.reflect.Method
 
+
 /**
  * Groovy script to run JUnit tests inside the {@link TestCommand}.
  * Needs to be compiled along with the actual code to work properly.

spring-boot-cli/src/main/groovy/testers/tester.groovy

@@ -16,6 +16,7 @@
 
 import org.springframework.boot.cli.command.tester.TestResults
 
+
 /**
  * Groovy script define abstract basis for automated testers for {@link TestCommand}.
  * Needs to be compiled along with the actual code to work properly.
@@ -28,14 +29,14 @@ public abstract class AbstractTester {
         Set<Class<?>> testable = findTestableClasses(compiled)
 
         if (testable.size() == 0) {
-            return TestResults.none
+            return TestResults.NONE
         }
 
         return test(testable.toArray(new Class<?>[0]))
     }
 
-    abstract protected Set<Class<?>> findTestableClasses(List<Class<?>> compiled)
+    protected abstract Set<Class<?>> findTestableClasses(List<Class<?>> compiled)
 
-    abstract protected TestResults test(Class<?>[] testable)
+    protected abstract TestResults test(Class<?>[] testable)
 
 }

spring-boot-cli/src/main/java/org/springframework/boot/cli/command/FileOptions.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright 2012-2013 the original author or authors.
+ *
+ * Licensed 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.springframework.boot.cli.command;
+
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import joptsimple.OptionSet;
+
+/**
+ * Extract file options (anything following '--' in an {@link OptionSet}).
+ * 
+ * @author Phillip Webb
+ * @author Dave Syer
+ * @author Greg Turnquist
+ */
+public class FileOptions {
+
+	private List<File> files;
+
+	private List<?> args;
+
+	/**
+	 * Create a new {@link FileOptions} instance.
+	 * @param options the source option set
+	 */
+	public FileOptions(OptionSet options) {
+		this(options, null);
+	}
+
+	/**
+	 * Create a new {@link FileOptions} instance.
+	 * @param optionSet the source option set
+	 * @param classLoader an optional classloader used to try and load files that are not
+	 * found directly.
+	 */
+	public FileOptions(OptionSet optionSet, ClassLoader classLoader) {
+		List<?> nonOptionArguments = optionSet.nonOptionArguments();
+		List<File> files = new ArrayList<File>();
+		for (Object option : nonOptionArguments) {
+			if (option instanceof String) {
+				String filename = (String) option;
+				if ("--".equals(filename)) {
+					break;
+				}
+				if (filename.endsWith(".groovy") || filename.endsWith(".java")) {
+					File file = getFile(filename, classLoader);
+					if (file == null) {
+						throw new RuntimeException("Can't find " + filename);
+					}
+					files.add(file);
+				}
+			}
+		}
+		if (files.size() == 0) {
+			throw new RuntimeException("Please specify a file to run");
+		}
+		this.files = Collections.unmodifiableList(files);
+		this.args = Collections.unmodifiableList(nonOptionArguments.subList(files.size(),
+				nonOptionArguments.size()));
+	}
+
+	private File getFile(String filename, ClassLoader classLoader) {
+		File file = new File(filename);
+		if (file.isFile() && file.canRead()) {
+			return file;
+		}
+		if (classLoader != null) {
+			URL url = classLoader.getResource(filename);
+			if (url != null && url.toString().startsWith("file:")) {
+				return new File(url.toString().substring("file:".length()));
+			}
+		}
+		return null;
+	}
+
+	public List<?> getArgs() {
+		return this.args;
+	}
+
+	public String[] getArgsArray() {
+		return this.args.toArray(new String[this.args.size()]);
+	}
+
+	public List<File> getFiles() {
+		return this.files;
+	}
+
+	public File[] getFilesArray() {
+		return this.files.toArray(new File[this.files.size()]);
+	}
+
+}

spring-boot-cli/src/main/java/org/springframework/boot/cli/command/RunCommand.java

@@ -17,9 +17,6 @@
 package org.springframework.boot.cli.command;
 
 import java.awt.Desktop;
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.logging.Level;
 
 import joptsimple.OptionSet;
@@ -94,13 +91,10 @@ public class RunCommand extends OptionParsingCommand {
 
 		@Override
 		protected void run(OptionSet options) throws Exception {
-			List<?> nonOptionArguments = options.nonOptionArguments();
-			File[] files = getFileArguments(nonOptionArguments);
-			List<?> args = nonOptionArguments.subList(files.length,
-					nonOptionArguments.size());
+			FileOptions fileOptions = new FileOptions(options);
 
 			if (options.has(this.editOption)) {
-				Desktop.getDesktop().edit(files[0]);
+				Desktop.getDesktop().edit(fileOptions.getFiles().get(0));
 			}
 
 			SpringApplicationRunnerConfiguration configuration = new SpringApplicationRunnerConfigurationAdapter(
@@ -108,33 +102,11 @@ public class RunCommand extends OptionParsingCommand {
 			if (configuration.isLocal() && System.getProperty("grape.root") == null) {
 				System.setProperty("grape.root", ".");
 			}
-			this.runner = new SpringApplicationRunner(configuration, files,
-					args.toArray(new String[args.size()]));
+			this.runner = new SpringApplicationRunner(configuration,
+					fileOptions.getFilesArray(), fileOptions.getArgsArray());
 			this.runner.compileAndRun();
 		}
 
-		private File[] getFileArguments(List<?> nonOptionArguments) {
-			List<File> files = new ArrayList<File>();
-			for (Object option : nonOptionArguments) {
-				if (option instanceof String) {
-					String filename = (String) option;
-					if ("--".equals(filename)) {
-						break;
-					}
-					if (filename.endsWith(".groovy") || filename.endsWith(".java")) {
-						File file = new File(filename);
-						if (file.isFile() && file.canRead()) {
-							files.add(file);
-						}
-					}
-				}
-			}
-			if (files.size() == 0) {
-				throw new RuntimeException("Please specify a file to run");
-			}
-			return files.toArray(new File[files.size()]);
-		}
-
 		/**
 		 * Simple adapter class to present the {@link OptionSet} as a
 		 * {@link SpringApplicationRunnerConfiguration}.

spring-boot-cli/src/main/java/org/springframework/boot/cli/command/TestCommand.java

@@ -21,10 +21,10 @@ import groovy.lang.GroovyObject;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.PrintWriter;
-import java.net.URL;
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.logging.Level;
@@ -32,6 +32,7 @@ import java.util.logging.Level;
 import joptsimple.OptionSet;
 
 import org.apache.ivy.util.FileUtil;
+import org.codehaus.groovy.control.CompilationFailedException;
 import org.springframework.boot.cli.Log;
 import org.springframework.boot.cli.command.tester.Failure;
 import org.springframework.boot.cli.command.tester.TestResults;
@@ -39,7 +40,7 @@ import org.springframework.boot.cli.compiler.GroovyCompiler;
 import org.springframework.boot.cli.compiler.GroovyCompilerConfiguration;
 
 /**
- * Invokes testing for autocompiled scripts
+ * Invokes testing for auto-compiled scripts
  * 
  * @author Greg Turnquist
  */
@@ -63,6 +64,7 @@ public class TestCommand extends OptionParsingCommand {
 
 	private static class TestGroovyCompilerConfiguration implements
 			GroovyCompilerConfiguration {
+
 		@Override
 		public boolean isGuessImports() {
 			return true;
@@ -99,10 +101,8 @@ public class TestCommand extends OptionParsingCommand {
 
 		@Override
 		protected void run(OptionSet options) throws Exception {
-			List<?> nonOptionArguments = options.nonOptionArguments();
-
-			Set<File> testerFiles = new HashSet<File>();
-			File[] files = getFileArguments(nonOptionArguments, testerFiles);
+			FileOptions fileOptions = new FileOptions(options, getClass()
+					.getClassLoader());
 
 			/*
 			 * Need to compile the code twice: The first time automatically pulls in
@@ -112,44 +112,24 @@ public class TestCommand extends OptionParsingCommand {
 			 * context. Then the testers can be fetched and invoked through reflection
 			 * against the composite AST.
 			 */
-			// Compile - Pass 1
-			Object[] sources = this.compiler.sources(files);
-
-			boolean testing = false;
-			try {
-				check("org.junit.Test", sources);
-				testerFiles.add(locateSourceFromUrl("junit", "testers/junit.groovy"));
-				testing = true;
-			}
-			catch (ClassNotFoundException e) {
-			}
 
-			try {
-				check("spock.lang.Specification", sources);
-				testerFiles.add(locateSourceFromUrl("spock", "testers/spock.groovy"));
-				testing = true;
-			}
-			catch (ClassNotFoundException e) {
-			}
-
-			if (testing) {
-				testerFiles.add(locateSourceFromUrl("tester", "testers/tester.groovy"));
-			}
+			// Compile - Pass 1 - collect testers
+			Object[] sources = this.compiler.sources(fileOptions.getFilesArray());
+			Set<File> testerFiles = compileAndCollectTesterFiles(sources);
 
 			// Compile - Pass 2 - with appropriate testers added in
-			files = getFileArguments(nonOptionArguments, testerFiles);
-			sources = this.compiler.sources(files);
-
+			List<File> files = new ArrayList<File>(fileOptions.getFiles());
+			files.addAll(testerFiles);
+			sources = this.compiler.sources(files.toArray(new File[files.size()]));
 			if (sources.length == 0) {
 				throw new RuntimeException("No classes found in '" + files + "'");
 			}
 
-			List<Class<?>> testers = new ArrayList<Class<?>>();
-
 			// Extract list of compiled classes
 			List<Class<?>> compiled = new ArrayList<Class<?>>();
+			List<Class<?>> testers = new ArrayList<Class<?>>();
 			for (Object source : sources) {
-				if (source.getClass() == Class.class) {
+				if (source instanceof Class) {
 					Class<?> sourceClass = (Class<?>) source;
 					if (sourceClass.getSuperclass().getName().equals("AbstractTester")) {
 						testers.add(sourceClass);
@@ -161,85 +141,53 @@ public class TestCommand extends OptionParsingCommand {
 			}
 
 			this.results = new TestResults();
-
 			for (Class<?> tester : testers) {
 				GroovyObject obj = (GroovyObject) tester.newInstance();
 				this.results.add((TestResults) obj.invokeMethod("findAndTest", compiled));
 			}
-
 			printReport(this.results);
 		}
 
-		private File locateSourceFromUrl(String name, String path) {
-			try {
-				File file = File.createTempFile(name, ".groovy");
-				file.deleteOnExit();
-				FileUtil.copy(getClass().getClassLoader().getResourceAsStream(path),
-						file, null);
-				return file;
+		private Set<File> compileAndCollectTesterFiles(Object[] sources)
+				throws CompilationFailedException, IOException {
+			Set<File> testerFiles = new LinkedHashSet<File>();
+			addTesterOnClass(sources, "org.junit.Test", "junit", testerFiles);
+			addTesterOnClass(sources, "spock.lang.Specification", "spock", testerFiles);
+			if (!testerFiles.isEmpty()) {
+				testerFiles.add(createTempTesterFile("tester"));
 			}
-			catch (IOException ex) {
-				throw new IllegalStateException("Could not create temp file for source: "
-						+ name);
-			}
-		}
 
-		private Class<?> check(String className, Object[] sources)
-				throws ClassNotFoundException {
-			Class<?> classToReturn = null;
-			ClassNotFoundException classNotFoundException = null;
-			for (Object source : sources) {
-				try {
-					classToReturn = ((Class<?>) source).getClassLoader().loadClass(
-							className);
-				}
-				catch (ClassNotFoundException e) {
-					classNotFoundException = e;
-				}
-			}
-			if (classToReturn != null) {
-				return classToReturn;
-			}
-			throw classNotFoundException;
+			return testerFiles;
 		}
 
-		private File[] getFileArguments(List<?> nonOptionArguments, Set<File> testerFiles) {
-			List<File> files = new ArrayList<File>();
-			for (Object option : nonOptionArguments) {
-				if (option instanceof String) {
-					String filename = (String) option;
-					if ("--".equals(filename)) {
-						break;
+		private void addTesterOnClass(Object[] sources, String className,
+				String testerName, Set<File> testerFiles) {
+			for (Object source : sources) {
+				if (source instanceof Class<?>) {
+					try {
+						((Class<?>) source).getClassLoader().loadClass(className);
+						testerFiles.add(createTempTesterFile(testerName));
+						return;
 					}
-					if (filename.endsWith(".groovy") || filename.endsWith(".java")) {
-						File file = new File(filename);
-						if (file.isFile() && file.canRead()) {
-							files.add(file);
-						}
-						else {
-							URL url = getClass().getClassLoader().getResource(filename);
-							if (url != null) {
-								if (url.toString().startsWith("file:")) {
-									files.add(new File(url.toString().substring(
-											"file:".length())));
-								}
-							}
-							else {
-								throw new RuntimeException("Can't find " + filename);
-							}
-						}
+					catch (ClassNotFoundException ex) {
 					}
 				}
 			}
-			if (files.size() == 0) {
-				throw new RuntimeException("Please specify a file to run");
-			}
+		}
 
-			for (File testerFile : testerFiles) {
-				files.add(testerFile);
+		private File createTempTesterFile(String name) {
+			try {
+				File file = File.createTempFile(name, ".groovy");
+				file.deleteOnExit();
+				InputStream resource = getClass().getClassLoader().getResourceAsStream(
+						"testers/" + name + ".groovy");
+				FileUtil.copy(resource, file, null);
+				return file;
+			}
+			catch (IOException ex) {
+				throw new IllegalStateException("Could not create temp file for source: "
+						+ name);
 			}
-
-			return files.toArray(new File[files.size()]);
 		}
 
 		private void printReport(TestResults results) throws FileNotFoundException {

spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/AbstractTester.java

@@ -20,20 +20,23 @@ import java.io.FileNotFoundException;
 import java.util.List;
 import java.util.Set;
 
+/**
+ * Abstract base class for tester implementations.
+ * 
+ * @author Greg Turnquist
+ */
 public abstract class AbstractTester {
 
-    public TestResults findAndTest(List<Class<?>> compiled) throws FileNotFoundException {
-        Set<Class<?>> testable = findTestableClasses(compiled);
-
-        if (testable.size() == 0) {
-            return TestResults.none;
-        }
-
-        return test(testable.toArray(new Class<?>[]{}));
-    }
+	public TestResults findAndTest(List<Class<?>> compiled) throws FileNotFoundException {
+		Set<Class<?>> testable = findTestableClasses(compiled);
+		if (testable.size() == 0) {
+			return TestResults.NONE;
+		}
+		return test(testable.toArray(new Class<?>[] {}));
+	}
 
-    abstract protected Set<Class<?>> findTestableClasses(List<Class<?>> compiled);
+	protected abstract Set<Class<?>> findTestableClasses(List<Class<?>> compiled);
 
-    abstract protected TestResults test(Class<?>[] testable);
+	protected abstract TestResults test(Class<?>[] testable);
 
 }

spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/Failure.java

@@ -18,31 +18,35 @@ package org.springframework.boot.cli.command.tester;
 
 /**
  * Platform neutral way to capture a test failure
- *
+ * 
  * NOTE: This is needed to avoid having to add JUnit jar file to the deployable artifacts
+ * 
+ * @author Greg Turnquist
  */
 public class Failure {
-    private String description;
-    private String trace;
 
-    public Failure(String description, String trace) {
-        this.description = description;
-        this.trace = trace;
-    }
+	private String description;
+
+	private String trace;
+
+	public Failure(String description, String trace) {
+		this.description = description;
+		this.trace = trace;
+	}
 
-    public String getDescription() {
-        return description;
-    }
+	public String getDescription() {
+		return this.description;
+	}
 
-    public void setDescription(String description) {
-        this.description = description;
-    }
+	public void setDescription(String description) {
+		this.description = description;
+	}
 
-    public String getTrace() {
-        return trace;
-    }
+	public String getTrace() {
+		return this.trace;
+	}
 
-    public void setTrace(String trace) {
-        this.trace = trace;
-    }
+	public void setTrace(String trace) {
+		this.trace = trace;
+	}
 }

spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/TestResults.java

@@ -21,74 +21,78 @@ import java.util.List;
 
 /**
  * Platform neutral way to collect test results
- *
- * NOTE: This is needed to avoid having to add JUnit's jar file to the deployable artifacts
+ * 
+ * NOTE: This is needed to avoid having to add JUnit's jar file to the deployable
+ * artifacts
+ * 
+ * @author Greg Turnquist
  */
 public class TestResults {
 
-    public static final NoTestResults none = new NoTestResults();
+	public static final TestResults NONE = new TestResults() {
+
+		@Override
+		public int getRunCount() {
+			return 0;
+		}
+
+		@Override
+		public int getFailureCount() {
+			return 0;
+		}
 
-    private int runCount;
-    private int failureCount;
-    private Failure[] failures = new Failure[0];
+		@Override
+		public Failure[] getFailures() {
+			return new Failure[0];
+		}
 
-    public void add(TestResults results) {
-        this.runCount += results.getRunCount();
-        this.failureCount += results.getFailureCount();
+		@Override
+		public boolean wasSuccessful() {
+			return true;
+		}
 
-        List<Failure> failures = Arrays.asList(this.failures);
-        failures.addAll(Arrays.asList(results.getFailures()));
-        this.failures = failures.toArray(new Failure[]{});
-    }
+	};
 
+	private int runCount;
 
-    public boolean wasSuccessful() {
-        return this.failureCount == 0;
-    }
+	private int failureCount;
 
-    public int getRunCount() {
-        return runCount;
-    }
+	private Failure[] failures = new Failure[0];
 
-    public void setRunCount(int runCount) {
-        this.runCount = runCount;
-    }
+	public void add(TestResults results) {
+		this.runCount += results.getRunCount();
+		this.failureCount += results.getFailureCount();
+		List<Failure> failures = Arrays.asList(this.failures);
+		failures.addAll(Arrays.asList(results.getFailures()));
+		this.failures = failures.toArray(new Failure[] {});
+	}
 
-    public int getFailureCount() {
-        return failureCount;
-    }
+	public boolean wasSuccessful() {
+		return this.failureCount == 0;
+	}
 
-    public void setFailureCount(int failureCount) {
-        this.failureCount = failureCount;
-    }
+	public int getRunCount() {
+		return this.runCount;
+	}
 
-    public Failure[] getFailures() {
-        return failures;
-    }
+	public void setRunCount(int runCount) {
+		this.runCount = runCount;
+	}
 
-    public void setFailures(Failure[] failures) {
-        this.failures = failures;
-    }
+	public int getFailureCount() {
+		return this.failureCount;
+	}
 
-    private static class NoTestResults extends TestResults {
-        @Override
-        public int getRunCount() {
-            return 0;
-        }
+	public void setFailureCount(int failureCount) {
+		this.failureCount = failureCount;
+	}
 
-        @Override
-        public int getFailureCount() {
-            return 0;
-        }
+	public Failure[] getFailures() {
+		return this.failures;
+	}
 
-        @Override
-        public Failure[] getFailures() {
-            return new Failure[0];
-        }
+	public void setFailures(Failure[] failures) {
+		this.failures = failures;
+	}
 
-        @Override
-        public boolean wasSuccessful() {
-            return true;
-        }
-    }
-}
+}