Integrate instrumentation tests #131

vRallev · 2018-10-11T00:06:10Z

Generate a task that runs unit tests and instrumentation tests for a module and combines their execution data in a single report. Add the option to include instrumentation tests in the global merged report.

…module and combines their execution data in a single report. Add the option to include instrumentation tests in the global merged report.

vRallev · 2018-10-11T00:06:46Z

src/main/groovy/com/vanniktech/android/junit/jacoco/GenerationPlugin.groovy

@@ -115,6 +115,10 @@ class GenerationPlugin implements Plugin<Project> {
            it.jacoco.includeNoLocationClasses = extension.includeNoLocationClasses
        }

+        if (extension.configureInstrumentationCoverage) {
+            subProject.android.jacoco.version = extension.jacocoVersion


This sets the Jacoco version for instrumentation tests

vRallev · 2018-10-11T00:07:44Z

src/main/groovy/com/vanniktech/android/junit/jacoco/GenerationPlugin.groovy

+            if (combined) {
+                // add instrumentation coverage execution data
+                executionData += subProject.fileTree("${subProject.buildDir}/outputs/code_coverage").matching {
+                    include "**/*.ec"
                }


This adds the execution data from instrumentation tests to the report

vRallev · 2018-10-11T00:08:32Z

src/main/groovy/com/vanniktech/android/junit/jacoco/JunitJacocoExtension.groovy

+     * value is true.
+     * @since 0.13.0
+     */
+    boolean configureInstrumentationCoverage = true


This is enabled by default, but note that you need to manually set testCoverageEnabled true nonetheless

Can we get it to set the testCoverageEnabled too?

vRallev · 2018-10-11T00:08:51Z

src/main/groovy/com/vanniktech/android/junit/jacoco/JunitJacocoExtension.groovy

+     * Note that this will run all instrumentation tests when true.
+     * @since 0.13.0
+     */
+    boolean includeInstrumentationCoverageInMergedReport = false


Since this can be really heavy, I thought it's safer to turn this off by default

Yup agreed. I also think the majority won't need a single report

vRallev · 2018-10-11T00:09:45Z

src/main/groovy/com/vanniktech/android/junit/jacoco/GenerationPlugin.groovy

+    private static void addJacocoTask(final boolean combined, final Project subProject, final JunitJacocoExtension extension,
+                                      JacocoMerge mergeTask, JacocoReport mergedReportTask, final String taskName,
+                                      final String jvmTestTaskName, final String instrumentationTestTaskName, final String sourceName,
+                                      final String sourcePath, final String productFlavorName, final String buildTypeName) {


It's ugly to pass all these arguments, but I wanted to make the task creation reusable.

codecov · 2018-10-11T00:10:08Z

Codecov Report

Merging #131 into master will increase coverage by 3.16%.
The diff coverage is 84.48%.

@@             Coverage Diff              @@
##             master     #131      +/-   ##
============================================
+ Coverage     63.69%   66.85%   +3.16%     
- Complexity       34       36       +2     
============================================
  Files             1        1              
  Lines           157      175      +18     
  Branches         20       25       +5     
============================================
+ Hits            100      117      +17     
  Misses           46       46              
- Partials         11       12       +1

Impacted Files	Coverage Δ	Complexity Δ
...ktech/android/junit/jacoco/GenerationPlugin.groovy	`66.85% <84.48%> (+3.16%)`	`36 <2> (+2)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 331fa7b...7e5fa38. Read the comment docs.

vanniktech

Really liking it

vanniktech · 2018-10-11T09:48:31Z

README.md

 - Task `jacocoTestReport<Flavor><BuildType>`
  - Executes the `test<Flavor><BuildType>UnitTest` task before
  - Gets executed when the `check` task is executed
  - Generated Jacoco reports can be found under `build/reports/jacoco/<Flavor>/<BuildType>`.

+*Instrumentation tests*
+- Task `combinedTestReport<Flavor><BuildType>`
+  - Executes the `test<Flavor><BuildType>UnitTest` and `create<Flavor><BuildType>CoverageReports` tasks before (JVM and instrumentation tests)


also capital Instrumentation?

The documentation speaks of instrumented tests (lowercase). I'll use that.

vanniktech · 2018-10-11T09:48:47Z

README.md

+  - Executes the `test<Flavor><BuildType>UnitTest` and `create<Flavor><BuildType>CoverageReports` tasks before (JVM and instrumentation tests)
+  - Gets executed when the `check` task is executed
+  - Generated Jacoco reports can be found under `build/reports/jacocoCombined/<Flavor>/<BuildType>`.
+Note that this task is only generated, if you set `testCoverageEnabled = true` for your [build type](https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.BuildType.html#com.android.build.gradle.internal.dsl.BuildType:testCoverageEnabled), e.g.


Ah that makes sense!

README.md

vanniktech · 2018-10-11T09:50:11Z

src/main/groovy/com/vanniktech/android/junit/jacoco/GenerationPlugin.groovy

-                ]
+            def classPaths = [
+                "**/intermediates/classes/${sourcePath}/**",
+                "**/intermediates/javac/${sourceName}/*/classes/**" // Android Gradle Plugin 3.2.x support.


Ah here we already support AGP 3.2.x

vanniktech · 2018-10-11T09:51:04Z

src/test/groovy/com/vanniktech/android/junit/jacoco/ProjectHelper.groovy

 import com.android.builder.model.BuildType
 import groovy.mock.interceptor.MockFor
 import org.gradle.api.Project
 import org.gradle.testfixtures.ProjectBuilder
-


nit: can we keep this new line?

I think the provided editor config is messing with this.

vanniktech · 2018-10-11T09:52:48Z

Regarding configureInstrumentationCoverage and testCoverageEnabled, can we get rid of configureInstrumentationCoverage and just use the testCoverageEnabled. That way consumers can define where they want things to be added or not and they're not forced to be using 2 options.

vRallev · 2018-10-11T16:01:20Z

Yes, that makes sense. I'll remove configureInstrumentationCoverage

vanniktech

Superb

langerhans · 2018-10-16T14:25:02Z

Maybe I'm missing something but this breaks my pipeline currently. Before I had two test steps in my pipeline. I'd run ./gradlew :app:connectedDebugAndroidTest :app:createDebugCoverageReport [... more modules] and in the following step ./gradlew mergeJacocoReports. The first part would execute only the Android instrumentation tests, second one would execute only the unit tests and then merge the reports of both runs.
But now mergeJacocoReports will also execute instrumentation tests. It's not clear to me how to reproduce the old behavior after this PR.
Reason to do it that way is that we have a pretty large instrumentation test set and use lockable resources to not block our jenkins slots too much.

vRallev · 2018-10-16T19:43:24Z

@langerhans AFAIK createDebugCoverageReport runs instrumentation tests, too. But I see, that now mergeJacocoReports runs the instrumentation tests as well.

@vanniktech I agree that this should be optional. What do you think?

vanniktech · 2018-10-16T20:17:34Z

createDebugCoverageReport runs the tests - yeah. AFAIK.

@vanniktech I agree that this should be optional. What do you think?

I guess so. mergeJacocoReports should only take the outputs then and running them should not be a dependency. (Should be consistent between Android + Java projects)

vRallev · 2018-10-16T20:21:20Z

Do you mean running unit tests AND instrumentation tests should be optional? I would agree. I can take a look, it shouldn't be hard.

vanniktech · 2018-10-16T20:23:11Z

Yes. I'd assume then mergeJacocoReports only merges them (as the name states) and you need to take care the reports are present (running instrumentation tests / running tests manually).

vRallev · 2018-10-16T20:23:57Z

Yes, let me prepare something.

langerhans · 2018-10-17T06:07:11Z

Thanks for the quick replies! Also thanks for the hint about the duplicate tasks, I guess that was changed at some point. I have for now downgraded the plugin but I'll keep an eye on the releases here 👍

vRallev · 2018-10-18T00:04:26Z

See #134

Generate a task that runs unit tests and instrumentation tests for a …

c6481b5

…module and combines their execution data in a single report. Add the option to include instrumentation tests in the global merged report.

vRallev commented Oct 11, 2018

View reviewed changes

Verify that the Jacoco version is set for instrumentation tests

821bbb2

vanniktech approved these changes Oct 11, 2018

View reviewed changes

vRallev added 2 commits October 11, 2018 09:11

Remove the configureInstrumentationCoverage flag

789ef57

Add missing space

7e5fa38

vanniktech approved these changes Oct 11, 2018

View reviewed changes

vRallev merged commit 1950af8 into vanniktech:master Oct 11, 2018

vRallev deleted the task/instrumentation-coverage branch October 11, 2018 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate instrumentation tests #131

Integrate instrumentation tests #131

vRallev commented Oct 11, 2018

vRallev Oct 11, 2018

vRallev Oct 11, 2018

vRallev Oct 11, 2018

vanniktech Oct 11, 2018

vRallev Oct 11, 2018

vanniktech Oct 11, 2018

vRallev Oct 11, 2018

codecov bot commented Oct 11, 2018 •

edited

Loading

vanniktech left a comment

vanniktech Oct 11, 2018

vRallev Oct 11, 2018

vanniktech Oct 11, 2018

vanniktech Oct 11, 2018

vanniktech Oct 11, 2018

vRallev Oct 11, 2018

vanniktech commented Oct 11, 2018

vRallev commented Oct 11, 2018

vanniktech left a comment

langerhans commented Oct 16, 2018

vRallev commented Oct 16, 2018

vanniktech commented Oct 16, 2018

vRallev commented Oct 16, 2018

vanniktech commented Oct 16, 2018

vRallev commented Oct 16, 2018

langerhans commented Oct 17, 2018

vRallev commented Oct 18, 2018

Integrate instrumentation tests #131

Integrate instrumentation tests #131

Conversation

vRallev commented Oct 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 11, 2018 • edited Loading

Codecov Report

vanniktech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanniktech commented Oct 11, 2018

vRallev commented Oct 11, 2018

vanniktech left a comment

Choose a reason for hiding this comment

langerhans commented Oct 16, 2018

vRallev commented Oct 16, 2018

vanniktech commented Oct 16, 2018

vRallev commented Oct 16, 2018

vanniktech commented Oct 16, 2018

vRallev commented Oct 16, 2018

langerhans commented Oct 17, 2018

vRallev commented Oct 18, 2018

codecov bot commented Oct 11, 2018 •

edited

Loading