Polishing of JaCoCo coverage validation changes

subprojects/docs/src/docs/userguide/jacocoPlugin.xml

@@ -90,10 +90,9 @@
         </note>
         <para>
             The <apilink class="org.gradle.testing.jacoco.tasks.JacocoCoverageVerification"/> task can be used to verify if code coverage
-            metrics are met based on configured rules. Its API exposes two methods,
-            <apilink class="org.gradle.testing.jacoco.tasks.JacocoCoverageVerification" method="violationRules(groovy.lang.Closure)"/> and
-            <apilink class="org.gradle.testing.jacoco.tasks.JacocoCoverageVerification" method="violationRules(org.gradle.api.Action)"/>,
-            the main entry point for configuring rules.
+            metrics are met based on configured rules. Its API exposes the method
+            <apilink class="org.gradle.testing.jacoco.tasks.JacocoCoverageVerification" method="violationRules(org.gradle.api.Action)"/>
+            which is used as main entry point for configuring rules.
             Invoking any of those methods returns an instance of <apilink class="org.gradle.testing.jacoco.tasks.rules.JacocoViolationRulesContainer"/> providing
             extensive configuration options. The build fails if any of the configured rules are not met. JaCoCo only reports the first violated rule.
         </para>

subprojects/jacoco/src/integTest/groovy/org/gradle/testing/jacoco/plugins/rules/JacocoPluginCoverageVerificationIntegrationTest.groovy

@@ -21,8 +21,8 @@ import org.gradle.testing.jacoco.plugins.JacocoMultiVersionIntegrationTest
 import org.gradle.testing.jacoco.plugins.fixtures.JacocoCoverage
 import spock.lang.Unroll
 
-import static JacocoViolationRulesLimit.Insufficient
-import static JacocoViolationRulesLimit.Sufficient
+import static org.gradle.testing.jacoco.plugins.rules.JacocoViolationRulesLimit.Insufficient
+import static org.gradle.testing.jacoco.plugins.rules.JacocoViolationRulesLimit.Sufficient
 
 @TargetCoverage({ JacocoCoverage.SUPPORTS_JDK_8_OR_HIGHER })
 class JacocoPluginCoverageVerificationIntegrationTest extends JacocoMultiVersionIntegrationTest {
@@ -150,12 +150,12 @@ class JacocoPluginCoverageVerificationIntegrationTest extends JacocoMultiVersion
         executedAndNotSkipped(TEST_AND_JACOCO_COVERAGE_VERIFICATION_TASK_PATHS)
 
         where:
-        limits                                 | description
-        [Sufficient.LINE_METRIC_COVERED_RATIO] | 'line metric with covered ratio'
-        [Sufficient.CLASS_METRIC_MISSED_COUNT] | 'class metric with missed count'
+        limits                                               | description
+        [Sufficient.LINE_METRIC_COVERED_RATIO]               | 'line metric with covered ratio'
+        [Sufficient.CLASS_METRIC_MISSED_COUNT]               | 'class metric with missed count'
         [Sufficient.LINE_METRIC_COVERED_RATIO,
-         Sufficient.CLASS_METRIC_MISSED_COUNT] | 'line and class metric'
-
+         Sufficient.CLASS_METRIC_MISSED_COUNT]               | 'line and class metric'
+        [Sufficient.LINE_METRIC_COVERED_RATIO_OUT_OF_BOUNDS] | 'line metric with covered ratio with values out of bounds'
     }
 
     @Unroll
@@ -179,17 +179,40 @@ class JacocoPluginCoverageVerificationIntegrationTest extends JacocoMultiVersion
         errorOutput.contains("Rule violated for bundle $testDirectory.name: $errorMessage")
 
         where:
