diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 72d233bbb..140e07856 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -270,6 +270,33 @@ jobs: :tests:system:resolveAllDependencies \ :services:api:bootJar + - name: Upload the jar for the test jobs + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: api-jar + # `bootJar` carries the `boot` classifier here; the plain `jar` next + # to it is the test-fixtures artifact and does not run. + path: services/api/build/libs/*-boot.jar + retention-days: 1 + if-no-files-found: error + + api-lint: + # ktlint and detekt are source-only: they need the warm Gradle cache + # api-static leaves behind, but nothing needs them. Off the critical path + # they cost their own runner rather than 46s in front of every test job. + name: API formatting and static analysis + needs: api-static + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up Java + Gradle + uses: ./.github/actions/setup-java-gradle + with: + java-version: ${{ env.JAVA_VERSION }} + - name: Check Kotlin formatting # Its own step so a formatting failure reads as one rather than as a # broken compile. @@ -285,21 +312,61 @@ jobs: detektIntegrationTestSourceSet \ detektTestFixturesSourceSet - - name: Upload the jar for the test jobs + api-unit-tests: + # Unit tests need no database at all, so they no longer queue behind the + # integration suite in the same job. 1006 tests in about a minute. + name: API unit tests + runs-on: ubuntu-latest + timeout-minutes: 20 + needs: [ api-static, fe-static ] + + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Install WebP tools + run: sudo apt-get update && sudo apt-get install -y --no-install-recommends webp + + - name: Set up Java + Gradle + uses: ./.github/actions/setup-java-gradle + with: + java-version: ${{ env.JAVA_VERSION }} + + - name: Run API unit tests + run: | + ./gradlew --no-daemon --build-cache \ + :services:api:test \ + :services:api:jacocoTestReport \ + :services:api:jacocoTestCoverageVerification + + - name: Upload API unit test reports + if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: api-jar - # `bootJar` carries the `boot` classifier here; the plain `jar` next - # to it is the test-fixtures artifact and does not run. - path: services/api/build/libs/*-boot.jar - retention-days: 1 - if-no-files-found: error + name: api-unit-test-reports + path: | + services/api/build/reports/jacoco/jacocoTestReport/jacocoTestReport.xml + services/api/build/reports/jacoco/jacocoTestReport/html + services/api/build/reports/tests/test + if-no-files-found: warn - api-tests: - name: API unit and integration tests + api-integration-tests: + # The integration suite is 121 classes over about five minutes, run + # serially in one JVM. Shard it the way :tests:system and the frontend e2e + # suite are sharded: a stable hash of the class name picks the slice, so a + # class always lands on the same shard. + # + # Each shard gets its own MariaDB service, which is what makes the split + # safe — the fixtures collide on unique fields, so the suite cannot share + # one database across parallel writers. + name: API integration tests (shard ${{ matrix.shard }}/6) runs-on: ubuntu-latest timeout-minutes: 30 needs: [ api-static, fe-static ] + strategy: + fail-fast: false + matrix: + shard: [ 1, 2, 3, 4, 5, 6 ] services: db: @@ -345,37 +412,74 @@ jobs: with: java-version: ${{ env.JAVA_VERSION }} - - name: Run API unit and integration tests - run: | - ./gradlew --no-daemon --build-cache \ - :services:api:test \ - :services:api:integrationTest \ - :services:api:jacocoTestReport \ - :services:api:jacocoIntegrationTestReport + # The floors are not run here: a shard's .exec covers only its own slice, + # so no floor holds against one alone. api-coverage merges the six and + # measures the whole picture. + - name: Run API integration tests + env: + SHARD_TOTAL: 6 + SHARD_INDEX: ${{ matrix.shard }} + run: ./gradlew --no-daemon --build-cache :services:api:integrationTest + + - name: Upload this shard's coverage execution data + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: api-integration-exec-${{ matrix.shard }} + path: services/api/build/jacoco/integrationTest.exec + retention-days: 1 + if-no-files-found: error - # The floors have to be named: they are not reached through `check`, and - # running the reports does not run them (#1224). Separate from the step above - # so a coverage failure reads as a coverage failure rather than as a test one, - # and after it so the reports are still uploaded when a floor is missed. - - name: Hold coverage to its floors + - name: Upload API integration test reports + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: api-integration-test-reports-shard-${{ matrix.shard }} + path: services/api/build/reports/tests/integrationTest + if-no-files-found: warn + + api-coverage: + # Fan-in for the sharded integration suite. `-PjacocoExecDir` points the + # report and the floors at every shard's execution data instead of the one + # local file, and lifts the dependency that would otherwise re-run the + # suite here. The floors stay exactly as strict as they were unsharded. + name: API integration coverage + runs-on: ubuntu-latest + timeout-minutes: 20 + needs: api-integration-tests + if: always() && needs.api-integration-tests.result != 'skipped' + + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up Java + Gradle + uses: ./.github/actions/setup-java-gradle + with: + java-version: ${{ env.JAVA_VERSION }} + + - name: Download every shard's coverage execution data + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + path: services/api/build/jacoco/shards + pattern: api-integration-exec-* + + - name: Report on the merged coverage and hold it to its floors run: | ./gradlew --no-daemon --build-cache \ - :services:api:jacocoTestCoverageVerification \ + -PjacocoExecDir=services/api/build/jacoco/shards \ + :services:api:jacocoIntegrationTestReport \ :services:api:jacocoIntegrationTestCoverageVerification - - name: Upload API test reports + - name: Upload API integration coverage report if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: api-test-reports + name: api-integration-coverage path: | - services/api/build/reports/jacoco/jacocoTestReport/jacocoTestReport.xml - services/api/build/reports/jacoco/jacocoTestReport/html services/api/build/reports/jacoco/jacocoIntegrationTestReport/jacocoIntegrationTestReport.xml services/api/build/reports/jacoco/jacocoIntegrationTestReport/html - services/api/build/reports/tests/test - services/api/build/reports/tests/integrationTest - if-no-files-found: warn + if-no-files-found: error build-images: # Builds the production images so a broken Dockerfile is caught on the pull @@ -756,7 +860,10 @@ jobs: - flux-kustomize-check - fe-static - api-static - - api-tests + - api-lint + - api-unit-tests + - api-integration-tests + - api-coverage - build-images - system-tests - acceptance-features diff --git a/build-logic/src/main/kotlin/testing-conventions.gradle.kts b/build-logic/src/main/kotlin/testing-conventions.gradle.kts index 0cb00fab3..762a494af 100644 --- a/build-logic/src/main/kotlin/testing-conventions.gradle.kts +++ b/build-logic/src/main/kotlin/testing-conventions.gradle.kts @@ -1,9 +1,77 @@ +import java.io.File + plugins { java jacoco id("test-logging-conventions") } +// Sharding via env vars `SHARD_TOTAL` / `SHARD_INDEX` (1-based), the same +// partition :tests:system uses: scan the test classes directory sorted by FQCN, +// take `absoluteHash(fqcn) % SHARD_TOTAL`, and include only that slice through +// Gradle's `--tests` filter. A class always lands on the same shard, so a +// failure is easy to place, and leaving the env vars unset runs everything in +// one JVM exactly as before. +fun Test.shardByTestClass() { + val shardTotal = System.getenv("SHARD_TOTAL")?.toIntOrNull()?.takeIf { it > 1 } ?: return + val shardIndex = System.getenv("SHARD_INDEX")?.toIntOrNull()?.takeIf { it in 1..shardTotal } ?: return + // The filter below is applied in doFirst, which Gradle does not hash, so + // without these every shard has identical inputs and one shard's result is + // served to the next FROM-CACHE. Naming them makes each slice its own task. + inputs.property("shardTotal", shardTotal) + inputs.property("shardIndex", shardIndex) + doFirst { + val classes = + testClassesDirs.asFileTree + .matching { include("**/*Test.class", "**/*IT.class") } + .files + .mapNotNull { f -> + val root = testClassesDirs.firstOrNull { f.startsWith(it) } ?: return@mapNotNull null + f.relativeTo(root).path.removeSuffix(".class").replace(File.separatorChar, '.') + }.filter { !it.contains('$') } // skip anonymous / nested $-classes + .sorted() + val mine = classes.filter { Math.floorMod(it.hashCode(), shardTotal) == shardIndex - 1 } + logger.lifecycle("Shard $shardIndex/$shardTotal — ${mine.size}/${classes.size} test classes") + filter { + isFailOnNoMatchingTests = false + if (mine.isEmpty()) { + // No classes assigned — exclude everything by including a pattern + // that cannot match. An empty include list would match all. + includeTestsMatching("__no_match__shard_${shardIndex}__") + } else { + mine.forEach { includeTestsMatching(it) } + } + } + } +} + +// Where a report or a floor reads its execution data from. CI shards the +// integration suite, so each shard's `.exec` covers only its own slice and no +// floor holds against one alone; the fan-in job points this at a directory of +// every shard's file and the merged picture is what gets measured. Unset, it is +// the single local file, which is what a developer's run produces. +val mergedExecDir: String? = providers.gradleProperty("jacocoExecDir").orNull + +fun Project.executionDataFor(defaultFile: String): Any { + val dir = mergedExecDir ?: return layout.buildDirectory.file(defaultFile) + // Resolved against the root, not this project: a relative path would land + // under services/api/ and quietly find nothing. + val resolved = rootProject.file(dir) + val found = fileTree(resolved) { include("**/*.exec") } + // An empty set is not an empty report — JacocoReport goes NO-SOURCE and the + // verification passes with nothing to measure, so a typo in the path reads + // as a green coverage gate. Refuse instead. + check(!found.isEmpty) { "No .exec files under $resolved — the merged coverage would measure nothing." } + return found +} + +// Without merged data a report or a floor runs the suite it measures. With it, +// the suite has already run on the shards and the .exec files are all there is +// to read, so the fan-in must not re-run anything. +fun Task.dependsOnUnlessMerged(producer: Any) { + if (mergedExecDir == null) dependsOn(producer) +} + jacoco { toolVersion = "0.8.14" } @@ -44,12 +112,13 @@ tasks.register("integrationTest") { excludeTags("system", "brevo-live", "discord-live") } shouldRunAfter(tasks.test) + shardByTestClass() } // JaCoCo: unit coverage report — wired to the default `test` task only. tasks.jacocoTestReport { - dependsOn(tasks.test) - executionData.setFrom(layout.buildDirectory.file("jacoco/test.exec")) + dependsOnUnlessMerged(tasks.test) + executionData.setFrom(executionDataFor("jacoco/test.exec")) sourceDirectories.setFrom(sourceSets.main.get().allSource.srcDirs) classDirectories.setFrom( files(sourceSets.main.get().output.classesDirs).asFileTree.matching { @@ -71,8 +140,8 @@ tasks.jacocoTestReport { // JaCoCo: integration coverage report — independent file and HTML output. val jacocoIntegrationTestReport by tasks.registering(JacocoReport::class) { - dependsOn(tasks.named("integrationTest")) - executionData.setFrom(layout.buildDirectory.file("jacoco/integrationTest.exec")) + dependsOnUnlessMerged(tasks.named("integrationTest")) + executionData.setFrom(executionDataFor("jacoco/integrationTest.exec")) sourceDirectories.setFrom(sourceSets.main.get().allSource.srcDirs) classDirectories.setFrom( files(sourceSets.main.get().output.classesDirs).asFileTree.matching { @@ -93,8 +162,8 @@ val jacocoIntegrationTestReport by tasks.registering(JacocoReport::class) { // integration tests by nature touch more code paths but lean on real // infrastructure and are slower to run. tasks.jacocoTestCoverageVerification { - dependsOn(tasks.test) - executionData.setFrom(layout.buildDirectory.file("jacoco/test.exec")) + dependsOnUnlessMerged(tasks.test) + executionData.setFrom(executionDataFor("jacoco/test.exec")) sourceDirectories.setFrom(sourceSets.main.get().allSource.srcDirs) classDirectories.setFrom( files(sourceSets.main.get().output.classesDirs).asFileTree.matching { @@ -116,8 +185,8 @@ tasks.jacocoTestCoverageVerification { } val jacocoIntegrationTestCoverageVerification by tasks.registering(JacocoCoverageVerification::class) { - dependsOn(tasks.named("integrationTest")) - executionData.setFrom(layout.buildDirectory.file("jacoco/integrationTest.exec")) + dependsOnUnlessMerged(tasks.named("integrationTest")) + executionData.setFrom(executionDataFor("jacoco/integrationTest.exec")) sourceDirectories.setFrom(sourceSets.main.get().allSource.srcDirs) classDirectories.setFrom( files(sourceSets.main.get().output.classesDirs).asFileTree.matching {