build: add JaCoCo coverage for the unit and integration lanes (3.x)

.github/actions/collect-coverage-exec/action.yml

@@ -0,0 +1,40 @@
+name: Collect coverage execution data
+description: >
+  Uploads the JaCoCo execution data a COVERAGE=true test lane left behind. Factored out because
+  every instrumented lane needs it verbatim, and because the layout it produces has to stay in
+  step with the step that reverses it in the coverage-report job.
+
+inputs:
+  artifact-name:
+    description: >
+      Name of the artifact to upload. Has to start with coverage-exec- for the coverage-report
+      job's download pattern to match it.
+    required: true
+
+runs:
+  using: composite
+
+  steps:
+
+    # Kept under <module>/target/ so the coverage-report job can put the tree straight back where
+    # it came from; which lane produced the data is already part of the file name. Only the
+    # top-level modules are collected, which is all jacoco:report-aggregate reads. The
+    # "Place execution data next to the classes it was recorded against" step reverses exactly
+    # this layout -- the two have to be changed together.
+    - name: Copy execution data into an artifact tree
+      shell: bash
+      run: |
+        shopt -s nullglob
+        mkdir -p coverage-exec
+        for exec_file in */target/jacoco-*.exec; do
+          mkdir -p "coverage-exec/$(dirname "$exec_file")"
+          cp "$exec_file" "coverage-exec/$exec_file"
+        done
+        find coverage-exec -name '*.exec' -printf '%p\t%s bytes\n'
+
+    - name: Upload execution data
+      uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+      with:
+        name: ${{ inputs.artifact-name }}
+        path: coverage-exec/
+        if-no-files-found: warn

.github/workflows/tests@v1.yml

@@ -128,8 +128,17 @@ jobs:
           path: ~/.m2/repository
           key: ${{ runner.os }}-${{ matrix.java-version }}-maven-${{ hashFiles('**/pom.xml') }}
 
+      # COVERAGE and the lane name are passed as make variables rather than through env: a
+      # target-specific COVERAGE_LANE assignment in the Makefile beats the environment, so an
+      # env: entry would quietly leave every lane of this matrix writing one file name.
       - name: Run unit tests
-        run: make test-unit
+        run: make test-unit COVERAGE=true COVERAGE_LANE=unit-${{ matrix.java-version }}
+
+      - name: Collect coverage execution data
+        if: ${{ !cancelled() }}
+        uses: ./.github/actions/collect-coverage-exec
+        with:
+          artifact-name: coverage-exec-unit-${{ matrix.java-version }}
 
       - name: Copy test results
         if: always()
@@ -233,11 +242,20 @@ jobs:
           path: ~/.ccm/repository
           key: ccm-cassandra-${{ runner.os }}-${{ steps.cassandra-version.outputs.value }}
 
+      # COVERAGE and the lane name are passed as make variables rather than through env: a
+      # target-specific COVERAGE_LANE assignment in the Makefile beats the environment, so an
+      # env: entry would quietly leave every lane of this matrix writing one file name.
       - name: Run integration tests on Cassandra (${{ steps.cassandra-version.outputs.value }})
         id: run-integration-tests
         env:
           CASSANDRA_VERSION_RESOLVED: ${{ steps.cassandra-version.outputs.value }}
-        run: make test-integration-cassandra
+        run: make test-integration-cassandra COVERAGE=true COVERAGE_LANE=cassandra-${{ matrix.cassandra-version }}
+
+      - name: Collect coverage execution data
+        if: ${{ !cancelled() }}
+        uses: ./.github/actions/collect-coverage-exec
+        with:
+          artifact-name: coverage-exec-cassandra-${{ matrix.cassandra-version }}
 
       - name: Copy test results
         if: steps.run-integration-tests.outcome == 'failure'
@@ -332,11 +350,20 @@ jobs:
           path: ~/.ccm/scylla-repository
           key: ccm-scylla-${{ runner.os }}-${{ steps.scylla-version.outputs.value }}
 