-        limits                                   | description                                   | errorMessage
-        [Insufficient.LINE_METRIC_COVERED_RATIO] | 'line metric with covered ratio'              | 'lines covered ratio is 1.0, but expected maximum is 0.5'
-        [Insufficient.CLASS_METRIC_MISSED_COUNT] | 'class metric with missed count'              | 'classes missed count is 0.0, but expected minimum is 0.5'
+        limits                                                      | description                                                              | errorMessage
+        [Insufficient.LINE_METRIC_COVERED_RATIO]                    | 'line metric with covered ratio'                                         | 'lines covered ratio is 1.0, but expected maximum is 0.5'
+        [Insufficient.CLASS_METRIC_MISSED_COUNT]                    | 'class metric with missed count'                                         | 'classes missed count is 0.0, but expected minimum is 0.5'
         [Insufficient.LINE_METRIC_COVERED_RATIO,
-         Insufficient.CLASS_METRIC_MISSED_COUNT] | 'first of multiple insufficient limits fails' | 'lines covered ratio is 1.0, but expected maximum is 0.5'
+         Insufficient.CLASS_METRIC_MISSED_COUNT]                    | 'first of multiple insufficient limits fails'                            | 'lines covered ratio is 1.0, but expected maximum is 0.5'
         [Sufficient.LINE_METRIC_COVERED_RATIO,
          Insufficient.CLASS_METRIC_MISSED_COUNT,
-         Sufficient.CLASS_METRIC_MISSED_COUNT]   | 'first insufficient limits fails'             | 'classes missed count is 0.0, but expected minimum is 0.5'
+         Sufficient.CLASS_METRIC_MISSED_COUNT]                      | 'first insufficient limits fails'                                        | 'classes missed count is 0.0, but expected minimum is 0.5'
+        [Insufficient.CLASS_METRIC_MISSED_COUNT_MINIMUM_GT_MAXIMUM] | 'class metric with missed count with minimum greater than maximum value' | 'classes missed count is 0.0, but expected minimum is 0.5'
+    }
+
+    def "can define same rule multiple times"() {
+        given:
+        buildFile << """
+            jacocoTestCoverageVerification {
+                violationRules {
+                    rule {
+                        ${Sufficient.LINE_METRIC_COVERED_RATIO}
+                    }
+                    rule {
+                        ${Sufficient.LINE_METRIC_COVERED_RATIO}
+                    }
+                }
+            }
+        """
+
+        when:
+        succeeds TEST_AND_JACOCO_COVERAGE_VERIFICATION_TASK_PATHS
+
+        then:
+        executedAndNotSkipped(TEST_AND_JACOCO_COVERAGE_VERIFICATION_TASK_PATHS)
     }
 
