Add code coverage measurement for unit and integration tests

.github/workflows/tests@v1.yml

@@ -27,6 +27,19 @@ on:
       - ".gitignore"
   workflow_dispatch:
 
+# JaCoCo matches execution data to classes by an id derived from the compiled
+# bytes, and javac 11 and javac 17 do not emit the same bytes for the same
+# source even under --release 11 (they order the constant pool differently).
+# Execution data recorded on one of them is therefore invisible to a report
+# rendered against classes compiled by the other, and invisible is literal:
+# JaCoCo only warns about a name it has no id for at all, so data that loses
+# to a matching id from another lane is dropped without a word. Only the lanes
+# on this JDK are instrumented, and the aggregating job compiles on it and
+# rejects data from any other, so the number stays one the lanes actually
+# produced.
+env:
+  COVERAGE_JAVA_VERSION: "17"
+
 jobs:
   build:
     name: Build
@@ -133,8 +146,35 @@ jobs:
           key: ${{ runner.os }}-${{ matrix.java-version }}-maven-${{ hashFiles('**/pom.xml') }}
 
       - name: Run unit tests
+        env:
+          COVERAGE: ${{ matrix.java-version == env.COVERAGE_JAVA_VERSION }}
         run: make test-unit
 
+      # Flattened to one file per module so the artifact layout does not depend
+      # on how many modules happened to produce data, and named per lane so the
+      # aggregating job can keep each lane's contribution apart.
+      - name: Collect coverage execution data
+        if: ${{ !cancelled() && matrix.java-version == env.COVERAGE_JAVA_VERSION }}
+        run: |
+          shopt -s nullglob
+          mkdir -p coverage-exec
+          # Read back by the aggregating job, which refuses data compiled by a
+          # different javac than the one it renders the report against.
+          echo '${{ matrix.java-version }}' > coverage-exec/jdk.txt
+          for exec_file in */target/jacoco.exec metrics/*/target/jacoco.exec; do
+            module="${exec_file%/target/jacoco.exec}"
+            cp "$exec_file" "coverage-exec/${module//\//-}.exec"
+          done
+          ls -l coverage-exec
+
+      - name: Upload coverage execution data
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        if: ${{ !cancelled() && matrix.java-version == env.COVERAGE_JAVA_VERSION }}
+        with:
+          name: coverage-exec-unit-${{ matrix.java-version }}
+          path: coverage-exec/
+          if-no-files-found: warn
+
       - name: Upload test results
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         if: always()
@@ -256,8 +296,31 @@ jobs:
           GET_VERSION_VERSION: 0.4.5
           GH_TOKEN: ${{ github.token }}
           MAVEN_EXTRA_ARGS: ${{ steps.test-skip-args.outputs.value }}
+          COVERAGE: ${{ matrix.java-version == env.COVERAGE_JAVA_VERSION }}
         run: make test-integration-cassandra
 
+      - name: Collect coverage execution data
+        if: ${{ !cancelled() && matrix.java-version == env.COVERAGE_JAVA_VERSION }}
+        run: |
+          shopt -s nullglob
+          mkdir -p coverage-exec
+          # Read back by the aggregating job, which refuses data compiled by a
+          # different javac than the one it renders the report against.
+          echo '${{ matrix.java-version }}' > coverage-exec/jdk.txt
+          for exec_file in */target/jacoco.exec metrics/*/target/jacoco.exec; do
+            module="${exec_file%/target/jacoco.exec}"
+            cp "$exec_file" "coverage-exec/${module//\//-}.exec"
+          done
+          ls -l coverage-exec
+
+      - name: Upload coverage execution data
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        if: ${{ !cancelled() && matrix.java-version == env.COVERAGE_JAVA_VERSION }}
+        with:
+          name: coverage-exec-cassandra-${{ matrix.cassandra-version }}-${{ matrix.java-version }}-${{ matrix.test-group }}
+          path: coverage-exec/
+          if-no-files-found: warn
+
       - name: Upload test results
         if: failure() && steps.run-integration-tests.outcome == 'failure'
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
@@ -369,8 +432,31 @@ jobs:
         env:
           SCYLLA_VERSION_RESOLVED: ${{ steps.scylla-version.outputs.value }}
           MAVEN_EXTRA_ARGS: ${{ steps.test-skip-args.outputs.value }}
+          COVERAGE: ${{ matrix.java-version == env.COVERAGE_JAVA_VERSION }}
         run: make test-integration-scylla
 
+      - name: Collect coverage execution data
+        if: ${{ !cancelled() && matrix.java-version == env.COVERAGE_JAVA_VERSION }}
+        run: |
+          shopt -s nullglob
+          mkdir -p coverage-exec
+          # Read back by the aggregating job, which refuses data compiled by a
+          # different javac than the one it renders the report against.
+          echo '${{ matrix.java-version }}' > coverage-exec/jdk.txt
+          for exec_file in */target/jacoco.exec metrics/*/target/jacoco.exec; do
+            module="${exec_file%/target/jacoco.exec}"
+            cp "$exec_file" "coverage-exec/${module//\//-}.exec"
+          done
+          ls -l coverage-exec
+
+      - name: Upload coverage execution data
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        if: ${{ !cancelled() && matrix.java-version == env.COVERAGE_JAVA_VERSION }}
+        with:
+          name: coverage-exec-scylla-${{ matrix.scylla-version }}-${{ matrix.java-version }}-${{ matrix.test-group }}
+          path: coverage-exec/
+          if-no-files-found: warn
+
       - name: Upload test results
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         if: failure() && steps.run-integration-tests.outcome == 'failure'
@@ -396,3 +482,107 @@ 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
+
+    # Only needs to read the checkout; same-run artifacts are handled by the
+    # Actions runtime token rather than GITHUB_TOKEN. Scoped on this job alone
+    # so the existing lanes keep the token permissions their reporting steps
+    # rely on.
+    permissions:
+      contents: read
+
+    env:
+      # Overrides the Makefile default of `mvn -B -X -ntp`; this job has nothing
+      # to debug and -X buys a log measured in hundreds of megabytes.
+      MVNCMD: mvn -B -ntp
+
+    steps:
+      - name: Checkout source
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
+        with:
+          persist-credentials: false
+
+      - name: Set up JDK ${{ env.COVERAGE_JAVA_VERSION }}
+        uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+        with:
+          java-version: ${{ env.COVERAGE_JAVA_VERSION }}
+          distribution: 'temurin'
+
+      - name: Restore maven repository cache
+        uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-${{ env.COVERAGE_JAVA_VERSION }}-maven-${{ hashFiles('**/pom.xml') }}
+
+      - name: Download coverage execution data
+        uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
+        with:
+          pattern: coverage-exec-*
+          path: coverage-exec
+
+      # jacoco:report-aggregate picks up every *.exec in a module's target
+      # directory, so each lane's data only has to land there under a name of
+      # its own. The module name was flattened on upload (metrics/micrometer ->
+      # metrics-micrometer), so undo that to find the directory again.
+      #
+      # The lanes are listed to the job summary because "if: !cancelled()"
+      # lets this job run with some of them failed or skipped, and a lane
+      # missing its data costs the percentage silently: the report is simply
+      # lower, with nothing in it recording what it was built from.
+      - name: Place execution data next to the classes it was recorded against
+        run: |
+          set -o pipefail
+          shopt -s nullglob
+          lanes=(coverage-exec/*/)
+          if [[ ${#lanes[@]} -eq 0 ]]; then
+            echo '::error::No coverage execution data was downloaded from any test lane.'
+            exit 1
+          fi
+          {
+            echo '### Coverage execution data'
+            echo
+            echo "| Lane | Modules |"
+            echo "| --- | --- |"
+          } | tee -a "${GITHUB_STEP_SUMMARY:-/dev/null}"
+          for lane in "${lanes[@]}"; do
+            lane_name="$(basename "$lane")"
+            lane_jdk="$(cat "$lane/jdk.txt" 2>/dev/null || true)"
+            if [[ "$lane_jdk" != "$COVERAGE_JAVA_VERSION" ]]; then
+              echo "::error::${lane_name} recorded its execution data on JDK ${lane_jdk:-unknown}, but this job renders the report against classes compiled by JDK ${COVERAGE_JAVA_VERSION}. JaCoCo would drop that lane's data without a warning, so fail here instead."
+              exit 1
+            fi
+            modules=()
+            for exec_file in "$lane"*.exec; do
+              module="$(basename "$exec_file" .exec)"
+              if [[ ! -d "$module" && -d "${module/-//}" ]]; then
+                module="${module/-//}"
+              fi
+              modules+=("$module")
+              mkdir -p "$module/target"
+              cp "$exec_file" "$module/target/jacoco-${lane_name#coverage-exec-}.exec"
+            done
+            echo "| ${lane_name#coverage-exec-} | ${#modules[@]}: ${modules[*]} |" \
+              | tee -a "${GITHUB_STEP_SUMMARY:-/dev/null}"
+          done
+          find . -name 'jacoco-*.exec' -printf '%p\t%s bytes\n'
+
+      - name: Aggregate coverage
+        run: make coverage-report
+
+      - name: Upload coverage report
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        if: ${{ !cancelled() }}
+        with:
+          name: coverage-report
+          path: coverage-report/target/site/jacoco-aggregate
+          if-no-files-found: error

Makefile

@@ -24,6 +24,31 @@ MAVEN_OPTS ?=
 
 RELEASE_SKIP_TESTS ?=
 
+# 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. Off by default: the agent slows every fork down,
+# and the existing test lanes have to stay able to run without it.
+COVERAGE ?= false
+# Spellings are normalised, and anything outside the two lists below is an
+# error rather than a silent "off": COVERAGE=TRUE used to run without the
+# agent, and the miss only surfaced much later, when `make coverage-report`
+# said to run with COVERAGE=true -- the thing you thought you had just done.
+_COVERAGE_NORM := $(or $(shell printf '%s' '$(COVERAGE)' | tr '[:upper:]' '[:lower:]'),false)
+ifneq ($(filter $(_COVERAGE_NORM),true 1 yes on),)
+	MVN_COVERAGE := -Pcoverage
+	COVERAGE_PREREQ := .clean-coverage-data
+else ifneq ($(filter $(_COVERAGE_NORM),false 0 no off),)
+	MVN_COVERAGE :=
+	COVERAGE_PREREQ :=
+else
+# Not tab-indented, unlike the assignments above: make reads a tab-indented
+# line that is not an assignment as a recipe line.
+$(error COVERAGE must be one of true/1/yes/on or false/0/no/off, got '$(COVERAGE)')
+endif
+COVERAGE_REPORT_DIR := coverage-report/target/site/jacoco-aggregate
+COVERAGE_MAVEN_LOG_DIR := coverage-report/target
+COVERAGE_MAVEN_LOG := ${COVERAGE_MAVEN_LOG_DIR}/coverage-maven.log
+
 ifeq (${CCM_CONFIG_DIR},)
 	CCM_CONFIG_DIR = ~/.ccm
 endif
@@ -33,6 +58,18 @@ export SCYLLA_EXT_OPTS
 export SCYLLA_VERSION
 export PATH := $(MAKEFILE_PATH)/bin:$(PATH)
 
+# JaCoCo appends to its execution data by default, which is what lets one lane
+# accumulate coverage across several forks (integration-tests alone runs three).
+# The flip side is that data from an earlier run survives a recompile, and a
+# class that changed in between is then reported uncovered because its checksum
+# no longer matches. Truncating before a run is the fix.
+#
+# Only jacoco.exec is removed -- the file the agent is about to write. Data
+# renamed out of the way to keep one lane's results while another runs (as the
+# CI coverage job does) is left alone.
+.clean-coverage-data:
+	@find . -name 'jacoco.exec' -delete
+
 .install-guava-shaded:
 	$(MVNCMD) install -pl guava-shaded
 
@@ -363,10 +400,10 @@ check:
 fix:
 	$(MVNCMD) fmt:format xml-format:xml-format
 
-test-unit: .install-guava-shaded
-	$(MVNCMD) test -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
+test-unit: .install-guava-shaded $(COVERAGE_PREREQ)
+	$(MVNCMD) test $(MVN_COVERAGE) -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true
 
-test-integration-scylla: .install-all-modules .prepare-scylla-ccm resolve-scylla-version .prepare-environment-update-aio-max-nr
+test-integration-scylla: .install-all-modules .prepare-scylla-ccm resolve-scylla-version .prepare-environment-update-aio-max-nr $(COVERAGE_PREREQ)
 	@if [[ -z "$${SCYLLA_VERSION_RESOLVED}" ]]; then
 		SCYLLA_VERSION_RESOLVED=`cat '${SCYLLA_VERSION_FILE}'`
 	fi
@@ -377,9 +414,9 @@ test-integration-scylla: .install-all-modules .prepare-scylla-ccm resolve-scylla
 	# Check it here too, where the version is handed to CCM: this value can arrive from
 	# the environment, which is how CI passes it, so the resolver never saw it.
 	$(call REQUIRE_FULLY_QUALIFIED_VERSION,SCYLLA_VERSION_RESOLVED,ScyllaDB)
-	mvn -B -e verify -pl integration-tests -Dccm.version=$${SCYLLA_VERSION_RESOLVED} -Dccm.distribution=scylla -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true $(MAVEN_EXTRA_ARGS)
+	mvn -B -e verify $(MVN_COVERAGE) -pl integration-tests -Dccm.version=$${SCYLLA_VERSION_RESOLVED} -Dccm.distribution=scylla -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true $(MAVEN_EXTRA_ARGS)
 
-test-integration-cassandra: .install-all-modules .prepare-scylla-ccm resolve-cassandra-version
+test-integration-cassandra: .install-all-modules .prepare-scylla-ccm resolve-cassandra-version $(COVERAGE_PREREQ)
 	@if [[ -z "$${CASSANDRA_VERSION_RESOLVED}" ]]; then
 		CASSANDRA_VERSION_RESOLVED=`cat '${CASSANDRA_VERSION_FILE}'`
 	fi
@@ -390,7 +427,66 @@ test-integration-cassandra: .install-all-modules .prepare-scylla-ccm resolve-cas
 	# Check it here too, where the version is handed to CCM: this value can arrive from
 	# the environment, which is how CI passes it, so the resolver never saw it.
 	$(call REQUIRE_FULLY_QUALIFIED_VERSION,CASSANDRA_VERSION_RESOLVED,Cassandra)
-	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)
+	mvn -B -e verify $(MVN_COVERAGE) -pl integration-tests -Dccm.version=$${CASSANDRA_VERSION_RESOLVED} -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true $(MAVEN_EXTRA_ARGS)
+
+# Aggregates the execution data left behind by any COVERAGE=true test run into
+# a single cross-module report -- most importantly attributing the coverage
+# core gets *through* the integration suite back to core's own source, which
+# each module's own report cannot see. Tests are skipped here on purpose: this
+# only reads what is already on disk, so the same target serves one local lane
+# and execution data collected from several CI jobs.
+#
+# report-aggregate is bound to `verify` inside the "coverage" profile (see
+# coverage-report/pom.xml) rather than requested as a bare CLI goal: a CLI goal
+# runs against every project the -am reactor pulls in, which rendered a stray
+# report in all eleven of them (one of those over guava-shaded's relocated
+# classes), and it never sees the execution's own configuration. Keeping the
+# binding inside the profile still leaves a plain `mvn verify`/`mvn install`
+# rendering nothing.
+#
+# .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: .install-guava-shaded
+	@# Without this the recipe's exit status is that of the tee on its last
+	@# line, not of the python that feeds it, so the empty-report check would
+	@# print its complaint and let the target pass anyway.
+	set -o pipefail
+	if [[ -z "$$(find . -name 'jacoco*.exec' -not -path './coverage-report/*' -print -quit)" ]]; then
+		echo 'No JaCoCo execution data found.'
+		echo "Run the tests with COVERAGE=true first, e.g. 'make test-unit COVERAGE=true'."
+		exit 1
+	fi
+	rm -rf '${COVERAGE_REPORT_DIR}'
+	mkdir -p '${COVERAGE_MAVEN_LOG_DIR}'
+	$(MVNCMD) verify -Pcoverage -pl coverage-report -am -DskipTests -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true 2>&1 | tee '${COVERAGE_MAVEN_LOG}'
+	# JaCoCo matches execution data to classes by checksum and only warns when
+	# it does not match, dropping that class's data; the report still renders,
+	# just quietly short. Nothing else in this target can see that -- the
+	# percentage is simply lower -- so fail on the warning itself.
+	if grep -q 'Execution data for class .* does not match' '${COVERAGE_MAVEN_LOG}'; then
+		echo 'Execution data does not match the compiled classes, so the report below understates coverage.'
+		echo 'The data has to come from the same build of the classes the report is rendered against.'
+		grep 'Execution data for class .* does not match' '${COVERAGE_MAVEN_LOG}' | sort -u
+		exit 1
+	fi
+	if [[ ! -f '${COVERAGE_REPORT_DIR}/jacoco.xml' ]]; then
+		echo 'Maven produced no report at ${COVERAGE_REPORT_DIR}/jacoco.xml.'
+		exit 1
+	fi
+	echo 'HTML report: ${COVERAGE_REPORT_DIR}/index.html'
+	# Read the report-level LINE counter rather than the sibling csv, whose
+	# fields are unquoted and so shift on any class name containing a comma.
+	# Zero covered lines means the execution data did not match these classes
+	# (look for a checksum mismatch in the log), which is worth failing on:
+	# the alternative is a confident-looking 0%.
+	python3 -c 'import sys, xml.etree.ElementTree as ET; r = ET.parse(sys.argv[1]).getroot(); c = next(x for x in r.findall("counter") if x.get("type") == "LINE"); missed, covered = int(c.get("missed")), int(c.get("covered")); total = missed + covered; print("Line coverage: {}/{} ({:.2f}%)".format(covered, total, 100.0 * covered / total if total else 0.0)); sys.exit("No lines are recorded as covered: the execution data is either missing or does not match these classes. Look for a checksum mismatch warning in the Maven log.") if covered == 0 else None' '${COVERAGE_REPORT_DIR}/jacoco.xml' | tee -a "$${GITHUB_STEP_SUMMARY:-/dev/null}"
+
+clean-coverage:
+	find . -name 'jacoco*.exec' -delete
+	find . -type d -path '*/target/site/jacoco*' -exec rm -rf {} +
+	rm -rf coverage-report/target/site '${COVERAGE_MAVEN_LOG}'
 
 check-no-compile-warnings:
 	@$(MAKE) compile-all | grep WARNING >/tmp/all-compile-warnings.log || true