From e5d06a50d5c0c9d1000d81a57bd162ef920d5f2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jose=20Alc=C3=A9rreca?= <jalc@google.com>
Date: Thu, 21 Mar 2024 14:33:18 +0000
Subject: [PATCH] Small fixes and tweaks to the workflow file

Change-Id: I0b9d4dec4625ebbc769aa32e53f6e2af3603eb1f
---
 .github/workflows/Build.yaml                  | 25 ++++++++++---------
 .../samples/apps/nowinandroid/Jacoco.kt       | 17 +++++++------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml
index 62f038b34..8086118af 100644
--- a/.github/workflows/Build.yaml
+++ b/.github/workflows/Build.yaml
@@ -101,7 +101,7 @@ 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
@@ -128,16 +128,6 @@ jobs:
           name: local-test-results
           path: '**/build/test-results/test*UnitTest/**.xml'
 
-      - name: Upload local coverage reports (XML + HTML)
-        if: always()
-        uses: actions/upload-artifact@v4
-        with:
-          name: coverage-reports
-          if-no-files-found: error
-          compression-level: 1
-          overwrite: false
-          path: '**/build/reports/coverage/*'
-
       - name: Check lint
         run: ./gradlew :app:lintProdRelease :app-nia-catalog:lintRelease :lint:lint
 
@@ -226,7 +216,18 @@ jobs:
         id: jacoco
         uses: madrapps/jacoco-report@v1.6.1
         with:
-          title: Local test coverage report
+          title: Combined test coverage report
           paths: |
             ${{ github.workspace }}/**/build/reports/jacoco/**/*Report.xml
           token: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Upload local coverage reports (XML + HTML)
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-reports
+          if-no-files-found: error
+          compression-level: 1
+          overwrite: false
+          path: |
+            '**/build/reports/jacoco/**/*Report.xml'
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 43a7b193f..3a10db7b0 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
@@ -50,6 +50,15 @@ 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<*, *, *>,
 ) {
@@ -58,8 +67,6 @@ internal fun Project.configureJacoco(
     }
 
     androidComponentsExtension.onVariants { variant ->
-        val testTaskName = "test${variant.name.capitalize()}UnitTest"
-        val androidtestTaskName = "connected${variant.name.capitalize()}AndroidTest"
         val myObjFactory = project.objects
         val buildDir = layout.buildDirectory.get().asFile
         val allJars: ListProperty<RegularFile> = myObjFactory.listProperty(RegularFile::class.java)
@@ -67,12 +74,6 @@ internal fun Project.configureJacoco(
         val reportTask =
             tasks.register("create${variant.name.capitalize()}CombinedCoverageReport", JacocoReport::class) {
 
-//                if (tasks.findByName(testTaskName) != null) {
-//                    dependsOn(testTaskName)
-//                }
-//                if (tasks.findByName(androidtestTaskName) != null) {
-//                    dependsOn(androidtestTaskName)
-//                }
                 classDirectories.setFrom(
                     allJars,
                     allDirectories.map { dirs ->