diff --git a/.github/actions/collect-coverage-exec/action.yml b/.github/actions/collect-coverage-exec/action.yml new file mode 100644 index 00000000000..2bbeb248aee --- /dev/null +++ b/.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 /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 diff --git a/.github/workflows/tests@v1.yml b/.github/workflows/tests@v1.yml index 273bcd63115..dae41666926 100644 --- a/.github/workflows/tests@v1.yml +++ b/.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 /target/jacoco-.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 diff --git a/.gitignore b/.gitignore index 9458be86d97..d9e2683b75d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,4 @@ target/ -cobertura-history/ -testing/ .settings .classpath .project diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4b7611a6d25..926aca743cb 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/Makefile b/Makefile index 76430900850..6012a76df3a 100644 --- a/Makefile +++ b/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 diff --git a/driver-core/pom.xml b/driver-core/pom.xml index f6b5bcd3991..0e7ddee5ac1 100644 --- a/driver-core/pom.xml +++ b/driver-core/pom.xml @@ -36,6 +36,12 @@ (CQL3) and Cassandra's binary protocol. + + + false + + diff --git a/driver-coverage-report/pom.xml b/driver-coverage-report/pom.xml new file mode 100644 index 00000000000..44241a50478 --- /dev/null +++ b/driver-coverage-report/pom.xml @@ -0,0 +1,148 @@ + + + + 4.0.0 + + + com.scylladb + scylla-driver-parent + 3.11.5.19-SNAPSHOT + + + scylla-driver-coverage-report + pom + Java Driver for Scylla and Apache Cassandra - Coverage report + Aggregates the JaCoCo execution data produced by the other modules into a single + coverage report. Produces no released artifact. + + + + + + com.scylladb + scylla-driver-core + + + + com.scylladb + scylla-driver-mapping + + + + com.scylladb + scylla-driver-extras + + + + + + + + + + + + org.codehaus.mojo + clirr-maven-plugin + + true + + + + + maven-source-plugin + + true + + + + + maven-javadoc-plugin + + true + + + + + maven-gpg-plugin + + true + + + + + maven-install-plugin + + true + + + + + maven-deploy-plugin + + true + + + + + + + + + + + + coverage-report + + + + org.jacoco + jacoco-maven-plugin + + + report-aggregate + verify + + report-aggregate + + + Java Driver for Scylla and Apache Cassandra 3.x + + ${project.build.directory}/site/jacoco-aggregate + + + + + + + + + + + diff --git a/driver-extras/pom.xml b/driver-extras/pom.xml index 2cbff699e3b..cfe44329e08 100644 --- a/driver-extras/pom.xml +++ b/driver-extras/pom.xml @@ -34,6 +34,12 @@ Java Driver for Scylla and Apache Cassandra - Extras Extended functionality for the Java driver. + + + false + + diff --git a/driver-mapping/pom.xml b/driver-mapping/pom.xml index e1bd1b59eb9..75c9822eb3a 100644 --- a/driver-mapping/pom.xml +++ b/driver-mapping/pom.xml @@ -34,6 +34,12 @@ Java Driver for Scylla and Apache Cassandra - Object Mapping Object mapper for the CQL Java Driver. + + + false + + diff --git a/pom.xml b/pom.xml index ea0d22cae71..30c64082b9a 100644 --- a/pom.xml +++ b/pom.xml @@ -44,6 +44,7 @@ driver-examples driver-tests driver-dist + driver-coverage-report @@ -93,6 +94,12 @@ updated. --> 1.5.9 3.5.4 + 0.8.14 + + 127.0.1. unit @@ -734,13 +741,19 @@ + + org.jacoco + jacoco-maven-plugin + ${jacoco.version} + + maven-surefire-plugin ${surefire.version} ${test.groups} false - -Djdk.attach.allowAttachSelf=true + -Djdk.attach.allowAttachSelf=true @{jacoco.argline} alphabetical ${cassandra.version} @@ -869,7 +882,7 @@ true central - scylla-driver-tests-parent,scylla-driver-tests-osgi,scylla-driver-tests-osgi-common,scylla-driver-tests-osgi-shaded,scylla-driver-tests-shading,scylla-driver-tests-shading-shaded,scylla-driver-tests-shading-unshaded,scylla-driver-tests-osgi-unshaded,scylla-driver-tests-stress,scylla-driver-dist,scylla-driver-examples + scylla-driver-coverage-report,scylla-driver-tests-parent,scylla-driver-tests-osgi,scylla-driver-tests-osgi-common,scylla-driver-tests-osgi-shaded,scylla-driver-tests-shading,scylla-driver-tests-shading-shaded,scylla-driver-tests-shading-unshaded,scylla-driver-tests-osgi-unshaded,scylla-driver-tests-stress,scylla-driver-dist,scylla-driver-examples ${release.autopublish} validated @@ -941,6 +954,51 @@ + + + coverage + + + local + ${project.build.directory}/jacoco-${coverage.lane}.exec + + true + + + + + org.jacoco + jacoco-maven-plugin + + + prepare-agent + + prepare-agent + + + + jacoco.argline + + + + + + + +