From 78e3725f9fb01df9c250c6983144fdb6f5abdb14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Alc=C3=A9rreca?= Date: Mon, 25 Mar 2024 18:51:44 +0100 Subject: [PATCH] Sets up jacoco coverage with a combined report and a GHA (#1303) * Uses jacoco coverage reporting in AGP --- .github/workflows/Build.yaml | 45 +++++++++-- app/build.gradle.kts | 1 - ...ndroidApplicationJacocoConventionPlugin.kt | 17 +++-- .../AndroidLibraryJacocoConventionPlugin.kt | 18 +++-- .../samples/apps/nowinandroid/Jacoco.kt | 74 +++++++++++++------ gradle.properties | 3 + 6 files changed, 115 insertions(+), 43 deletions(-) diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml index 001140a87..db326c380 100644 --- a/.github/workflows/Build.yaml +++ b/.github/workflows/Build.yaml @@ -17,6 +17,7 @@ jobs: permissions: contents: write + pull-requests: write timeout-minutes: 60 @@ -100,12 +101,13 @@ jobs: commit_message: "🤖 Updates screenshots" # Run local tests after screenshot tests to avoid wrong UP-TO-DATE. TODO: Ignore screenshots. - - name: Run local tests + - name: Run local tests and create report if: always() run: ./gradlew testDemoDebug :lint:test # Replace task exclusions with `-Pandroidx.baselineprofile.skipgeneration` when # https://android-review.googlesource.com/c/platform/frameworks/support/+/2602790 landed in a # release build + - name: Build all build type and flavor permutations run: ./gradlew :app:assemble :benchmarks:assemble -x pixel6Api33ProdNonMinifiedReleaseAndroidTest @@ -119,11 +121,11 @@ jobs: name: APKs path: '**/build/outputs/apk/**/*.apk' - - name: Upload test results (XML) + - name: Upload JVM local results (XML) if: always() uses: actions/upload-artifact@v4 with: - name: test-results + name: local-test-results path: '**/build/test-results/test*UnitTest/**.xml' - name: Check lint @@ -180,10 +182,7 @@ jobs: - name: Setup Gradle uses: gradle/gradle-build-action@v3 - - name: Build projects before running emulator - run: ./gradlew packageDemoDebug packageDemoDebugAndroidTest - - - name: Run instrumentation tests + - name: Build projects and run instrumentation tests uses: reactivecircus/android-emulator-runner@v2 with: api-level: ${{ matrix.api-level }} @@ -193,9 +192,41 @@ jobs: heap-size: 600M script: ./gradlew connectedDemoDebugAndroidTest --daemon + - name: Run local tests (including Roborazzi) for the combined coverage report (only API 30) + if: matrix.api-level == 30 + # There is no need to verify Roborazzi tests to generate coverage. + run: ./gradlew testDemoDebugUnitTest -Proborazzi.test.verify=false # Add Prod if we ever add JVM tests for prod + + # Add `createProdDebugUnitTestCoverageReport` if we ever add JVM tests for prod + - name: Generate coverage reports for Debug variants (only API 30) + if: matrix.api-level == 30 + run: ./gradlew createDemoDebugCombinedCoverageReport + - name: Upload test reports if: always() uses: actions/upload-artifact@v4 with: name: test-reports-${{ matrix.api-level }} path: '**/build/reports/androidTests' + + - name: Display local test coverage (only API 30) + if: matrix.api-level == 30 + id: jacoco + uses: madrapps/jacoco-report@v1.6.1 + with: + title: Combined test coverage report + min-coverage-overall: 40 + min-coverage-changed-files: 60 + paths: | + ${{ github.workspace }}/**/build/reports/jacoco/**/*Report.xml + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Upload local coverage reports (XML + HTML) (only API 30) + if: matrix.api-level == 30 + uses: actions/upload-artifact@v4 + with: + name: coverage-reports + if-no-files-found: error + compression-level: 1 + overwrite: false + path: '**/build/reports/jacoco/' diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 9e68ffa7d..2fe18645e 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -21,7 +21,6 @@ plugins { alias(libs.plugins.nowinandroid.android.application.flavors) alias(libs.plugins.nowinandroid.android.application.jacoco) alias(libs.plugins.nowinandroid.android.hilt) - id("jacoco") alias(libs.plugins.nowinandroid.android.application.firebase) id("com.google.android.gms.oss-licenses-plugin") alias(libs.plugins.baselineprofile) diff --git a/build-logic/convention/src/main/kotlin/AndroidApplicationJacocoConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidApplicationJacocoConventionPlugin.kt index 4c3acc520..ac385b0d9 100644 --- a/build-logic/convention/src/main/kotlin/AndroidApplicationJacocoConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidApplicationJacocoConventionPlugin.kt @@ -15,6 +15,7 @@ */ import com.android.build.api.variant.ApplicationAndroidComponentsExtension +import com.android.build.gradle.internal.dsl.BaseAppModuleExtension import com.google.samples.apps.nowinandroid.configureJacoco import org.gradle.api.Plugin import org.gradle.api.Project @@ -23,13 +24,15 @@ import org.gradle.kotlin.dsl.getByType class AndroidApplicationJacocoConventionPlugin : Plugin { override fun apply(target: Project) { with(target) { - with(pluginManager) { - apply("org.gradle.jacoco") - apply("com.android.application") + pluginManager.apply("jacoco") + val androidExtension = extensions.getByType() + + androidExtension.buildTypes.configureEach { + enableAndroidTestCoverage = true + enableUnitTestCoverage = true } - val extension = extensions.getByType() - configureJacoco(extension) + + configureJacoco(extensions.getByType()) } } - -} \ No newline at end of file +} diff --git a/build-logic/convention/src/main/kotlin/AndroidLibraryJacocoConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidLibraryJacocoConventionPlugin.kt index 86ca091c3..6f2ff60c5 100644 --- a/build-logic/convention/src/main/kotlin/AndroidLibraryJacocoConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidLibraryJacocoConventionPlugin.kt @@ -14,6 +14,8 @@ * limitations under the License. */ +import com.android.build.api.dsl.LibraryExtension +import com.android.build.api.variant.ApplicationAndroidComponentsExtension import com.android.build.api.variant.LibraryAndroidComponentsExtension import com.google.samples.apps.nowinandroid.configureJacoco import org.gradle.api.Plugin @@ -23,13 +25,15 @@ import org.gradle.kotlin.dsl.getByType class AndroidLibraryJacocoConventionPlugin : Plugin { override fun apply(target: Project) { with(target) { - with(pluginManager) { - apply("org.gradle.jacoco") - apply("com.android.library") + pluginManager.apply("jacoco") + val androidExtension = extensions.getByType() + + androidExtension.buildTypes.configureEach { + enableAndroidTestCoverage = true + enableUnitTestCoverage = true } - val extension = extensions.getByType() - configureJacoco(extension) + + configureJacoco(extensions.getByType()) } } - -} \ No newline at end of file +} diff --git a/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Jacoco.kt b/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Jacoco.kt index 596c4f579..7820a978e 100644 --- a/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Jacoco.kt +++ b/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Jacoco.kt @@ -1,5 +1,5 @@ /* - * Copyright 2022 The Android Open Source Project + * Copyright 2024 The Android Open Source Project * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,8 +16,13 @@ package com.google.samples.apps.nowinandroid +import com.android.build.api.artifact.ScopedArtifact import com.android.build.api.variant.AndroidComponentsExtension +import com.android.build.api.variant.ScopedArtifacts import org.gradle.api.Project +import org.gradle.api.file.Directory +import org.gradle.api.file.RegularFile +import org.gradle.api.provider.ListProperty import org.gradle.api.tasks.testing.Test import org.gradle.kotlin.dsl.configure import org.gradle.kotlin.dsl.register @@ -32,13 +37,24 @@ private val coverageExclusions = listOf( "**/R.class", "**/R\$*.class", "**/BuildConfig.*", - "**/Manifest*.*" + "**/Manifest*.*", + "**/*_Hilt*.class", + "**/Hilt_*.class", ) private fun String.capitalize() = replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.getDefault()) else it.toString() } +/** + * Creates a new task that generates a combined coverage report with data from local and + * instrumented tests. + * + * `create{variant}CombinedCoverageReport` + * + * Note that coverage data must exist before running the task. This allows us to run device + * tests on CI using a different Github Action or an external device farm. + */ internal fun Project.configureJacoco( androidComponentsExtension: AndroidComponentsExtension<*, *, *>, ) { @@ -46,37 +62,53 @@ internal fun Project.configureJacoco( toolVersion = libs.findVersion("jacoco").get().toString() } - val jacocoTestReport = tasks.create("jacocoTestReport") - androidComponentsExtension.onVariants { variant -> - val testTaskName = "test${variant.name.capitalize()}UnitTest" + val myObjFactory = project.objects val buildDir = layout.buildDirectory.get().asFile - val reportTask = tasks.register("jacoco${testTaskName.capitalize()}Report", JacocoReport::class) { - dependsOn(testTaskName) + val allJars: ListProperty = myObjFactory.listProperty(RegularFile::class.java) + val allDirectories: ListProperty = myObjFactory.listProperty(Directory::class.java) + val reportTask = + tasks.register("create${variant.name.capitalize()}CombinedCoverageReport", JacocoReport::class) { - reports { - xml.required.set(true) - html.required.set(true) - } - - classDirectories.setFrom( - fileTree("$buildDir/tmp/kotlin-classes/${variant.name}") { - exclude(coverageExclusions) + classDirectories.setFrom( + allJars, + allDirectories.map { dirs -> + dirs.map { dir -> + myObjFactory.fileTree().setDir(dir).exclude(coverageExclusions) + } + } + ) + reports { + xml.required.set(true) + html.required.set(true) } - ) - sourceDirectories.setFrom(files("$projectDir/src/main/java", "$projectDir/src/main/kotlin")) - executionData.setFrom(file("$buildDir/jacoco/$testTaskName.exec")) - } + // TODO: This is missing files in src/debug/, src/prod, src/demo, src/demoDebug... + sourceDirectories.setFrom(files("$projectDir/src/main/java", "$projectDir/src/main/kotlin")) + + executionData.setFrom( + project.fileTree("$buildDir/outputs/unit_test_code_coverage/${variant.name}UnitTest") + .matching { include("**/*.exec") }, + + project.fileTree("$buildDir/outputs/code_coverage/${variant.name}AndroidTest") + .matching { include("**/*.ec") } + ) + } + - jacocoTestReport.dependsOn(reportTask) + variant.artifacts.forScope(ScopedArtifacts.Scope.PROJECT) + .use(reportTask) + .toGet( + ScopedArtifact.CLASSES, + { _ -> allJars }, + { _ -> allDirectories }, + ) } tasks.withType().configureEach { configure { // Required for JaCoCo + Robolectric // https://github.com/robolectric/robolectric/issues/2230 - // TODO: Consider removing if not we don't add Robolectric isIncludeNoLocationClasses = true // Required for JDK 11 with the above diff --git a/gradle.properties b/gradle.properties index c0acfeb02..97f940e2e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -39,3 +39,6 @@ kotlin.code.style=official # https://developer.android.com/build/releases/gradle-plugin#default-changes android.defaults.buildfeatures.resvalues=false android.defaults.buildfeatures.shaders=false + +# Run Roborazzi screenshot tests with the local tests +roborazzi.test.verify=true