[Core] Merge duplicate plugin options (#2191)

It is possible to declare duplicate plugins. Either through `@CucumberOptions`,
or `cucumber.plugin`, ect or a combination thereof. This can be used to ensure
certain plugins are always active during a build in CI.

Merging duplicate plugin options should ensure that plugins are not
instantiated twice with the same output.

Fixes: #2190

Author: mpkorstanje

Diff for: CHANGELOG.md

@@ -17,7 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 ### Fixed
-* [Core] SummaryPrinter outputs clickable links ([#2184](https://github.com/cucumber/cucumber-jvm/issues/2184) M.P. Korstanje)
+ * [Core] SummaryPrinter outputs clickable links ([#2184](https://github.com/cucumber/cucumber-jvm/issues/2184) M.P. Korstanje)
+ * [Core] Merge duplicate plugin options ([#2190](https://github.com/cucumber/cucumber-jvm/issues/2190) M.P. Korstanje)
 
 ## [6.9.0] (2020-11-12)
 

Diff for: core/src/main/java/io/cucumber/core/options/PluginOption.java

@@ -25,6 +25,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -206,4 +207,16 @@ boolean isSummaryPrinter() {
         return SummaryPrinter.class.isAssignableFrom(pluginClass);
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        PluginOption that = (PluginOption) o;
+        return pluginClass.equals(that.pluginClass) && Objects.equals(argument, that.argument);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(pluginClass, argument);
+    }
 }

Diff for: core/src/main/java/io/cucumber/core/options/RuntimeOptions.java

@@ -12,6 +12,7 @@
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -37,8 +38,8 @@ public final class RuntimeOptions implements
     private final List<Expression> tagExpressions = new ArrayList<>();
     private final List<Pattern> nameFilters = new ArrayList<>();
     private final List<FeatureWithLines> featurePaths = new ArrayList<>();
-    private final List<Plugin> formatters = new ArrayList<>();
-    private final List<Plugin> summaryPrinters = new ArrayList<>();
+    private final Set<Plugin> formatters = new LinkedHashSet<>();
+    private final Set<Plugin> summaryPrinters = new LinkedHashSet<>();
     private boolean dryRun;
     private boolean monochrome = false;
     private boolean wip = false;
@@ -106,11 +107,11 @@ void setThreads(int threads) {
 
     @Override
     public List<Plugin> plugins() {
-        List<Plugin> plugins = new ArrayList<>();
+        Set<Plugin> plugins = new LinkedHashSet<>();
         plugins.addAll(formatters);
         plugins.addAll(summaryPrinters);
         plugins.addAll(getPublishPlugin());
-        return plugins;
+        return new ArrayList<>(plugins);
     }
 
     private List<Plugin> getPublishPlugin() {

Diff for: core/src/test/java/io/cucumber/core/options/BooleanStringTest.java

@@ -11,25 +11,25 @@
 class BooleanStringTest {
 
     @Test
-    public void null_is_false() {
+    void null_is_false() {
         assertThat(BooleanString.parseBoolean(null), is(false));
     }
 
     @ParameterizedTest
     @ValueSource(strings = { "false", "no", "0" })
-    public void falsy_values_are_false(String value) {
+    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) {
+    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) {
+    void unknown_values_throw_illegal_argument_exception(String value) {
         assertThrows(IllegalArgumentException.class, () -> BooleanString.parseBoolean(value));
     }
 

Diff for: core/src/test/java/io/cucumber/core/options/PluginOptionTest.java

@@ -1,5 +1,6 @@
 package io.cucumber.core.options;
 
+import io.cucumber.core.plugin.HtmlFormatter;
 import io.cucumber.core.plugin.PrettyFormatter;
 import io.cucumber.core.plugin.TeamCityPlugin;
 import org.junit.jupiter.api.Test;
@@ -8,6 +9,8 @@
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsNull.nullValue;
 import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class PluginOptionTest {
@@ -77,7 +80,7 @@ void throws_for_plugins_that_do_not_implement_plugin() {
     }
 
     @Test
-    void throws_for_uknown_plugins() {
+    void throws_for_unknown_plugins() {
         IllegalArgumentException exception = assertThrows(IllegalArgumentException.class,
             () -> PluginOption.parse("no-such-plugin"));
 
@@ -93,4 +96,19 @@ void throws_for_uknown_plugins() {
                 "PLUGIN can also be a fully qualified class name, allowing registration of 3rd party plugins. The 3rd party plugin must implement io.cucumber.plugin.Plugin"));
     }
 
+
+    @Test
+    void should_implement_equals_and_hashcode(){
+        PluginOption prettyPluginA = PluginOption.forClass(PrettyFormatter.class);
+        PluginOption prettyPluginB = PluginOption.forClass(PrettyFormatter.class);
+        PluginOption htmlPluginA = PluginOption.forClass(HtmlFormatter.class, "out.html");
+        PluginOption htmlPluginB = PluginOption.forClass(HtmlFormatter.class, "out.html");
+
+        assertEquals(prettyPluginA, prettyPluginB);
+        assertEquals(prettyPluginA.hashCode(), prettyPluginB.hashCode());
+        assertEquals(htmlPluginA, htmlPluginB);
+        assertEquals(htmlPluginA.hashCode(), htmlPluginB.hashCode());
+        assertNotEquals(prettyPluginA, htmlPluginA);
+        assertNotEquals(prettyPluginA.hashCode(), htmlPluginA.hashCode());
+    }
 }

Diff for: core/src/test/java/io/cucumber/core/options/RuntimeOptionsTest.java

@@ -0,0 +1,24 @@
+package io.cucumber.core.options;
+
+import io.cucumber.core.plugin.PrettyFormatter;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+
+import static java.util.Collections.singletonList;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+class RuntimeOptionsTest {
+
+    private final PluginOption aPlugin = PluginOption.forClass(PrettyFormatter.class);
+
+    @Test
+    void shouldRemoveDuplicatePluginRegistrations() {
+        RuntimeOptions runtimeOptions = RuntimeOptions.defaultOptions();
+        runtimeOptions.addSummaryPrinters(Arrays.asList(aPlugin, aPlugin));
+        runtimeOptions.addFormatters(Arrays.asList(aPlugin, aPlugin));
+        assertThat(runtimeOptions.plugins(), is(singletonList(aPlugin)));
+    }
+
+}