From ca0e7ac1c639f62f5200ada4909ba9b9c34c9322 Mon Sep 17 00:00:00 2001 From: Roy Dahan Date: Mon, 17 Aug 2026 18:00:01 +0300 Subject: [PATCH 1/4] Add code coverage measurement for unit and integration tests jacoco-maven-plugin was already declared in the parent pom (prepare-agent + report bound to every module), but two things kept it from doing anything useful: 1. core/pom.xml and integration-tests/pom.xml (surefire and failsafe, respectively) set to just their own JVM flags (${mockitoopens.argline} / ${blockhound.argline}), completely replacing rather than combining with the value jacoco:prepare-agent injects into that property. Confirmed empirically: before this fix, running core's unit tests never wrote core/target/jacoco.exec at all -- the -javaagent flag jacoco set up never reached the forked test JVM. Fixed by combining both via Maven's deferred-property syntax, `@{argLine} ${mockitoopens.argline}` (`@{...}` rather than `${...}` because prepare-agent sets `argLine` at build-execution time, after the POM's own `${...}` references would already have been resolved). distribution-tests/pom.xml had the same bug and got the same fix, for the modules that do have real jacoco data. 2. Nothing merged the resulting per-module jacoco.exec files into one cross-module view -- coverage `core` gets exercised through the integration suite, for instance, was never attributed back to core's own source. Added a new `coverage-report` module (packaging pom, depends on core/query-builder/mapper-runtime/mapper-processor/ metrics-micrometer/metrics-microprofile/integration-tests) that runs jacoco:report-aggregate over all of them. Makefile: `test-unit-coverage` is a new target, not a coverage-flavored variant of the existing `test-unit`. It has to test each module as its own `mvn` invocation rather than one reactor-wide `mvn test`: in a single reactor build, a test failure in core makes Maven skip every module depending on it (query-builder, mapper-runtime, ...) too, losing their coverage data along with core's -- confirmed empirically, and unaffected by -fae/-fn, since those only rescue independent modules in the reactor, not ones with a real dependency on the failed one. test-integration-scylla/test-integration-cassandra need no such variant: maven-failsafe-plugin already separates running ITs (integration-test phase, which always completes) from failing the build on their results (verify phase), so a test failure there was never able to lose coverage data to begin with. `coverage-report` merges and renders whatever the above collected; `clean-coverage` resets it. CI (.github/workflows/coverage.yml) runs this against a single canonical ScyllaDB version on every push/PR, posts a summary to the job log, and uploads the HTML/XML/CSV reports as a build artifact -- surfaced this way instead of through a third-party service like Codecov, matching the choice already made for the Python, Go, and Rust drivers' equivalent tooling this session. Fixes: https://scylladb.atlassian.net/browse/DRIVER-891 Co-Authored-By: Claude Sonnet 5 --- .github/workflows/coverage.yml | 130 +++++++++++++++++++++++++++++++++ Makefile | 39 ++++++++++ README-dev.md | 35 +++++++++ core/pom.xml | 2 +- coverage-report/pom.xml | 101 +++++++++++++++++++++++++ distribution-tests/pom.xml | 2 +- integration-tests/pom.xml | 6 +- pom.xml | 3 +- 8 files changed, 312 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/coverage.yml create mode 100644 coverage-report/pom.xml diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml new file mode 100644 index 00000000000..45d5a692e78 --- /dev/null +++ b/.github/workflows/coverage.yml @@ -0,0 +1,130 @@ +name: Code coverage + +permissions: + contents: read + +on: + push: + branches: [ scylla-4.*x ] + paths-ignore: + - "docs/**" + - .github/workflows/docs-pages.yml + - .github/workflows/docs-pr.yml + - .github/workflows/release.yml + - "*.md" + - "*.sh" + - "renovate.json" + - ".snyk" + - ".gitignore" + pull_request: + branches: [ scylla-4.*x ] + paths-ignore: + - "docs/**" + - .github/workflows/docs-pages.yml + - .github/workflows/docs-pr.yml + - .github/workflows/release.yml + - "*.md" + - "*.sh" + - "renovate.json" + - ".snyk" + - ".gitignore" + workflow_dispatch: + +jobs: + coverage: + name: Measure code coverage + runs-on: ubuntu-latest + timeout-minutes: 60 + + steps: + - name: Checkout source + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false + + - name: Set up JDK 17 + uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0 + with: + java-version: 17 + distribution: 'temurin' + + - name: Restore maven repository cache + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.m2/repository + key: ${{ runner.os }}-17-maven-${{ hashFiles('**/pom.xml') }} + + - name: Setup Python 3 + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: '3.13' + + # From here on, every step uses `if: always()`: a failing test must not + # skip the integration suite or report generation -- a broken test + # would otherwise leave no coverage output at all to diagnose it with. + # test-unit-coverage (rather than test-unit) tests each module as its + # own `mvn` invocation for the same reason: in one reactor-wide `mvn + # test`, a failure in core would make Maven skip every module that + # depends on it too, losing their coverage data along with core's own. + - name: Run unit tests with coverage + if: always() + run: make test-unit-coverage + + - name: Install ScyllaDB CCM + if: always() + run: make install-scylla-ccm + + - name: Get scylla version + id: scylla-version + if: always() + env: + GH_TOKEN: ${{ github.token }} + run: make resolve-scylla-version + + - name: Pull CCM image from the cache + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + if: always() + id: ccm-cache + with: + path: ~/.ccm/scylla-repository + key: ccm-scylla-${{ runner.os }}-${{ steps.scylla-version.outputs.value }} + + - name: Download Scylla (${{ steps.scylla-version.outputs.value }}) image + if: always() && steps.ccm-cache.outputs.cache-hit != 'true' + env: + SCYLLA_VERSION_RESOLVED: ${{ steps.scylla-version.outputs.value }} + run: make download-scylla + + - name: Save CCM image into the cache + if: always() && steps.ccm-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.ccm/scylla-repository + key: ccm-scylla-${{ runner.os }}-${{ steps.scylla-version.outputs.value }} + + - name: Run integration tests with coverage + if: always() + env: + SCYLLA_VERSION_RESOLVED: ${{ steps.scylla-version.outputs.value }} + run: make test-integration-scylla + + - name: Generate coverage report + if: always() + run: make coverage-report MVNCMD="mvn -B -ntp" + + - name: Publish coverage summary + if: always() + run: | + { + echo '## Coverage report' + echo '```' + awk -F, 'NR>1 {missed+=$8; covered+=$9} END {total=missed+covered; if (total>0) printf "Line coverage: %d/%d (%.1f%%)\n", covered, total, 100*covered/total; else print "No coverage data"}' coverage-report/target/site/jacoco-aggregate/jacoco.csv + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: coverage-report + path: coverage-report/target/site/jacoco-aggregate diff --git a/Makefile b/Makefile index ba74a9af677..d804e0b5275 100644 --- a/Makefile +++ b/Makefile @@ -257,6 +257,27 @@ fix: test-unit: .install-guava-shaded $(MVNCMD) test -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true +# Modules with real unit tests. Kept as an explicit list (rather than e.g. +# "every module but X") since test-unit-coverage below relies on it precisely +# to sidestep Maven's reactor dependency rules. +UNIT_TEST_MODULES := core query-builder mapper-runtime mapper-processor metrics/micrometer metrics/microprofile + +# Unlike test-unit, this tests each module as its own `mvn` invocation instead +# of one reactor-wide `mvn test`. That matters here specifically: in a single +# reactor build, if core's tests fail, Maven skips every module that depends +# on core (query-builder, mapper-runtime, ...) *regardless* of -fae/-fn -- +# those flags only rescue independent modules, not ones with a real +# dependency on the failed one. Testing each module separately, against +# core's already-installed (.install-all-modules) artifact, means one +# module's test failure can only cost that module's own coverage data, not +# every other module's too. +test-unit-coverage: .install-all-modules + @status=0 + for module in $(UNIT_TEST_MODULES); do + $(MVNCMD) test -pl $$module -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true || status=1 + done + exit $$status + test-integration-scylla: .install-all-modules .prepare-scylla-ccm resolve-scylla-version .prepare-environment-update-aio-max-nr @if [[ -z "$${SCYLLA_VERSION_RESOLVED}" ]]; then SCYLLA_VERSION_RESOLVED=`cat '${SCYLLA_VERSION_FILE}'` @@ -277,6 +298,24 @@ test-integration-cassandra: .install-all-modules .prepare-scylla-ccm resolve-cas fi mvn -B -e verify -pl integration-tests -Dccm.version=$${CASSANDRA_VERSION_RESOLVED} -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true $(MAVEN_EXTRA_ARGS) +# jacoco-maven-plugin's prepare-agent/report executions (bound repo-wide by the +# parent pom) already instrument test-unit/test-integration-* runs -- run +# whichever of those you want measured first, then this to merge and render +# them. It rebuilds the reactor (-am) to resolve coverage-report's +# dependencies, but -DskipTests means that rebuild does not re-run (or +# overwrite the coverage data from) any module's tests. +# +# .PHONY here (unlike the rest of this file) because these target names +# collide with real paths -- coverage-report/ is the module's own directory -- +# so make would otherwise treat the target as already up to date and skip it. +.PHONY: coverage-report clean-coverage +coverage-report: + $(MVNCMD) verify -pl coverage-report -am -DskipTests -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true + +clean-coverage: + find . -name jacoco.exec -delete + rm -rf coverage-report/target/site + check-no-compile-warnings: @$(MAKE) compile-all | grep WARNING >/tmp/all-compile-warnings.log || true if [ -s /tmp/all-compile-warnings.log ]; then diff --git a/README-dev.md b/README-dev.md index 07a4673f8a2..114f6bcd1da 100644 --- a/README-dev.md +++ b/README-dev.md @@ -27,4 +27,39 @@ Most day-to-day tasks are wrapped in the top-level `Makefile` so you do not have - `make fix` executes `mvn fmt:format` to format the code. - `make clean` removes Maven targets, shaded artifacts, and release backups to reset the tree. +### Measuring code coverage + +`jacoco-maven-plugin` is already wired into every module (see the root `pom.xml`), so +`test-unit`/`test-integration-*` collect coverage data as a side effect of running normally -- +each module writes its own `target/jacoco.exec`. What's missing without the targets below is a +combined, cross-module view (e.g. attributing coverage `core` gets *through* the integration +suite back to `core`'s own source), and, for unit tests specifically, resilience to one module's +test failure discarding every other module's data: + +- `make test-unit-coverage` runs the same modules as `make test-unit`, but as one `mvn` + invocation per module instead of a single reactor-wide one. This matters because in a single + `mvn test` reactor build, a test failure in `core` makes Maven skip every module that depends on + it (`query-builder`, `mapper-runtime`, ...) too -- losing their coverage data along with core's, + regardless of `-fae`/`-fn`, since those flags only rescue *independent* modules, not ones with + a real dependency on the failed one. +- `make test-integration-scylla` / `make test-integration-cassandra` need no coverage-specific + variant: `maven-failsafe-plugin` already separates running integration tests (`integration-test` + phase, which always completes) from failing the build on their results (`verify` phase), so a + test failure there was never able to lose coverage data in the first place. +- `make coverage-report` merges whatever the above collected into one cross-module report, via a + dedicated `coverage-report` module that depends on the others and runs + `jacoco:report-aggregate`. Run whichever of `test-unit-coverage` / `test-integration-scylla` / + `test-integration-cassandra` you want measured first, then this. It prints a per-module summary + and writes an HTML report to `coverage-report/target/site/jacoco-aggregate/index.html` (open it + in a browser for a line-by-line view), plus `jacoco.xml`/`jacoco.csv` alongside it. +- `make clean-coverage` deletes every module's `jacoco.exec` and the generated report. + +Note: the surefire/failsafe configs in `core`, `integration-tests`, and `distribution-tests` +previously set `` to just their own JVM flags (e.g. `${mockitoopens.argline}`), which +silently discarded the `-javaagent` flag `jacoco:prepare-agent` injects into the `argLine` +property -- coverage was being collected for every *other* module, but not these three. They now +combine both via Maven's deferred-property syntax: `@{argLine} ${mockitoopens.argline}` +(`@{...}` is necessary rather than `${...}` because `jacoco:prepare-agent` sets `argLine` at +build-execution time, after the POM's own `${...}` references would already have been resolved). + The Makefile automatically installs the shaded Guava dependency and, for integration tests, bootstraps the appropriate CCM toolchain and raises kernel `aio-max-nr` when required. If a target fails because the toolchain is missing, rerun after installing the prerequisites highlighted in the target output. diff --git a/core/pom.xml b/core/pom.xml index 13d93bf56e1..a41afc98f0b 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -261,7 +261,7 @@ maven-surefire-plugin ${testing.jvm}/bin/java - ${mockitoopens.argline} + @{argLine} ${mockitoopens.argline} 1 diff --git a/coverage-report/pom.xml b/coverage-report/pom.xml new file mode 100644 index 00000000000..ec5ebb2a5d0 --- /dev/null +++ b/coverage-report/pom.xml @@ -0,0 +1,101 @@ + + + + + 4.0.0 + + com.scylladb + java-driver-parent + 4.19.2.1-SNAPSHOT + + java-driver-coverage-report + pom + Java driver for Scylla and Apache Cassandra(R) - coverage report + + + + com.scylladb + java-driver-core + + + com.scylladb + java-driver-query-builder + + + com.scylladb + java-driver-mapper-runtime + + + com.scylladb + java-driver-mapper-processor + + + com.scylladb + java-driver-metrics-micrometer + + + com.scylladb + java-driver-metrics-microprofile + + + com.scylladb + java-driver-integration-tests + ${project.version} + + + + + + org.jacoco + jacoco-maven-plugin + + + + default + none + + + report + none + + + report-aggregate + verify + + report-aggregate + + + + + + + diff --git a/distribution-tests/pom.xml b/distribution-tests/pom.xml index 2e8060a3050..9004782c135 100644 --- a/distribution-tests/pom.xml +++ b/distribution-tests/pom.xml @@ -81,7 +81,7 @@ maven-surefire-plugin ${testing.jvm}/bin/java - ${mockitoopens.argline} + @{argLine} ${mockitoopens.argline} 1 diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index 8d1f5ae67b1..040d0679d76 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -283,7 +283,7 @@ ${test.parallel.threads} ${project.build.directory}/failsafe-reports/failsafe-summary-parallelized.xml ${skipParallelizableITs} - ${blockhound.argline} + @{argLine} ${blockhound.argline} ${testing.jvm}/bin/java @@ -296,7 +296,7 @@ com.datastax.oss.driver.categories.ParallelizableTests, com.datastax.oss.driver.categories.IsolatedTests ${project.build.directory}/failsafe-reports/failsafe-summary-serial.xml ${skipSerialITs} - ${blockhound.argline} + @{argLine} ${blockhound.argline} ${testing.jvm}/bin/java @@ -312,7 +312,7 @@ false ${project.build.directory}/failsafe-reports/failsafe-summary-isolated.xml ${skipIsolatedITs} - ${blockhound.argline} + @{argLine} ${blockhound.argline} ${testing.jvm}/bin/java diff --git a/pom.xml b/pom.xml index 78e3fa6a69b..25eb2b94974 100644 --- a/pom.xml +++ b/pom.xml @@ -50,6 +50,7 @@ distribution-tests examples bom + coverage-report UTF-8 @@ -712,7 +713,7 @@ true central - java-driver-distribution-source,java-driver-distribution-tests,java-driver-distribution,java-driver-examples,java-driver-integration-tests,java-driver-osgi-tests + java-driver-distribution-source,java-driver-distribution-tests,java-driver-distribution,java-driver-examples,java-driver-integration-tests,java-driver-osgi-tests,java-driver-coverage-report ${release.autopublish} validated From 6b1657770441b98578a755fc264cc44accd0f481 Mon Sep 17 00:00:00 2001 From: Roy Dahan Date: Wed, 19 Aug 2026 20:20:54 +0300 Subject: [PATCH 2/4] Fix missing mapper/metrics module coverage in the aggregate report jacoco:report-aggregate only aggregates compile/runtime-scoped reactor dependencies. The root pom's dependencyManagement pins mapper-runtime, mapper-processor, metrics-micrometer, and metrics-microprofile to scope=test (correct for their other consumers like integration-tests), and coverage-report inherited that scope for its own dependency on them, so their classes were silently excluded from the aggregate even though their jacoco.exec data was loaded (confirmed: all 7 exec files load, only 3 modules got analyzed as bundles). This lost their own integration test coverage entirely (MicrometerMetricsIT, MicroProfileMetricsIT). Override the scope to compile for coverage-report's own dependency declarations. --- coverage-report/pom.xml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/coverage-report/pom.xml b/coverage-report/pom.xml index ec5ebb2a5d0..03ee9591f67 100644 --- a/coverage-report/pom.xml +++ b/coverage-report/pom.xml @@ -39,6 +39,18 @@ executions are disabled below (not via jacoco.skip, which would also disable report-aggregate itself) since this module has no tests or classes of its own for them to act on. + + scope=compile is explicit on mapper-runtime/mapper-processor/metrics-* + below because the root pom's dependencyManagement pins them to + scope=test (correct for their other consumers, like integration-tests, + which only need them for testing); report-aggregate only aggregates + compile/runtime-scoped reactor dependencies, so without this override + these 4 modules' classes are silently excluded from the aggregate + report even though their jacoco.exec data is loaded (confirmed by + comparing the loaded-execution-data-file log lines against the + Analyzed-bundle log lines: all 7 files load, only 3 modules get + analyzed). That's real, substantial coverage lost from these modules' + own integration tests (MicrometerMetricsIT, MicroProfileMetricsIT). --> @@ -52,18 +64,22 @@ com.scylladb java-driver-mapper-runtime + compile com.scylladb java-driver-mapper-processor + compile com.scylladb java-driver-metrics-micrometer + compile com.scylladb java-driver-metrics-microprofile + compile com.scylladb From 7283c0fe7d5e3e6bd51793030df90664dedd950d Mon Sep 17 00:00:00 2001 From: Roy Dahan Date: Thu, 20 Aug 2026 06:54:10 +0300 Subject: [PATCH 3/4] Revert "Fix missing mapper/metrics module coverage in the aggregate report" This reverts commit 6b1657770441b98578a755fc264cc44accd0f481. --- coverage-report/pom.xml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/coverage-report/pom.xml b/coverage-report/pom.xml index 03ee9591f67..ec5ebb2a5d0 100644 --- a/coverage-report/pom.xml +++ b/coverage-report/pom.xml @@ -39,18 +39,6 @@ executions are disabled below (not via jacoco.skip, which would also disable report-aggregate itself) since this module has no tests or classes of its own for them to act on. - - scope=compile is explicit on mapper-runtime/mapper-processor/metrics-* - below because the root pom's dependencyManagement pins them to - scope=test (correct for their other consumers, like integration-tests, - which only need them for testing); report-aggregate only aggregates - compile/runtime-scoped reactor dependencies, so without this override - these 4 modules' classes are silently excluded from the aggregate - report even though their jacoco.exec data is loaded (confirmed by - comparing the loaded-execution-data-file log lines against the - Analyzed-bundle log lines: all 7 files load, only 3 modules get - analyzed). That's real, substantial coverage lost from these modules' - own integration tests (MicrometerMetricsIT, MicroProfileMetricsIT). --> @@ -64,22 +52,18 @@ com.scylladb java-driver-mapper-runtime - compile com.scylladb java-driver-mapper-processor - compile com.scylladb java-driver-metrics-micrometer - compile com.scylladb java-driver-metrics-microprofile - compile com.scylladb From b411483319d702a25045462fbc1c1c6f171e167a Mon Sep 17 00:00:00 2001 From: Roy Dahan Date: Thu, 20 Aug 2026 08:56:20 +0300 Subject: [PATCH 4/4] Reapply "Fix missing mapper/metrics module coverage in the aggregate report" This reverts commit 7283c0fe7d5e3e6bd51793030df90664dedd950d. --- coverage-report/pom.xml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/coverage-report/pom.xml b/coverage-report/pom.xml index ec5ebb2a5d0..03ee9591f67 100644 --- a/coverage-report/pom.xml +++ b/coverage-report/pom.xml @@ -39,6 +39,18 @@ executions are disabled below (not via jacoco.skip, which would also disable report-aggregate itself) since this module has no tests or classes of its own for them to act on. + + scope=compile is explicit on mapper-runtime/mapper-processor/metrics-* + below because the root pom's dependencyManagement pins them to + scope=test (correct for their other consumers, like integration-tests, + which only need them for testing); report-aggregate only aggregates + compile/runtime-scoped reactor dependencies, so without this override + these 4 modules' classes are silently excluded from the aggregate + report even though their jacoco.exec data is loaded (confirmed by + comparing the loaded-execution-data-file log lines against the + Analyzed-bundle log lines: all 7 files load, only 3 modules get + analyzed). That's real, substantial coverage lost from these modules' + own integration tests (MicrometerMetricsIT, MicroProfileMetricsIT). --> @@ -52,18 +64,22 @@ com.scylladb java-driver-mapper-runtime + compile com.scylladb java-driver-mapper-processor + compile com.scylladb java-driver-metrics-micrometer + compile com.scylladb java-driver-metrics-microprofile + compile com.scylladb