+      # COVERAGE and the lane name are passed as make variables rather than through env: a
+      # target-specific COVERAGE_LANE assignment in the Makefile beats the environment, so an
+      # env: entry would quietly leave every lane of this matrix writing one file name.
       - name: Run integration tests on Scylla (${{ steps.scylla-version.outputs.value }})
         id: run-integration-tests
         env:
           SCYLLA_VERSION_RESOLVED: ${{ steps.scylla-version.outputs.value }}
-        run: make test-integration-scylla
+        run: make test-integration-scylla COVERAGE=true COVERAGE_LANE=scylla-${{ matrix.scylla-version }}
+
+      - name: Collect coverage execution data
+        if: ${{ !cancelled() }}
+        uses: ./.github/actions/collect-coverage-exec
+        with:
+          artifact-name: coverage-exec-scylla-${{ matrix.scylla-version }}
 
       - name: Copy test results
         if: steps.run-integration-tests.outcome == 'failure'
@@ -370,3 +397,82 @@ jobs:
           detailed_summary: true
           updateComment: false
           skip_annotations: true
+
+  coverage-report:
+    name: Coverage report
+    runs-on: ubuntu-latest
+    needs: [unit-tests, cassandra-integration-tests, scylla-integration-tests]
+    # Runs even when a test lane failed: partial coverage data is still worth reporting, and
+    # continue-on-error keeps a flaky integration test from turning this metric into a second
+    # failure on the pull request.
+    if: ${{ !cancelled() }}
+    continue-on-error: true
+    timeout-minutes: 20
+
+    steps:
+      - name: Checkout source
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
+        with:
+          persist-credentials: false
+
+      - name: Set up JDK 8
+        uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+        with:
+          java-version: 8
+          distribution: 'temurin'
+
+      - name: Restore maven repository cache
+        uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-8-maven-${{ hashFiles('**/pom.xml') }}
+
+      - name: Download coverage execution data
+        uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
+        with:
+          pattern: coverage-exec-*
+          path: coverage-exec
+
+      # Each artifact holds <module>/target/jacoco-<lane>.exec, and jacoco:report-aggregate reads
+      # target/*.exec from every module it reports on, so the files only have to go back to where
+      # the lane wrote them.
+      - name: Place execution data next to the classes it was recorded against
+        run: |
+          shopt -s nullglob
+          for exec_file in coverage-exec/*/*/target/jacoco-*.exec; do
+            dest="${exec_file#coverage-exec/*/}"
+            mkdir -p "$(dirname "$dest")"
+            cp "$exec_file" "$dest"
+          done
+          find driver-* -maxdepth 2 -name 'jacoco-*.exec' -printf '%p\t%s bytes\n'
+
+      # A lane that produced nothing fails nothing on its own: the upload only warns, and this
+      # job is continue-on-error. It just lowers the percentage, and a lower percentage published
+      # on an otherwise green run reads as a regression caused by the pull request. Checked per
+      # kind rather than per matrix entry on purpose: a hard-coded count would rot silently.
+      - name: Check that every kind of test lane contributed
+        run: |
+          missing=()
+          for kind in unit cassandra scylla; do
+            if [[ -z "$(find coverage-exec -maxdepth 1 -type d -name "coverage-exec-$kind-*" -print -quit 2>/dev/null)" ]]; then
+              missing+=("$kind")
+            fi
+          done
+          if (( ${#missing[@]} > 0 )); then
+            echo "No coverage execution data from: ${missing[*]}." >&2
+            echo 'Those lanes did not run, so any number produced here would understate coverage.' >&2
+            exit 1
+          fi
+
+      - name: Aggregate coverage
+        run: make coverage-report
+
+      # Kept even on failure: `make coverage-report` exits non-zero when nothing is recorded as
+      # covered, and the report is what shows which bundles came out empty.
+      - name: Upload coverage report
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        if: ${{ !cancelled() }}
+        with:
+          name: coverage-report
+          path: driver-coverage-report/target/site/jacoco-aggregate
+          if-no-files-found: error

.gitignore

@@ -1,6 +1,4 @@
 target/
-cobertura-history/
-testing/
 .settings
 .classpath
 .project

CONTRIBUTING.md

@@ -125,6 +125,51 @@ sudo ifconfig lo0 alias 127.0.1.2 up
 ...
 ```
 
+### Code coverage
+
+Coverage is measured with [JaCoCo](https://www.jacoco.org/jacoco/) and is off by default: the agent
+slows every forked test JVM down, so it is opt-in through the `coverage` Maven profile. Pass
+`COVERAGE=true` to any of the `test-*` Make targets to enable it, then aggregate:
+
+```sh
+make test-unit COVERAGE=true
+make coverage-report
+```
+
+`make coverage-report` reads whatever execution data is already on disk, so several lanes can be
+combined into one number:
+
+```sh
+make test-unit COVERAGE=true
+make test-integration-scylla COVERAGE=true
+make coverage-report
+```
+
+Nothing has to be moved out of the way between the two: each lane writes its execution data under a
+name of its own (`jacoco-unit.exec`, `jacoco-scylla-LATEST.exec`, ...) and truncates only that file
+before it starts, so one lane can never overwrite another's results or read stale ones as its own.
+
+The report lands in `driver-coverage-report/target/site/jacoco-aggregate` (HTML, XML and CSV), and
+`make clean-coverage` removes it along with the execution data. In CI, the unit and integration jobs
+upload their execution data and the "Coverage report" job aggregates it; the percentage shows up in
+that job's summary and the HTML report is attached as an artifact.
+
+Two things to know about the scope of the report:
+
+- It covers the modules that `driver-coverage-report` depends on: `driver-core`,
+  `driver-mapping` and `driver-extras`. `driver-examples`, `driver-tests/**` and `driver-dist`
+  are out of scope, and the agent is not attached to them at all: the data would only be recorded
+  for nobody to read.
+- Only Surefire is instrumented. That covers the unit tests and, because 3.x runs its integration
+  tests as TestNG `short`-group tests, the integration tests as well. The Failsafe-run tests in
+  `driver-tests/**` (OSGi, shading) are left out: the OSGi ones load the driver inside their own
+  Pax Exam container, and those modules are outside the report's scope in any case.
+
+JaCoCo matches execution data to classes by checksum, so the data has to come from the same build of
+the classes the report is rendered against. If a report shows code you know was exercised as
+uncovered, look for `Execution data for class ... does not match` in the Maven log; the usual cause
+is stale execution data from before a recompile, which `make clean-coverage` clears.
+
 ## Updating GitHub Actions workflows
 
 GitHub Actions workflows in this repository pin all third-party actions to specific commit SHAs

Makefile

@@ -22,6 +22,36 @@ SONATYPE_TOKEN_PASSWORD ?=
 RELEASE_SKIP_TESTS ?= false
 RELEASE_TARGET_TAG ?=
 
+# Set COVERAGE=true on any of the test-* targets to attach the JaCoCo agent to the forked test
+# JVMs; `make coverage-report` then aggregates whatever execution data is on disk.
+COVERAGE ?= false
+COVERAGE_LOWER := $(shell printf '%s' '$(COVERAGE)' | tr '[:upper:]' '[:lower:]')
+# An exported but empty COVERAGE means off: the `?=` above does not apply to a variable that is
+# defined and empty, and aborting every unrelated target over it would be absurd.
+ifeq ($(strip $(COVERAGE_LOWER)),)
+COVERAGE_LOWER := false
+endif
+# A misspelled value is rejected rather than ignored: it used to produce a normal-looking run with
+# no coverage in it, and in CI that silently drops one lane out of the aggregate.
+ifeq ($(filter true 1 false 0,$(COVERAGE_LOWER)),)
+$(error COVERAGE must be true or false, got '$(COVERAGE)')
+endif
+ifeq ($(filter true 1,$(COVERAGE_LOWER)),)
+	MVN_COVERAGE =
+else
+	MVN_COVERAGE = -Pcoverage -Dcoverage.lane=$(COVERAGE_LANE)
+endif
+# Names this lane's execution data file, so that no two lanes ever write to one file. The default
+# is set per target below; CI overrides it with the matrix entry its uploaded artifact is named
+# after. That override has to be a make command-line variable, as in
+# `make test-unit COVERAGE_LANE=unit-8`: a target-specific assignment beats the environment, so
+# passing it as `env:` would quietly do nothing and put several lanes back on one file name.
+COVERAGE_LANE = local
+COVERAGE_REPORT_DIR := driver-coverage-report/target/site/jacoco-aggregate
+# Must stay in step with driver-coverage-report's dependencies: jacoco:report-aggregate reads
+# target/*.exec from those modules and nowhere else, so data anywhere else is not a report.
+COVERAGE_EXEC_DIRS := driver-core/target driver-mapping/target driver-extras/target
+
 ifeq (${CCM_CONFIG_DIR},)
 	CCM_CONFIG_DIR = ~/.ccm
 endif
@@ -67,6 +97,18 @@ export PATH := $(MAKEFILE_PATH)/bin:$(PATH)
   		$(MAKE) install-cassandra-ccm
 	fi
 
+# JaCoCo appends to its execution data files by default, which is what lets a single lane
+# accumulate coverage across several forks. The flip side is that data from an earlier run
+# survives a recompile, and classes that changed in between are then reported as uncovered
+# because their checksum no longer matches. Truncating before a run is the fix.
+#
+# Only this lane's file is removed, and only this lane ever writes it: another lane's results are
+# never in the way, and never left to go stale behind this one's back. Expanded inside each test
+# recipe rather than made a prerequisite of them, because make builds a prerequisite once per
+# invocation: `make test-unit test-integration-scylla COVERAGE=true` would then truncate the
+# first lane's file only, and let the second lane append to whatever it found.
+CLEAN_COVERAGE_DATA = $(if $(MVN_COVERAGE),find . -name 'jacoco-$(COVERAGE_LANE).exec' -delete,:)
+
 .prepare-environment-update-aio-max-nr:
 	@if (( $$(< /proc/sys/fs/aio-max-nr) < 2097152 )); then
 		echo 2097152 | sudo tee /proc/sys/fs/aio-max-nr >/dev/null
@@ -240,9 +282,12 @@ check:
 fix:
 	$(MVNCMD) fmt:format
 
+test-unit: COVERAGE_LANE = unit
 test-unit:
-	$(MVNCMD) test -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
+	$(CLEAN_COVERAGE_DATA)
+	$(MVNCMD) test $(MVN_COVERAGE) -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
 
+test-integration-scylla: COVERAGE_LANE = scylla-$(SCYLLA_VERSION)
 test-integration-scylla: .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}'`
@@ -251,8 +296,10 @@ test-integration-scylla: .prepare-scylla-ccm resolve-scylla-version .prepare-env
 		echo "ScyllaDB version ${SCYLLA_VERSION} was not resolved"
 		exit 1
 	fi
-	mvn -B verify -Pshort -Dscylla.version=$${SCYLLA_VERSION_RESOLVED} -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
+	$(CLEAN_COVERAGE_DATA)
+	mvn -B verify -Pshort $(MVN_COVERAGE) -Dscylla.version=$${SCYLLA_VERSION_RESOLVED} -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
 
+test-integration-cassandra: COVERAGE_LANE = cassandra-$(CASSANDRA_VERSION)
 test-integration-cassandra: .prepare-scylla-ccm resolve-cassandra-version
 	@if [[ -z "$${CASSANDRA_VERSION_RESOLVED}" ]]; then
 		CASSANDRA_VERSION_RESOLVED=`cat '${CASSANDRA_VERSION_FILE}'`
@@ -261,7 +308,62 @@ test-integration-cassandra: .prepare-scylla-ccm resolve-cassandra-version
 		echo "Cassandra version ${CASSANDRA_VERSION} was not resolved"
 		exit 1
 	fi
-	mvn -B verify -Pshort -Dcassandra.version=$${CASSANDRA_VERSION_RESOLVED} -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
+	$(CLEAN_COVERAGE_DATA)
+	mvn -B verify -Pshort $(MVN_COVERAGE) -Dcassandra.version=$${CASSANDRA_VERSION_RESOLVED} -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
+
+# Aggregates the execution data left behind by any COVERAGE=true test run into a single report.
+# Tests are skipped here on purpose: this only reads what is already on disk, so the same target
+# works for one local lane and for execution data collected from several CI jobs. Only test
+# execution is skipped, not test compilation: -Dmaven.test.skip=true would also suppress
+# driver-core's test-jar, which driver-mapping and driver-extras resolve at test scope, and
+# nothing in this build or in CI installs that jar for them to fall back on.
+#
+# mvn is called directly rather than through MVNCMD, whose -X default would tee a debug log
+# measured in hundreds of megabytes into the temp file this target then greps.
+coverage-report:
+	@set -eo pipefail
+	exec_files=$$(find $(COVERAGE_EXEC_DIRS) -maxdepth 1 -name '*.exec' 2>/dev/null | sort || true)
+	if [[ -z "$$exec_files" ]]; then
+		echo 'No JaCoCo execution data found in $(COVERAGE_EXEC_DIRS).'
+		echo "Run the tests with COVERAGE=true first, e.g. 'make test-unit COVERAGE=true'."
+		exit 1
+	fi
+	rm -rf '${COVERAGE_REPORT_DIR}'
+	maven_log=$$(mktemp)
+	trap 'rm -f "$$maven_log"' EXIT
+	mvn -B -ntp -Pcoverage-report -DskipTests verify -pl driver-coverage-report -am -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true 2>&1 | tee "$$maven_log"
+	if [[ ! -f '${COVERAGE_REPORT_DIR}/jacoco.xml' ]]; then
+		echo 'Maven produced no report at ${COVERAGE_REPORT_DIR}/jacoco.xml.'
+		exit 1
+	fi
+	# The report's own LINE counter, not a sum over jacoco.csv: the CSV has a row per class, and
+	# classes that share a source file share its lines, so adding the rows up counts those lines
+	# once per class. On this tree that alone moved the figure by 46 lines. This is the number
+	# JaCoCo puts in the HTML report, and it needs no assumption about column positions.
+	summary=$$(python3 -c 'import sys, xml.etree.ElementTree as ET; c = next(x for x in ET.parse(sys.argv[1]).getroot().findall("counter") if x.get("type") == "LINE"); missed, covered = int(c.get("missed")), int(c.get("covered")); total = missed + covered; print(covered, total, "{:.2f}%".format(100.0 * covered / total) if total else "n/a")' '${COVERAGE_REPORT_DIR}/jacoco.xml')
+	read -r covered total percentage <<< "$$summary"
+	mismatched=$$(grep -c 'does not match' "$$maven_log" || true)
+	# Which lanes went into the number is part of the number. A lane that was skipped or whose
+	# upload failed lowers the percentage, and on its own a lower percentage reads as a regression.
+	{
+		echo "Line coverage: $$covered/$$total ($$percentage)"
+		echo 'Aggregated from:'
+		printf '%s\n' "$$exec_files" | sed 's/^/  /'
+		if (( mismatched > 0 )); then
+			echo "WARNING: $$mismatched classes were dropped because their execution data was recorded"
+			echo "against differently compiled bytecode, so real coverage is higher than reported."
+		fi
+	} | tee -a "$${GITHUB_STEP_SUMMARY:-/dev/null}"
+	echo 'HTML report: ${COVERAGE_REPORT_DIR}/index.html'
+	if (( covered == 0 )); then
+		echo 'Nothing is recorded as covered: the execution data does not match these classes.' >&2
+		exit 1
+	fi
+
+
+clean-coverage:
+	@find . -name 'jacoco*.exec' -delete
+	rm -rf '${COVERAGE_REPORT_DIR}'
 
 check-no-compile-warnings:
 	@$(MAKE) compile-all | grep WARNING >/tmp/all-compile-warnings.log || true