Issue #2 Ensuring features are parsed before formatters are initialised

Author: tmullender

core/src/main/java/cucumber/runtime/RuntimeOptions.java

@@ -31,6 +31,7 @@ public class RuntimeOptions {
     private final List<Object> lineFilters = new ArrayList<Object>();
     private final List<Formatter> formatters = new ArrayList<Formatter>();
     private final List<String> featurePaths = new ArrayList<String>();
+    private final List<String> formatterNames = new ArrayList<String>();
     private final FormatterFactory formatterFactory;
     private URL dotCucumber;
     private boolean dryRun;
@@ -80,10 +81,9 @@ public RuntimeOptions(Env env, FormatterFactory formatterFactory, List<String> a
         }
         filters.addAll(lineFilters);
 
-        if (formatters.isEmpty()) {
-            formatters.add(formatterFactory.create("progress"));
+        if (formatterNames.isEmpty()) {
+            formatterNames.add("progress");
         }
-        setFormatterOptions();
     }
 
     private void parse(List<String> args) {
@@ -107,7 +107,7 @@ private void parse(List<String> args) {
             } else if (arg.equals("--tags") || arg.equals("-t")) {
                 parsedFilters.add(args.remove(0));
             } else if (arg.equals("--format") || arg.equals("-f")) {
-                formatters.add(formatterFactory.create(args.remove(0)));
+                formatterNames.add(args.remove(0));
             } else if (arg.equals("--dotcucumber")) {
                 String urlOrPath = args.remove(0);
                 dotCucumber = Utils.toURL(urlOrPath);
@@ -156,12 +156,24 @@ private void printUsage() {
     public List<CucumberFeature> cucumberFeatures(ResourceLoader resourceLoader) {
         return load(resourceLoader, featurePaths, filters, System.out);
     }
+    
+    protected List<Formatter> formatters() {
+        if (formatters.isEmpty()){
+            for (String formatterName : formatterNames){
+                final Formatter formatter = formatterFactory.create(formatterName);
+                formatters.add(formatter);
+                setMonochromeOnColorAwareFormatters(formatter);
+                setStrictOnStrictAwareFormatters(formatter);
+            }
+        }
+        return formatters;
+    }
 
     public Formatter formatter(ClassLoader classLoader) {
         return (Formatter) Proxy.newProxyInstance(classLoader, new Class<?>[]{Formatter.class}, new InvocationHandler() {
             @Override
             public Object invoke(Object target, Method method, Object[] args) throws Throwable {
-                for (Formatter formatter : formatters) {
+                for (Formatter formatter : formatters()) {
                     Utils.invoke(formatter, method, 0, args);
                 }
                 return null;
@@ -183,13 +195,6 @@ public Object invoke(Object target, Method method, Object[] args) throws Throwab
         });
     }
 
-    private void setFormatterOptions() {
-        for (Formatter formatter : formatters) {
-            setMonochromeOnColorAwareFormatters(formatter);
-            setStrictOnStrictAwareFormatters(formatter);
-        }
-    }
-
     private void setMonochromeOnColorAwareFormatters(Formatter formatter) {
         if (formatter instanceof ColorAware) {
             ColorAware colorAware = (ColorAware) formatter;

core/src/test/java/cucumber/runtime/RuntimeOptionsFactoryTest.java

@@ -105,7 +105,7 @@ public void inherit_formatter_from_baseclass() {
         RuntimeOptionsFactory runtimeOptionsFactory = new RuntimeOptionsFactory(SubClassWithFormatter.class, new Class[]{CucumberOptions.class});
         RuntimeOptions runtimeOptions = runtimeOptionsFactory.create();
 
-        List<Formatter> formatters = runtimeOptions.getFormatters();
+        List<Formatter> formatters = runtimeOptions.formatters();
         assertEquals(2, formatters.size());
         assertTrue(formatters.get(0) instanceof PrettyFormatter);
         assertTrue(formatters.get(1) instanceof JSONFormatter);

core/src/test/java/cucumber/runtime/RuntimeOptionsTest.java

@@ -76,7 +76,7 @@ public void assigns_dotcucumber() throws MalformedURLException {
     @Test
     public void creates_formatter() {
         RuntimeOptions options = new RuntimeOptions(asList("--format", "html:some/dir", "--glue", "somewhere"));
-        assertEquals("cucumber.runtime.formatter.HTMLFormatter", options.getFormatters().get(0).getClass().getName());
+        assertEquals("cucumber.runtime.formatter.HTMLFormatter", options.formatters().get(0).getClass().getName());
     }
 
     @Test
@@ -218,7 +218,8 @@ public void set_monochrome_on_color_aware_formatters() throws Exception {
         Formatter colorAwareFormatter = mock(Formatter.class, withSettings().extraInterfaces(ColorAware.class));
         when(factory.create("progress")).thenReturn(colorAwareFormatter);
 
-        new RuntimeOptions(new Env(), factory, asList("--monochrome", "--format", "progress"));
+        RuntimeOptions options = new RuntimeOptions(new Env(), factory, asList("--monochrome", "--format", "progress"));
+        options.formatters();
 
         verify((ColorAware) colorAwareFormatter).setMonochrome(true);
     }
@@ -229,7 +230,8 @@ public void set_strict_on_strict_aware_formatters() throws Exception {
         Formatter strictAwareFormatter = mock(Formatter.class, withSettings().extraInterfaces(StrictAware.class));
         when(factory.create("junit:out/dir")).thenReturn(strictAwareFormatter);
 
-        new RuntimeOptions(new Env(), factory, asList("--strict", "--format", "junit:out/dir"));
+        RuntimeOptions options = new RuntimeOptions(new Env(), factory, asList("--strict", "--format", "junit:out/dir"));
+        options.formatters();
 
         verify((StrictAware) strictAwareFormatter).setStrict(true);
     }

junit/src/main/java/cucumber/api/junit/Cucumber.java

@@ -62,8 +62,9 @@ public Cucumber(Class clazz) throws InitializationError, IOException {
         ClassFinder classFinder = new ResourceLoaderClassFinder(resourceLoader, classLoader);
         runtime = new Runtime(resourceLoader, classFinder, classLoader, runtimeOptions);
 
+        final List<CucumberFeature> cucumberFeatures = runtimeOptions.cucumberFeatures(resourceLoader);
         jUnitReporter = new JUnitReporter(runtimeOptions.reporter(classLoader), runtimeOptions.formatter(classLoader), runtimeOptions.isStrict());
-        addChildren(runtimeOptions.cucumberFeatures(resourceLoader));
+        addChildren(cucumberFeatures);
     }
 
     @Override

junit/src/test/java/cucumber/runtime/junit/CucumberTest.java

@@ -16,6 +16,7 @@
 
 import static java.util.Collections.emptyList;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 
 public class CucumberTest {
@@ -59,6 +60,16 @@ public void testThatParsingErrorsIsNicelyReported() throws Exception {
             assertEquals("Error parsing feature file cucumber/runtime/error/lexer_error.feature", e.getMessage());
         }
     }
+    
+    @Test
+    public void testThatFileIsNotCreatedOnParsingError() throws Exception {
+        try {
+            new Cucumber(FormatterWithLexerErrorFeature.class);
+            fail("Expecting error");
+        } catch (CucumberException e){
+            assertFalse("File is created despite Lexor Error", new File("lexor_error_feature.json").exists());
+        }
+    }
 
     @Test
     public void finds_no_features_when_explicit_feature_path_has_no_features() throws IOException, InitializationError {
@@ -110,4 +121,10 @@ private class ExplicitFeaturePathWithNoFeatures {
     private class LexerErrorFeature {
 
     }
+    
+    @CucumberOptions(features = {"classpath:cucumber/runtime/error/lexer_error.feature"}, format = {"json:lexor_error_feature.json"})
+    private class FormatterWithLexerErrorFeature {
+
+    }
+
 }