diff --git a/.github/workflows/tests@v1.yml b/.github/workflows/tests@v1.yml index 68da13e9823..dd3edfd292e 100644 --- a/.github/workflows/tests@v1.yml +++ b/.github/workflows/tests@v1.yml @@ -133,8 +133,32 @@ jobs: key: ${{ runner.os }}-${{ matrix.java-version }}-maven-${{ hashFiles('**/pom.xml') }} - name: Run unit tests + env: + COVERAGE: "true" 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() }} + run: | + shopt -s nullglob + mkdir -p coverage-exec + 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() }} + 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 +280,28 @@ jobs: GET_VERSION_VERSION: 0.4.5 GH_TOKEN: ${{ github.token }} MAVEN_EXTRA_ARGS: ${{ steps.test-skip-args.outputs.value }} + COVERAGE: "true" run: make test-integration-cassandra + - name: Collect coverage execution data + if: ${{ !cancelled() }} + run: | + shopt -s nullglob + mkdir -p coverage-exec + 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() }} + 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 +413,28 @@ jobs: env: SCYLLA_VERSION_RESOLVED: ${{ steps.scylla-version.outputs.value }} MAVEN_EXTRA_ARGS: ${{ steps.test-skip-args.outputs.value }} + COVERAGE: "true" run: make test-integration-scylla + - name: Collect coverage execution data + if: ${{ !cancelled() }} + run: | + shopt -s nullglob + mkdir -p coverage-exec + 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() }} + 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 +460,81 @@ 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 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: 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. + - name: Place execution data next to the classes it was recorded against + run: | + shopt -s nullglob + for lane in coverage-exec/*/; do + lane_name="$(basename "$lane")" + for exec_file in "$lane"*.exec; do + module="$(basename "$exec_file" .exec)" + if [[ ! -d "$module" && -d "${module/-//}" ]]; then + module="${module/-//}" + fi + mkdir -p "$module/target" + cp "$exec_file" "$module/target/jacoco-${lane_name#coverage-exec-}.exec" + done + 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 diff --git a/Makefile b/Makefile index b41799423d6..0d187c4e839 100644 --- a/Makefile +++ b/Makefile @@ -24,6 +24,20 @@ 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 +ifeq ($(filter true 1,$(COVERAGE)),) + MVN_COVERAGE := + COVERAGE_PREREQ := +else + MVN_COVERAGE := -Pcoverage + COVERAGE_PREREQ := .clean-coverage-data +endif +COVERAGE_REPORT_DIR := coverage-report/target/site/jacoco-aggregate + ifeq (${CCM_CONFIG_DIR},) CCM_CONFIG_DIR = ~/.ccm endif @@ -33,6 +47,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 @@ -290,10 +316,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 @@ -301,9 +327,9 @@ test-integration-scylla: .install-all-modules .prepare-scylla-ccm resolve-scylla echo "ScyllaDB version ${SCYLLA_VERSION} was not resolved" exit 1 fi - 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 @@ -311,7 +337,51 @@ test-integration-cassandra: .install-all-modules .prepare-scylla-ccm resolve-cas echo "Cassandra version ${CASSANDRA_VERSION} was not resolved" exit 1 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) + 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 + @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}' + $(MVNCMD) verify -Pcoverage -pl coverage-report -am -DskipTests -Dfmt.skip=true -Dclirr.skip=true -Danimal.sniffer.skip=true + 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 check-no-compile-warnings: @$(MAKE) compile-all | grep WARNING >/tmp/all-compile-warnings.log || true diff --git a/README-dev.md b/README-dev.md index 07a4673f8a2..2d96cda699d 100644 --- a/README-dev.md +++ b/README-dev.md @@ -27,4 +27,57 @@ 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 + +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: + +``` +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 -- which is the point of the separate `coverage-report` module: it +attributes the coverage `core` gets *through* the integration suite back to `core`'s own source, +which each module's own report cannot see. A `COVERAGE=true` run truncates `jacoco.exec` before it +starts, so rename the previous lane's data out of the way to keep it: + +``` +make test-unit COVERAGE=true +find . -name jacoco.exec -execdir mv jacoco.exec jacoco-unit.exec \; +make test-integration-scylla COVERAGE=true +make coverage-report +``` + +The report lands in `coverage-report/target/site/jacoco-aggregate` (HTML, XML and CSV), and +`make clean-coverage` removes it along with the execution data. `make coverage-report` fails rather +than rendering a confident-looking but empty report if it finds no execution data, or if the data +matches none of the classes. + +In CI, the unit and integration jobs in `tests@v1.yml` run with `COVERAGE=true` and upload their +execution data; the "Coverage report" job aggregates it, prints the percentage to its job summary +and attaches the HTML report as an artifact. That job is `continue-on-error`, so a flaky +integration test costs the metric some data rather than adding a second failure to the pull +request. Collecting from the existing lanes rather than a dedicated workflow keeps the Scylla suite +from being run twice. + +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. + +Note: the surefire/failsafe configs in `core` and `integration-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 two. 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). `argLine` itself +is declared, empty, as a root `pom.xml` property so that combination resolves to something even +outside the `coverage` profile, where `jacoco:prepare-agent` never runs to give it a real value. +(`distribution-tests` has no `src` of its own, so surefire never forks there either way; it was +left out of this.) + 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 45f2ee64cf6..136fcba0420 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..5fc8e79fb75 --- /dev/null +++ b/coverage-report/pom.xml @@ -0,0 +1,164 @@ + + + + + 4.0.0 + + com.scylladb + java-driver-parent + 4.19.2.2-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 + compile + + + com.scylladb + java-driver-mapper-processor + compile + + + com.scylladb + java-driver-metrics-micrometer + compile + + + com.scylladb + java-driver-metrics-microprofile + compile + + + com.scylladb + java-driver-integration-tests + ${project.version} + + + + + + org.jacoco + jacoco-maven-plugin + + + + default + none + + + report + none + + + + + maven-install-plugin + + true + + + + maven-deploy-plugin + + true + + + + + + + + coverage + + + + org.jacoco + jacoco-maven-plugin + + + report-aggregate + verify + + report-aggregate + + + Java Driver for Scylla and Apache Cassandra 4.x + + HTML + XML + CSV + + ${project.build.directory}/site/jacoco-aggregate + + + + + + + + + diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index 7f481242fce..dd05e343872 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 76caea447cc..ca2471cf70e 100644 --- a/pom.xml +++ b/pom.xml @@ -50,10 +50,20 @@ distribution-tests examples bom + coverage-report UTF-8 UTF-8 + + 1.4.8 2.2.2 @@ -101,6 +111,17 @@ false false + + false @@ -712,7 +733,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 @@ -770,24 +791,16 @@ - - org.jacoco - jacoco-maven-plugin - - - - prepare-agent - - - - report - prepare-package - - report - - - - + maven-surefire-plugin @@ -1040,6 +1053,39 @@ height="0" width="0" style="display:none;visibility:hidden"> + + + coverage + + + + org.jacoco + jacoco-maven-plugin + + + + prepare-agent + + + + report + prepare-package + + report + + + + + + + fast