[Core] Default to --strict mode (#1960)

Unlike other implementations Cucumber-JVM defaults to `--non-strict` mode. By defaulting to `--strict` Cucumber follows the same standards as other implementations. Additionally the option to set strict mode adds complexity for consumers of Cucumbers output. They have to interpret Cucumbers 6 scenario outcomes with this in mind. By removing `--strict/--non-strict` mode we remove this complexity all together. With PR Cucumber will: * default to `--strict` * throw an error on `--non-strict`. * warn when `--strict` is used. * only print snippets as part of the exception thrown by JUnit rather then use the summary printer * only print snippets as part of the exception thrown by TestNG rather then use the summary printer Partial fix for #1788, cucumber/common#714
cucumber · Apr 30, 2020 · 5fbae84 · 5fbae84
1 parent 6c0df8e
commit 5fbae84
Show file tree

Hide file tree

Showing 81 changed files with 710 additions and 842 deletions.
diff --git a/core/src/main/java/io/cucumber/core/cli/Main.java b/core/src/main/java/io/cucumber/core/cli/Main.java
@@ -68,13 +68,6 @@ public static byte run(String[] argv, ClassLoader classLoader) {
             return exitStatus.get();
         }
 
-        if (!runtimeOptions.isStrict()) {
-            log.warn(() -> "By default Cucumber is running in --non-strict mode.\n" +
-                "This default will change to --strict and --non-strict will be removed.\n" +
-                "You can use --strict to suppress this warning"
-            );
-        }
-
         final Runtime runtime = Runtime.builder()
             .withRuntimeOptions(runtimeOptions)
             .withClassLoader(() -> classLoader)

diff --git a/core/src/main/java/io/cucumber/core/options/CommandlineOptionsParser.java b/core/src/main/java/io/cucumber/core/options/CommandlineOptionsParser.java
@@ -3,6 +3,8 @@
 import io.cucumber.core.exception.CucumberException;
 import io.cucumber.core.feature.FeatureWithLines;
 import io.cucumber.core.feature.GluePath;
+import io.cucumber.core.logging.Logger;
+import io.cucumber.core.logging.LoggerFactory;
 import io.cucumber.datatable.DataTable;
 import io.cucumber.gherkin.GherkinDialect;
 import io.cucumber.gherkin.GherkinDialectProvider;
@@ -33,6 +35,8 @@
 
 public final class CommandlineOptionsParser {
 
+    private static final Logger log = LoggerFactory.getLogger(CommandlineOptionsParser.class);
+
     private static final String VERSION = ResourceBundle.getBundle("io.cucumber.core.version").getString("cucumber-jvm.version");
     // IMPORTANT! Make sure USAGE.txt is always uptodate if this class changes.
     private static final String USAGE_RESOURCE = "/io/cucumber/core/options/USAGE.txt";
@@ -89,8 +93,12 @@ private RuntimeOptionsBuilder parse(List<String> args) {
                 parsedOptions.addPluginName(removeArgFor(arg, args));
             } else if (arg.equals("--no-dry-run") || arg.equals("--dry-run") || arg.equals("-d")) {
                 parsedOptions.setDryRun(!arg.startsWith("--no-"));
-            } else if (arg.equals("--no-strict") || arg.equals("--strict") || arg.equals("-s")) {
-                parsedOptions.setStrict(!arg.startsWith("--no-"));
+            } else if (arg.equals("--no-strict")) {
+                out.println("--no-strict is no longer effective");
+                exitCode = 1;
+                return parsedOptions;
+            } else if (arg.equals("--strict") || arg.equals("-s")) {
+                log.warn(() -> "--strict is enabled by default. This option will be removed in a future release.");
             } else if (arg.equals("--no-monochrome") || arg.equals("--monochrome") || arg.equals("-m")) {
                 parsedOptions.setMonochrome(!arg.startsWith("--no-"));
             } else if (arg.equals("--snippets")) {

diff --git a/core/src/main/java/io/cucumber/core/options/Constants.java b/core/src/main/java/io/cucumber/core/options/Constants.java
@@ -45,12 +45,12 @@ public final class Constants {
     public static final String EXECUTION_ORDER_PROPERTY_NAME = "cucumber.execution.order";
 
     /**
-     * Property name used to enable strict execution: {@value}
+     * Property name used to disable strict execution: {@value}
      * <p>
      * When using strict execution Cucumber will treat undefined and pending
      * steps as errors.
      * <p>
-     * By default, strict execution is disabled
+     * By default, strict execution is enabled.
      */
     public static final String EXECUTION_STRICT_PROPERTY_NAME = "cucumber.execution.strict";
 

diff --git a/core/src/main/java/io/cucumber/core/options/CucumberOptionsAnnotationParser.java b/core/src/main/java/io/cucumber/core/options/CucumberOptionsAnnotationParser.java
@@ -152,8 +152,8 @@ private void addDefaultGlueIfNoOverridingGlueIsSpecified(RuntimeOptionsBuilder a
     }
 
     private void addStrict(CucumberOptions options, RuntimeOptionsBuilder args) {
-        if (options.strict()) {
-            args.setStrict(true);
+        if (!options.strict()) {
+            throw new CucumberException("@CucumberOptions(strict=false) is no longer supported. Please use strict=true");
         }
     }
 

diff --git a/core/src/main/java/io/cucumber/core/options/CucumberPropertiesParser.java b/core/src/main/java/io/cucumber/core/options/CucumberPropertiesParser.java
@@ -79,7 +79,7 @@ public RuntimeOptionsBuilder parse(Map<String, String> properties) {
         parse(properties,
             EXECUTION_STRICT_PROPERTY_NAME,
             Boolean::parseBoolean,
-            builder::setStrict
+            CucumberPropertiesParser::errorOnNonStrict
         );
 
         parseAll(properties,
@@ -137,6 +137,12 @@ public RuntimeOptionsBuilder parse(Map<String, String> properties) {
         return builder;
     }
 
+    private static void errorOnNonStrict(Boolean strict) {
+        if (!strict) {
+            throw new CucumberException(EXECUTION_STRICT_PROPERTY_NAME + "=false is no longer effective. Please use =true (the default) or remove this property");
+        }
+    }
+
     private static Stream<FeatureWithLines> parseFeatureFile(String property) {
         if (property.startsWith("@")) {
             return Stream.empty();

diff --git a/core/src/main/java/io/cucumber/core/options/RuntimeOptions.java b/core/src/main/java/io/cucumber/core/options/RuntimeOptions.java
@@ -35,7 +35,6 @@ public final class RuntimeOptions implements
     private final List<FeatureWithLines> featurePaths = new ArrayList<>();
 
     private boolean dryRun;
-    private boolean strict = false;
     private boolean monochrome = false;
     private boolean wip = false;
     private SnippetType snippetType = SnippetType.UNDERSCORE;
@@ -106,11 +105,6 @@ public List<URI> getGlue() {
         return unmodifiableList(glue);
     }
 
-    @Override
-    public boolean isStrict() {
-        return strict;
-    }
-
     @Override
     public boolean isDryRun() {
         return dryRun;
@@ -222,10 +216,6 @@ void setSnippetType(SnippetType snippetType) {
         this.snippetType = snippetType;
     }
 
-    void setStrict(boolean strict) {
-        this.strict = strict;
-    }
-
     void setThreads(int threads) {
         this.threads = threads;
     }

diff --git a/core/src/main/java/io/cucumber/core/options/RuntimeOptionsBuilder.java b/core/src/main/java/io/cucumber/core/options/RuntimeOptionsBuilder.java
@@ -23,7 +23,6 @@ public final class RuntimeOptionsBuilder {
     private List<FeatureWithLines> parsedRerunPaths = null;
     private Integer parsedThreads = null;
     private Boolean parsedDryRun = null;
-    private Boolean parsedStrict = null;
     private Boolean parsedMonochrome = null;
     private SnippetType parsedSnippetType = null;
     private Boolean parsedWip = null;
@@ -81,10 +80,6 @@ public RuntimeOptions build(RuntimeOptions runtimeOptions) {
             runtimeOptions.setDryRun(this.parsedDryRun);
         }
 
-        if (this.parsedStrict != null) {
-            runtimeOptions.setStrict(this.parsedStrict);
-        }
-
         if (this.parsedMonochrome != null) {
             runtimeOptions.setMonochrome(this.parsedMonochrome);
         }
@@ -186,15 +181,6 @@ public RuntimeOptionsBuilder setSnippetType(SnippetType snippetType) {
         return this;
     }
 
-    public RuntimeOptionsBuilder setStrict() {
-        return setStrict(true);
-    }
-
-    public RuntimeOptionsBuilder setStrict(boolean strict) {
-        this.parsedStrict = strict;
-        return this;
-    }
-
     public RuntimeOptionsBuilder setThreads(int threads) {
         this.parsedThreads = threads;
         return this;

diff --git a/core/src/main/java/io/cucumber/core/plugin/DefaultSummaryPrinter.java b/core/src/main/java/io/cucumber/core/plugin/DefaultSummaryPrinter.java
@@ -1,20 +1,19 @@
 package io.cucumber.core.plugin;
 
-import io.cucumber.plugin.event.EventPublisher;
-import io.cucumber.plugin.event.SnippetsSuggestedEvent;
-import io.cucumber.plugin.event.TestRunFinished;
 import io.cucumber.plugin.ColorAware;
 import io.cucumber.plugin.ConcurrentEventListener;
-import io.cucumber.plugin.StrictAware;
 import io.cucumber.plugin.SummaryPrinter;
+import io.cucumber.plugin.event.EventPublisher;
+import io.cucumber.plugin.event.SnippetsSuggestedEvent;
+import io.cucumber.plugin.event.TestRunFinished;
 
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
-public final class DefaultSummaryPrinter implements SummaryPrinter, ColorAware, StrictAware, ConcurrentEventListener {
+public final class DefaultSummaryPrinter implements SummaryPrinter, ColorAware, ConcurrentEventListener {
 
     private final Set<String> snippets = new LinkedHashSet<>();
     private final Stats stats = new Stats();
@@ -84,8 +83,5 @@ public void setMonochrome(boolean monochrome) {
         stats.setMonochrome(monochrome);
     }
 
-    @Override
-    public void setStrict(boolean strict) {
-        stats.setStrict(strict);
-    }
+
 }
diff --git a/core/src/main/java/io/cucumber/core/plugin/JUnitFormatter.java b/core/src/main/java/io/cucumber/core/plugin/JUnitFormatter.java
@@ -3,7 +3,6 @@
 import io.cucumber.core.exception.CucumberException;
 import io.cucumber.core.feature.FeatureParser;
 import io.cucumber.plugin.EventListener;
-import io.cucumber.plugin.StrictAware;
 import io.cucumber.plugin.event.EventPublisher;
 import io.cucumber.plugin.event.PickleStepTestStep;
 import io.cucumber.plugin.event.Result;
@@ -46,15 +45,14 @@
 import static java.util.Locale.ROOT;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
-public final class JUnitFormatter implements EventListener, StrictAware {
+public final class JUnitFormatter implements EventListener {
 
     private static final long MILLIS_PER_SECOND = SECONDS.toMillis(1L);
     private final Writer writer;
     private final Document document;
     private final Element rootElement;
     private Element root;
     private TestCase testCase;
-    private boolean strict = false;
     private URI currentFeatureFile = null;
     private String previousTestCaseName;
     private int exampleNumber;
@@ -97,11 +95,6 @@ private void handleTestRunStarted(TestRunStarted event) {
         this.started = event.getInstant();
     }
 
-    @Override
-    public void setStrict(boolean strict) {
-        this.strict = strict;
-    }
-
     private void handleTestSourceRead(TestSourceRead event) {
         TestSourceReadResource source = new TestSourceReadResource(event);
         parser.parseResource(source).ifPresent(feature ->
@@ -218,12 +211,8 @@ void addTestCaseElement(Document doc, Element tc, Result result) {
                 addStackTrace(sb, result);
                 child = createFailure(doc, sb, result.getError().getMessage(), result.getError().getClass());
             } else if (status.is(Status.PENDING) || status.is(Status.UNDEFINED)) {
-                if (strict) {
-                    Throwable error = result.getError();
-                    child = createFailure(doc, sb, "The scenario has pending or undefined step(s)", error == null ? Exception.class : error.getClass());
-                } else {
-                    child = createElement(doc, sb, "skipped");
-                }
+                Throwable error = result.getError();
+                child = createFailure(doc, sb, "The scenario has pending or undefined step(s)", error == null ? Exception.class : error.getClass());
             } else if (status.is(Status.SKIPPED) && result.getError() != null) {
                 addStackTrace(sb, result);
                 child = createSkipped(doc, sb, printStackTrace(result.getError()));
@@ -236,14 +225,7 @@ void addTestCaseElement(Document doc, Element tc, Result result) {
 
         void handleEmptyTestCase(Document doc, Element tc, Result result) {
             tc.setAttribute("time", calculateTotalDurationString(result.getDuration()));
-
-            Element child;
-            if (strict) {
-                child = createFailure(doc, new StringBuilder(), "The scenario has no steps", Exception.class);
-            } else {
-                child = createSkipped(doc, new StringBuilder(), "The scenario has no steps");
-            }
-
+            Element child = createFailure(doc, new StringBuilder(), "The scenario has no steps", Exception.class);
             tc.appendChild(child);
         }
 

diff --git a/core/src/main/java/io/cucumber/core/plugin/Options.java b/core/src/main/java/io/cucumber/core/plugin/Options.java
@@ -5,8 +5,6 @@ public interface Options {
 
     boolean isMonochrome();
 
-    boolean isStrict();
-
     boolean isWip();
 
     interface Plugin {

diff --git a/core/src/main/java/io/cucumber/core/plugin/Plugins.java b/core/src/main/java/io/cucumber/core/plugin/Plugins.java
@@ -77,7 +77,7 @@ private void setMonochromeOnColorAwarePlugins(Plugin plugin) {
     private void setStrictOnStrictAwarePlugins(Plugin plugin) {
         if (plugin instanceof StrictAware) {
             StrictAware strictAware = (StrictAware) plugin;
-            strictAware.setStrict(pluginOptions.isStrict());
+            strictAware.setStrict(true);
         }
     }
 

diff --git a/core/src/main/java/io/cucumber/core/plugin/RerunFormatter.java b/core/src/main/java/io/cucumber/core/plugin/RerunFormatter.java
@@ -2,7 +2,6 @@
 
 import io.cucumber.core.feature.FeatureWithLines;
 import io.cucumber.plugin.ConcurrentEventListener;
-import io.cucumber.plugin.StrictAware;
 import io.cucumber.plugin.event.EventPublisher;
 import io.cucumber.plugin.event.TestCase;
 import io.cucumber.plugin.event.TestCaseFinished;
@@ -22,12 +21,10 @@
  * Formatter for reporting all failed test cases and print their locations
  * Failed means: results that make the exit code non-zero.
  */
-public final class RerunFormatter implements ConcurrentEventListener, StrictAware {
+public final class RerunFormatter implements ConcurrentEventListener {
     private final NiceAppendable out;
     private final Map<URI, Collection<Integer>> featureAndFailedLinesMapping = new HashMap<>();
 
-    private boolean isStrict = false;
-
     public RerunFormatter(OutputStream out) {
         this.out = new NiceAppendable(new UTF8OutputStreamWriter(out));
     }
@@ -38,13 +35,8 @@ public void setEventPublisher(EventPublisher publisher) {
         publisher.registerHandlerFor(TestRunFinished.class, event -> finishReport());
     }
 
-    @Override
-    public void setStrict(boolean strict) {
-        isStrict = strict;
-    }
-
     private void handleTestCaseFinished(TestCaseFinished event) {
-        if (!event.getResult().getStatus().isOk(isStrict)) {
+        if (!event.getResult().getStatus().isOk()) {
             recordTestFailed(event.getTestCase());
         }
     }

diff --git a/core/src/main/java/io/cucumber/core/plugin/Stats.java b/core/src/main/java/io/cucumber/core/plugin/Stats.java
@@ -1,5 +1,7 @@
 package io.cucumber.core.plugin;
 
+import io.cucumber.plugin.ColorAware;
+import io.cucumber.plugin.ConcurrentEventListener;
 import io.cucumber.plugin.event.EventPublisher;
 import io.cucumber.plugin.event.PickleStepTestStep;
 import io.cucumber.plugin.event.Result;
@@ -9,9 +11,6 @@
 import io.cucumber.plugin.event.TestRunFinished;
 import io.cucumber.plugin.event.TestRunStarted;
 import io.cucumber.plugin.event.TestStepFinished;
-import io.cucumber.plugin.ColorAware;
-import io.cucumber.plugin.ConcurrentEventListener;
-import io.cucumber.plugin.StrictAware;
 
 import java.io.PrintStream;
 import java.text.DecimalFormat;
@@ -25,7 +24,7 @@
 import static java.util.Locale.ROOT;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
-class Stats implements ConcurrentEventListener, ColorAware, StrictAware {
+class Stats implements ConcurrentEventListener, ColorAware {
     private static final long ONE_SECOND = SECONDS.toNanos(1);
     private static final long ONE_MINUTE = 60 * ONE_SECOND;
     private final SubCounts scenarioSubCounts = new SubCounts();
@@ -39,7 +38,6 @@ class Stats implements ConcurrentEventListener, ColorAware, StrictAware {
     private final List<String> pendingScenarios = new ArrayList<>();
     private final List<String> undefinedScenarios = new ArrayList<>();
     private final List<Throwable> errors = new ArrayList<>();
-    private boolean strict;
 
     Stats() {
         this(Locale.getDefault());
@@ -58,11 +56,6 @@ public void setMonochrome(boolean monochrome) {
         }
     }
 
-    @Override
-    public void setStrict(boolean strict) {
-        this.strict = true;
-    }
-
     @Override
     public void setEventPublisher(EventPublisher publisher) {
         publisher.registerHandlerFor(TestRunStarted.class, this::setStartTime);
@@ -132,10 +125,8 @@ private void printDuration(PrintStream out) {
     private void printNonZeroResultScenarios(PrintStream out) {
         printScenarios(out, failedScenarios, Status.FAILED);
         printScenarios(out, ambiguousScenarios, Status.AMBIGUOUS);
-        if (strict) {
-            printScenarios(out, pendingScenarios, Status.PENDING);
-            printScenarios(out, undefinedScenarios, Status.UNDEFINED);
-        }
+        printScenarios(out, pendingScenarios, Status.PENDING);
+        printScenarios(out, undefinedScenarios, Status.UNDEFINED);
     }
 
     private void printScenarios(PrintStream out, List<String> scenarios, Status type) {