Polishing of JaCoCo coverage validation changes

subprojects/docs/src/docs/dsl/dsl.xml

@@ -391,6 +391,9 @@
             <tr>
                 <td>org.gradle.testing.jacoco.tasks.JacocoMerge</td>
             </tr>
+            <tr>
+                <td>org.gradle.testing.jacoco.tasks.JacocoCoverageVerification</td>
+            </tr>
             <tr>
                 <td>org.gradle.api.tasks.bundling.Jar</td>
             </tr>

subprojects/docs/src/docs/release/notes.md

@@ -12,9 +12,9 @@ Add-->
 
 ### Task for enforcing JaCoCo code coverage metrics
 
-Gradle introduces a feature for the JaCoCo plugin strongly requested by the community: enforcing code coverage metrics. The JaCoCo plugin now provides a new task of type `JaCoCoCoverageVerification` enabling the user to define and enforce violation rules. See the relevant user guide section on the “[JaCoCo plugin](userguide/jacoco_plugin.html#sec:jacoco_report_violation_rules)” for more information.
+Gradle introduces a feature for the JaCoCo plugin strongly requested by the community: enforcing code coverage metrics. The JaCoCo plugin now provides a new task of type `JacocoCoverageVerification` enabling the user to define and enforce violation rules. Coverage verification does not automatically run as part of the `check` task. See the relevant user guide section on the “[JaCoCo plugin](userguide/jacoco_plugin.html#sec:jacoco_report_violation_rules)” for more information.
 
-    tasks.withType(JaCoCoCoverageVerification) {
+    tasks.withType(JacocoCoverageVerification) {
         violationRules {
             rule {
                 limit {

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>
@@ -105,6 +104,12 @@
         <sample id="configViolationRules" dir="testing/jacoco/quickstart" includeLocation="true" title="Configuring violation rules">
             <sourcefile file="build.gradle" snippet="violation-rules-configuration"/>
         </sample>
+        <para>
+            The <apilink class="org.gradle.testing.jacoco.tasks.JacocoCoverageVerification"/> task is not a task dependency of the
+            <literal>check</literal> task provided by the Java plugin. There is a good reason for it. The task is currently not incremental as it
+            doesn't declare any outputs. Any violation of the declared rules would automatically result in a failed build when executing the
+            <literal>check</literal> task. This behavior might not be desirable for all users. Future versions of Gradle might change the behavior.
+        </para>
     </section>
 
     <section id="sec:jacoco_specific_task_configuration">
@@ -270,9 +275,10 @@
                     <filename>jacocoAnt</filename>
                 </td>
                 <td>The JaCoCo Ant library used for running the
-                    <literal>JacocoReport</literal>
-                    and
+                    <literal>JacocoReport</literal>,
                     <literal>JacocoMerge</literal>
+                    and
+                    <literal>JacocoCoverageVerification</literal>
                     tasks.
                 </td>
             </tr>

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/AbstractAntJacocoReport.java

@@ -19,6 +19,7 @@
 import com.google.common.collect.ImmutableMap;
 import groovy.lang.Closure;
 import groovy.lang.GroovyObjectSupport;
+import org.gradle.api.Action;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.internal.project.IsolatedAntBuilder;
 
@@ -27,58 +28,62 @@
 
 public abstract class AbstractAntJacocoReport<T> {
 
-    private final IsolatedAntBuilder ant;
+    protected final IsolatedAntBuilder ant;
 
     public AbstractAntJacocoReport(IsolatedAntBuilder ant) {
         this.ant = ant;
     }
 
-    public void execute(FileCollection classpath, final String projectName,
-                        final FileCollection allClassesDirs, final FileCollection allSourcesDirs,
-                        final FileCollection executionData,
-                        final T t) {
+    protected void configureAntReportTask(FileCollection classpath, final Action<GroovyObjectSupport> action) {
         ant.withClasspath(classpath).execute(new Closure<Object>(this, this) {
             @SuppressWarnings("UnusedDeclaration")
             public Object doCall(Object it) {
-                final GroovyObjectSupport antBuilder = (GroovyObjectSupport) it;
+                GroovyObjectSupport antBuilder = (GroovyObjectSupport) it;
                 antBuilder.invokeMethod("taskdef", ImmutableMap.of(
                         "name", "jacocoReport",
                         "classname", "org.jacoco.ant.ReportTask"
                 ));
-                final Map<String, Object> emptyArgs = Collections.emptyMap();
-                antBuilder.invokeMethod("jacocoReport", new Object[]{emptyArgs, new Closure<Object>(this, this) {
+                action.execute(antBuilder);
+                return null;
+            }
+        });
+    }
+
+    protected void invokeJacocoReport(final GroovyObjectSupport antBuilder, final String projectName,
+                     final FileCollection allClassesDirs, final FileCollection allSourcesDirs,
+                     final FileCollection executionData, final T t) {
+        final Map<String, Object> emptyArgs = Collections.emptyMap();
+        antBuilder.invokeMethod("jacocoReport", new Object[]{Collections.emptyMap(), new Closure<Object>(this, this) {
+            @SuppressWarnings("UnusedDeclaration")
+            public Object doCall(Object ignore) {
+                antBuilder.invokeMethod("executiondata", new Object[]{emptyArgs, new Closure<Object>(this, this) {
                     public Object doCall(Object ignore) {
-                        antBuilder.invokeMethod("executiondata", new Object[]{emptyArgs, new Closure<Object>(this, this) {
+                        executionData.addToAntBuilder(antBuilder, "resources");
+                        return null;
+                    }
+                }});
+                Map<String, Object> structureArgs = ImmutableMap.<String, Object>of("name", projectName);
+                antBuilder.invokeMethod("structure", new Object[]{structureArgs, new Closure<Object>(this, this) {
+                    public Object doCall(Object ignore) {
+                        antBuilder.invokeMethod("classfiles", new Object[]{emptyArgs, new Closure<Object>(this, this) {
                             public Object doCall(Object ignore) {
-                                executionData.addToAntBuilder(antBuilder, "resources");
+                                allClassesDirs.addToAntBuilder(antBuilder, "resources");
                                 return null;
                             }
                         }});
-                        Map<String, Object> structureArgs = ImmutableMap.<String, Object>of("name", projectName);
-                        antBuilder.invokeMethod("structure", new Object[]{structureArgs, new Closure<Object>(this, this) {
+                        antBuilder.invokeMethod("sourcefiles", new Object[]{emptyArgs, new Closure<Object>(this, this) {
                             public Object doCall(Object ignore) {
-                                antBuilder.invokeMethod("classfiles", new Object[]{emptyArgs, new Closure<Object>(this, this) {
-                                    public Object doCall(Object ignore) {
-                                        allClassesDirs.addToAntBuilder(antBuilder, "resources");
-                                        return null;
-                                    }
-                                }});
-                                antBuilder.invokeMethod("sourcefiles", new Object[]{emptyArgs, new Closure<Object>(this, this) {
-                                    public Object doCall(Object ignore) {
-                                        allSourcesDirs.addToAntBuilder(antBuilder, "resources");
-                                        return null;
-                                    }
-                                }});
+                                allSourcesDirs.addToAntBuilder(antBuilder, "resources");
                                 return null;
                             }
                         }});
-                        configureReport(antBuilder, t);
                         return null;
                     }
                 }});
+                configureReport(antBuilder, t);
                 return null;
             }
-        });
+        }});
     }
 
     protected abstract void configureReport(GroovyObjectSupport antBuilder, T t);

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

@@ -21,18 +21,23 @@
 import com.google.common.collect.ImmutableMap;
 import groovy.lang.Closure;
 import groovy.lang.GroovyObjectSupport;
+import org.gradle.api.Action;
+import org.gradle.api.file.FileCollection;
 import org.gradle.api.internal.project.IsolatedAntBuilder;
+import org.gradle.internal.reflect.JavaReflectionUtil;
 import org.gradle.testing.jacoco.tasks.rules.JacocoLimit;
 import org.gradle.testing.jacoco.tasks.rules.JacocoViolationRule;
 import org.gradle.testing.jacoco.tasks.rules.JacocoViolationRulesContainer;
 
 import java.util.HashMap;
+import java.util.Hashtable;
 import java.util.Map;
 
 import static com.google.common.collect.Iterables.filter;
 
 public class AntJacocoCheck extends AbstractAntJacocoReport<JacocoViolationRulesContainer> {
 
+    private static final String VIOLATIONS_ANT_PROPERTY = "jacocoViolations";
     private static final Predicate<JacocoViolationRule> RULE_ENABLED_PREDICATE = new Predicate<JacocoViolationRule>() {
         @Override
         public boolean apply(JacocoViolationRule rule) {
@@ -44,10 +49,31 @@ public AntJacocoCheck(IsolatedAntBuilder ant) {
         super(ant);
     }
 
+    public JacocoCheckResult execute(FileCollection classpath, final String projectName,
+                                     final FileCollection allClassesDirs, final FileCollection allSourcesDirs,
+                                     final FileCollection executionData, final JacocoViolationRulesContainer violationRules) {
+        final JacocoCheckResult jacocoCheckResult = new JacocoCheckResult();
+
+        configureAntReportTask(classpath, new Action<GroovyObjectSupport>() {
+            @Override
+            public void execute(GroovyObjectSupport antBuilder) {
+                try {
+                    invokeJacocoReport(antBuilder, projectName, allClassesDirs, allSourcesDirs, executionData, violationRules);
+                } catch (Exception e) {
+                    String violations = getViolations(antBuilder);
+                    jacocoCheckResult.setSuccess(false);
+                    jacocoCheckResult.setFailureMessage(violations != null ? violations : e.getMessage());
+                }
+            }
+        });
+
+        return jacocoCheckResult;
+    }
+
     @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(), "violationsproperty", VIOLATIONS_ANT_PROPERTY);
             antBuilder.invokeMethod("check", new Object[] {checkArgs, new Closure<Object>(this, this) {
                 @SuppressWarnings("UnusedDeclaration")
                 public Object doCall(Object ignore) {
@@ -79,4 +105,10 @@ public Object doCall(Object ignore) {
             }});
         }
     }
+
+    private String getViolations(GroovyObjectSupport antBuilder) {
+        Object project = antBuilder.getProperty("project");
+        Hashtable<String, Object> properties = JavaReflectionUtil.method(project, Hashtable.class, "getProperties").invoke(project, new Object[0]);
+        return (String) properties.get(VIOLATIONS_ANT_PROPERTY);
+    }
 }