-    def "can define multiple rules"() {
+    def "can define multiple, different rules"() {
         given:
         buildFile << """
             jacocoTestCoverageVerification {
@@ -240,7 +263,7 @@ class JacocoPluginCoverageVerificationIntegrationTest extends JacocoMultiVersion
         buildFile << """
             jacocoTestCoverageVerification {
                 violationRules {
-                    failOnViolation = true
+                    failOnViolation = false
 
                     rule {
                         $Insufficient.LINE_METRIC_COVERED_RATIO

subprojects/jacoco/src/integTest/groovy/org/gradle/testing/jacoco/plugins/rules/JacocoViolationRulesLimit.groovy

@@ -21,16 +21,18 @@ final class JacocoViolationRulesLimit {
     private JacocoViolationRulesLimit() {}
 
     static class Sufficient {
-        static final String LINE_METRIC_COVERED_RATIO = JacocoViolationRulesLimit.create('LINE', 'COVEREDRATIO', '0.0', '1.0')
-        static final String CLASS_METRIC_MISSED_COUNT = JacocoViolationRulesLimit.create('CLASS', 'MISSEDCOUNT', null, '0')
+        static final String LINE_METRIC_COVERED_RATIO = JacocoViolationRulesLimit.create('LINE', 'COVEREDRATIO', 0.0, 1.0)
+        static final String CLASS_METRIC_MISSED_COUNT = JacocoViolationRulesLimit.create('CLASS', 'MISSEDCOUNT', null, 0)
+        static final String LINE_METRIC_COVERED_RATIO_OUT_OF_BOUNDS = JacocoViolationRulesLimit.create('LINE', 'COVEREDRATIO', -1.0, 2.0)
     }
 
     static class Insufficient {
-        static final String LINE_METRIC_COVERED_RATIO = JacocoViolationRulesLimit.create('LINE', 'COVEREDRATIO', '0.0', '0.5')
-        static final String CLASS_METRIC_MISSED_COUNT = JacocoViolationRulesLimit.create('CLASS', 'MISSEDCOUNT', '0.5', null)
+        static final String LINE_METRIC_COVERED_RATIO = JacocoViolationRulesLimit.create('LINE', 'COVEREDRATIO', 0.0, 0.5)
+        static final String CLASS_METRIC_MISSED_COUNT = JacocoViolationRulesLimit.create('CLASS', 'MISSEDCOUNT', 0.5, null)
+        static final String CLASS_METRIC_MISSED_COUNT_MINIMUM_GT_MAXIMUM = JacocoViolationRulesLimit.create('CLASS', 'MISSEDCOUNT', 0.5, 0.1)
     }
 
-    private static String create(String counter, String value, String minimum, String maximum) {
+    private static String create(String counter, String value, BigDecimal minimum, BigDecimal maximum) {
         StringBuilder limit = new StringBuilder()
         limit <<= 'limit {\n'
 

subprojects/jacoco/src/main/java/org/gradle/internal/jacoco/AntJacocoCheck.java

@@ -47,7 +47,7 @@ public AntJacocoCheck(IsolatedAntBuilder ant) {
     @Override
     protected void configureReport(final GroovyObjectSupport antBuilder, final JacocoViolationRulesContainer violationRules) {
         if (!violationRules.getRules().isEmpty()) {
-            Map<String, Object> checkArgs = ImmutableMap.<String, Object>of("failonviolation", !violationRules.isFailOnViolation());
+            Map<String, Object> checkArgs = ImmutableMap.<String, Object>of("failonviolation", violationRules.isFailOnViolation());
             antBuilder.invokeMethod("check", new Object[] {checkArgs, new Closure<Object>(this, this) {
                 @SuppressWarnings("UnusedDeclaration")
                 public Object doCall(Object ignore) {

subprojects/jacoco/src/main/java/org/gradle/internal/jacoco/rules/JacocoLimitImpl.java

@@ -18,12 +18,14 @@
 
 import org.gradle.testing.jacoco.tasks.rules.JacocoLimit;
 
+import java.math.BigDecimal;
+
 public class JacocoLimitImpl implements JacocoLimit {
 
     private String counter = "INSTRUCTION";
     private String value = "COVEREDRATIO";
-    private Double minimum;
-    private Double maximum;
+    private BigDecimal minimum;
+    private BigDecimal maximum;
 
     @Override
     public String getCounter() {
@@ -46,22 +48,22 @@ public void setValue(String value) {
     }
 
     @Override
-    public Double getMinimum() {
+    public BigDecimal getMinimum() {
         return minimum;
     }
 
     @Override
-    public void setMinimum(Double minimum) {
+    public void setMinimum(BigDecimal minimum) {
         this.minimum = minimum;
     }
 
     @Override
-    public Double getMaximum() {
+    public BigDecimal getMaximum() {
         return maximum;
     }
 
     @Override
-    public void setMaximum(Double maximum) {
+    public void setMaximum(BigDecimal maximum) {
         this.maximum = maximum;
     }
 

subprojects/jacoco/src/main/java/org/gradle/internal/jacoco/rules/JacocoViolationRuleImpl.java

@@ -17,9 +17,7 @@
 package org.gradle.internal.jacoco.rules;
 
 import com.google.common.collect.ImmutableList;
-import groovy.lang.Closure;
 import org.gradle.api.Action;
-import org.gradle.api.internal.ClosureBackedAction;
 import org.gradle.testing.jacoco.tasks.rules.JacocoLimit;
 import org.gradle.testing.jacoco.tasks.rules.JacocoViolationRule;
 
@@ -80,11 +78,6 @@ public List<JacocoLimit> getLimits() {
         return Collections.unmodifiableList(limits);
     }
 
-    @Override
-    public JacocoLimit limit(Closure configureClosure) {
-        return limit(ClosureBackedAction.of(configureClosure));
-    }
-
     @Override
     public JacocoLimit limit(Action<? super JacocoLimit> configureAction) {
         JacocoLimit limit = new JacocoLimitImpl();

subprojects/jacoco/src/main/java/org/gradle/internal/jacoco/rules/JacocoViolationRulesContainerImpl.java

@@ -16,9 +16,8 @@
 
 package org.gradle.internal.jacoco.rules;
 
-import groovy.lang.Closure;
 import org.gradle.api.Action;
-import org.gradle.api.internal.ClosureBackedAction;
+import org.gradle.internal.reflect.Instantiator;
 import org.gradle.testing.jacoco.tasks.rules.JacocoViolationRule;
 import org.gradle.testing.jacoco.tasks.rules.JacocoViolationRulesContainer;
 
@@ -28,9 +27,14 @@
 
 public class JacocoViolationRulesContainerImpl implements JacocoViolationRulesContainer {
 
-    private boolean failOnViolation;
+    private final Instantiator instantiator;
+    private boolean failOnViolation = true;
     private final List<JacocoViolationRule> rules = new ArrayList<JacocoViolationRule>();
 
+    public JacocoViolationRulesContainerImpl(Instantiator instantiator) {
+        this.instantiator = instantiator;
+    }
+
     public void setFailOnViolation(boolean failOnViolation) {
         this.failOnViolation = failOnViolation;
     }
@@ -44,14 +48,9 @@ public List<JacocoViolationRule> getRules() {
         return Collections.unmodifiableList(rules);
     }
 
-    @Override
-    public JacocoViolationRule rule(Closure configureClosure) {
-        return rule(ClosureBackedAction.of(configureClosure));
-    }
-
     @Override
     public JacocoViolationRule rule(Action<? super JacocoViolationRule> configureAction) {
-        JacocoViolationRule validationRule = new JacocoViolationRuleImpl();
+        JacocoViolationRule validationRule = instantiator.newInstance(JacocoViolationRuleImpl.class);
         configureAction.execute(validationRule);
         rules.add(validationRule);
         return validationRule;

subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/plugins/JacocoPlugin.java

@@ -82,7 +82,7 @@ public File call() {
         applyToDefaultTasks(extension);
         configureDefaultOutputPathForJacocoMerge();
         configureJacocoReportsDefaults(extension);
-        addDefaultReportAndCheckTasks(extension);
+        addDefaultReportAndCoverageVerificationTasks(extension);
     }
 
     /**
@@ -203,11 +203,11 @@ public File call() {
     }
 
     /**
-     * Adds report and check tasks for specific default test tasks.
+     * Adds report and coverage verification tasks for specific default test tasks.
      *
      * @param extension the extension describing the test task names
      */
-    private void addDefaultReportAndCheckTasks(final JacocoPluginExtension extension) {
+    private void addDefaultReportAndCoverageVerificationTasks(final JacocoPluginExtension extension) {
         project.getPlugins().withType(JavaPlugin.class, new Action<JavaPlugin>() {
             @Override
             public void execute(JavaPlugin javaPlugin) {

subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/tasks/JacocoCoverageVerification.java

@@ -16,19 +16,15 @@
 
 package org.gradle.testing.jacoco.tasks;
 
-import groovy.lang.Closure;
-import groovy.lang.DelegatesTo;
 import org.gradle.api.Action;
 import org.gradle.api.Incubating;
-import org.gradle.api.internal.ClosureBackedAction;
 import org.gradle.api.tasks.Nested;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.internal.jacoco.AntJacocoCheck;
 import org.gradle.internal.jacoco.rules.JacocoViolationRulesContainerImpl;
+import org.gradle.internal.reflect.Instantiator;
 import org.gradle.testing.jacoco.tasks.rules.JacocoViolationRulesContainer;
 
-import static groovy.lang.Closure.DELEGATE_FIRST;
-
 /**
  * Task for verifying code coverage metrics. Fails the task if violations are detected based on specified rules.
  * <p>
@@ -43,7 +39,8 @@ public class JacocoCoverageVerification extends JacocoReportBase {
 
     public JacocoCoverageVerification() {
         super();
-        violationRules = getInstantiator().newInstance(JacocoViolationRulesContainerImpl.class);
+        Instantiator instantiator = getInstantiator();
+        violationRules = instantiator.newInstance(JacocoViolationRulesContainerImpl.class, instantiator);
     }
 
     /**
@@ -56,14 +53,6 @@ public JacocoViolationRulesContainer getViolationRules() {
         return violationRules;
     }
 
-    /**
-     * Configures the violation rules for this task.
-     */
-    @Incubating
-    public JacocoViolationRulesContainer violationRules(@DelegatesTo(value = JacocoViolationRulesContainer.class, strategy = DELEGATE_FIRST) Closure closure) {
-        return violationRules(new ClosureBackedAction<JacocoViolationRulesContainer>(closure));
-    }
-
     /**
      * Configures the violation rules for this task.
      */

subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/tasks/rules/JacocoLimit.java

@@ -21,6 +21,7 @@
 import org.gradle.api.tasks.Optional;
 
 import java.io.Serializable;
+import java.math.BigDecimal;
 
 /**
  * Defines a Jacoco rule limit.
@@ -55,16 +56,16 @@ public interface JacocoLimit extends Serializable {
      */
     @Input
     @Optional
-    Double getMinimum();
+    BigDecimal getMinimum();
 
-    void setMinimum(Double minimum);
+    void setMinimum(BigDecimal minimum);
 
     /**
      * Gets the maximum expected value for limit. Default to null.
      */
     @Input
     @Optional
-    Double getMaximum();
+    BigDecimal getMaximum();
 
-    void setMaximum(Double maximum);
+    void setMaximum(BigDecimal maximum);
 }

subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/tasks/rules/JacocoViolationRule.java

@@ -16,18 +16,13 @@
 
 package org.gradle.testing.jacoco.tasks.rules;
 
-import groovy.lang.Closure;
-import groovy.lang.DelegatesTo;
 import org.gradle.api.Action;
 import org.gradle.api.Incubating;
 import org.gradle.api.tasks.Input;
-import org.gradle.api.tasks.Nested;
 
 import java.io.Serializable;
 import java.util.List;
 
-import static groovy.lang.Closure.DELEGATE_FIRST;
-
 /**
  * Defines a Jacoco violation rule.
  *
@@ -75,13 +70,11 @@ public interface JacocoViolationRule extends Serializable {
     /**
      * Gets all limits defined for this rule. Defaults to an empty list.
      */
-    @Nested
+    @Input
     List<JacocoLimit> getLimits();
 
     /**
      * Adds a limit for this rule. Any number of limits can be added.
      */
-    JacocoLimit limit(@DelegatesTo(value = JacocoLimit.class, strategy = DELEGATE_FIRST) Closure configureClosure);
-
     JacocoLimit limit(Action<? super JacocoLimit> configureAction);
 }

subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/tasks/rules/JacocoViolationRulesContainer.java

@@ -16,16 +16,12 @@
 
 package org.gradle.testing.jacoco.tasks.rules;
 
-import groovy.lang.Closure;
-import groovy.lang.DelegatesTo;
 import org.gradle.api.Action;
 import org.gradle.api.Incubating;
 import org.gradle.api.tasks.Input;
 
 import java.util.List;
 
-import static groovy.lang.Closure.DELEGATE_FIRST;
-
 /**
  * The violation rules configuration for the {@link org.gradle.testing.jacoco.tasks.JacocoReport} task.
  *
@@ -51,7 +47,5 @@ public interface JacocoViolationRulesContainer {
     /**
      * Adds a violation rule. Any number of rules can be added.
      */
-    JacocoViolationRule rule(@DelegatesTo(value = JacocoViolationRule.class, strategy = DELEGATE_FIRST) Closure configureClosure);
-
     JacocoViolationRule rule(Action<? super JacocoViolationRule> configureAction);
 }