Advanced boolean parsing. Fixes #2095 (#2097)

1. Empty string will result in an illegal argument exception
2. Using any value other then true/false 1/0 or yes/no will result in an illegal argument exception
3. `CucumberEngineOptions` delegates to JUnit 5 via `getBoolean`
4. This (strictly parsing booleans) is a new feature.

Co-authored-by: M.P. Korstanje <[email protected]>

Author: aslakhellesoy

CHANGELOG.md

@@ -8,7 +8,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased] (In Git)
 
 ### Added
-
+* Boolean system properties and environment variables (`cucumber.*` and `CUCUMBER_*`)
+  are strictly parsed. The values `0`, `false`, `no` are interpreted as `false`.
+  The values `1`, `true`, `yes` are interpreted as `true`. All other values will
+  throw an exception.
+  ([#2095](https://github.com/cucumber/cucumber-jvm/pull/2097)
+   [#2097](https://github.com/cucumber/cucumber-jvm/pull/2097)
+   Aslak Hellesøy)
+   
 ### Changed
 
 ### Deprecated
@@ -24,7 +31,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * Publish instructions now recommend using `src/test/resources/cucumber.properties`.
  ([#2096](https://github.com/cucumber/cucumber-jvm/pull/2096)
   Aslak Hellesøy)
-
+  
 ## [6.5.0] (2020-08-17)
 
 ### Added

core/src/main/java/io/cucumber/core/options/BooleanString.java

@@ -0,0 +1,31 @@
+package io.cucumber.core.options;
+
+final class BooleanString {
+
+    static boolean parseBoolean(String s) {
+        if (s == null) {
+            return false;
+        }
+
+        if ("true".equalsIgnoreCase(s)) {
+            return true;
+        } else if ("false".equalsIgnoreCase(s)) {
+            return false;
+        }
+
+        if ("1".equalsIgnoreCase(s)) {
+            return true;
+        } else if ("0".equalsIgnoreCase(s)) {
+            return false;
+        }
+
+        if ("yes".equalsIgnoreCase(s)) {
+            return true;
+        } else if ("no".equalsIgnoreCase(s)) {
+            return false;
+        }
+
+        throw new IllegalArgumentException(
+            String.format("'%s' Was not a valid boolean value. Please use either 'true' or 'false'.", s));
+    }
+}

core/src/main/java/io/cucumber/core/options/CucumberPropertiesParser.java

@@ -42,12 +42,12 @@ public RuntimeOptionsBuilder parse(Map<String, String> properties) {
 
         parse(properties,
             ANSI_COLORS_DISABLED_PROPERTY_NAME,
-            Boolean::parseBoolean,
+            BooleanString::parseBoolean,
             builder::setMonochrome);
 
         parse(properties,
             EXECUTION_DRY_RUN_PROPERTY_NAME,
-            Boolean::parseBoolean,
+            BooleanString::parseBoolean,
             builder::setDryRun);
 
         parse(properties,
@@ -62,7 +62,7 @@ public RuntimeOptionsBuilder parse(Map<String, String> properties) {
 
         parse(properties,
             EXECUTION_STRICT_PROPERTY_NAME,
-            Boolean::parseBoolean,
+            BooleanString::parseBoolean,
             CucumberPropertiesParser::errorOnNonStrict);
 
         parseAll(properties,
@@ -107,12 +107,12 @@ public RuntimeOptionsBuilder parse(Map<String, String> properties) {
 
         parse(properties,
             PLUGIN_PUBLISH_ENABLED_PROPERTY_NAME,
-            Boolean::parseBoolean,
+            BooleanString::parseBoolean,
             builder::setPublish);
 
         parse(properties,
             PLUGIN_PUBLISH_QUIET_PROPERTY_NAME,
-            Boolean::parseBoolean,
+            BooleanString::parseBoolean,
             builder::setPublishQuiet);
 
         parse(properties,
@@ -122,7 +122,7 @@ public RuntimeOptionsBuilder parse(Map<String, String> properties) {
 
         parse(properties,
             WIP_PROPERTY_NAME,
-            Boolean::parseBoolean,
+            BooleanString::parseBoolean,
             builder::setWip);
 
         return builder;

core/src/test/java/io/cucumber/core/options/BooleanStringTest.java

@@ -0,0 +1,36 @@
+package io.cucumber.core.options;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class BooleanStringTest {
+
+    @Test
+    public void null_is_false() {
+        assertThat(BooleanString.parseBoolean(null), is(false));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "false", "no", "0"})
+    public void falsy_values_are_false(String value) {
+        assertThat(BooleanString.parseBoolean(value), is(false));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "true", "yes", "1" })
+    public void truthy_values_are_true(String value) {
+        assertThat(BooleanString.parseBoolean(value), is(true));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "y", "n", "-1", " ", "" })
+    public void unknown_values_throw_illegal_argument_exception(String value) {
+        assertThrows(IllegalArgumentException.class, () -> BooleanString.parseBoolean(value));
+    }
+
+}

junit-platform-engine/src/main/java/io/cucumber/junit/platform/engine/CucumberEngineOptions.java

@@ -156,7 +156,7 @@ public Class<? extends ObjectFactory> getObjectFactoryClass() {
 
     boolean isParallelExecutionEnabled() {
         return configurationParameters
-                .get(PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, Boolean::parseBoolean)
+                .getBoolean(PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME)
                 .orElse(false);
     }