From a5a5ea07fa4fb8e98afe48e1095934f06c2b6b16 Mon Sep 17 00:00:00 2001 From: stanbrub Date: Tue, 21 Jan 2025 17:12:35 -0700 Subject: [PATCH 1/5] Java coverage is now generated conditionally --- buildSrc/src/main/groovy/TestTools.groovy | 22 ------------------- .../groovy/io.deephaven.coverage-merge.gradle | 22 +++++++++++++++++++ ...o.deephaven.java-jacoco-conventions.gradle | 21 ++++++++++++------ .../io.deephaven.java-test-conventions.gradle | 6 ++--- 4 files changed, 39 insertions(+), 32 deletions(-) create mode 100644 buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle diff --git a/buildSrc/src/main/groovy/TestTools.groovy b/buildSrc/src/main/groovy/TestTools.groovy index 2c1ffc67db7..5754afcf6ce 100644 --- a/buildSrc/src/main/groovy/TestTools.groovy +++ b/buildSrc/src/main/groovy/TestTools.groovy @@ -116,28 +116,6 @@ By default only runs in CI; to run locally: .replace "${separator}test$separator", "$separator$type$separator" (report as SimpleReport).outputLocation.set new File(rebased) } - // this is not part of the standard class; it is glued on later by jacoco plugin; - // we want to give each test it's own output files for jacoco analysis, - // so we don't accidentally stomp on previous output. - // TODO: verify jenkins is analyzing _all_ information here. - if (project.findProperty('jacoco.enabled') == "true") { - (t['jacoco'] as JacocoTaskExtension).with { - destinationFile = project.provider({ new File(project.buildDir, "jacoco/${type}.exec".toString()) } as Callable) - classDumpDir = new File(project.buildDir, "jacoco/${type}Dumps".toString()) - } - (project['jacocoTestReport'] as JacocoReport).with { - reports { - JacocoReportsContainer c -> - c.xml.enabled = true - c.csv.enabled = true - c.html.enabled = true - } - } - } - - } - if (project.findProperty('jacoco.enabled') == "true") { - project.tasks.findByName('jacocoTestReport').mustRunAfter(t) } return t diff --git a/buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle b/buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle new file mode 100644 index 00000000000..3d9c83d6c1f --- /dev/null +++ b/buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle @@ -0,0 +1,22 @@ +plugins { + id 'base' + id 'jacoco-report-aggregation' +} + +jacoco { + toolVersion = '0.8.12' +} + +tasks.register("coverage-merge", JacocoReport) { + def jprojects = allprojects.findAll { p-> p.plugins.hasPlugin('java') } + additionalSourceDirs = files(jprojects.sourceSets.main.allSource.srcDirs) + sourceDirectories = files(jprojects.sourceSets.main.allSource.srcDirs) + classDirectories = files(jprojects.sourceSets.main.output) + reports { + html.required = true + csv.required = true + xml.required = false + } + def projRootDir = project.rootDir.absolutePath + executionData fileTree(projRootDir).include("**/build/jacoco/*.exec") +} diff --git a/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle index b87ec489798..903181a5759 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle @@ -4,17 +4,24 @@ plugins { } jacoco { - toolVersion = '0.8.8' + toolVersion = '0.8.12' +} + +test { + jacoco { + destinationFile = layout.buildDirectory.file('jacoco/jacoco.exec').get().asFile + } + finalizedBy jacocoTestReport } jacocoTestReport { + dependsOn test reports { - xml.enabled true - csv.enabled true - html.enabled true + csv.required = true + csv.destination = layout.buildDirectory.file('reports/jacoco/java-coverage.csv').get().asFile + xml.required = true + xml.outputLocation = layout.buildDirectory.file('reports/jacoco/java-coverage.xml').get().asFile + html.outputLocation = layout.buildDirectory.dir('reports/jacoco/html') } } -project.tasks.withType(Test).all { Test t -> - finalizedBy jacocoTestReport -} diff --git a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle index c39b90631ab..dbc209e582f 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle @@ -2,10 +2,10 @@ plugins { id 'java' } -if (project.findProperty('jacoco.enabled') == 'true') { - // Only load if jacoco enabled; otherwise - // instrumentation of the code is still done. +// Apply Jacoco instrumentation and coverage reporting +if (project.findProperty('coverage.enabled') == 'true') { project.apply plugin: 'io.deephaven.java-jacoco-conventions' + rootProject.apply plugin: 'io.deephaven.coverage-merge' } def testJar = project.tasks.register 'testJar', Jar, { Jar jar -> From f4605deeac5076bd667584e89c1cac28e5e268e4 Mon Sep 17 00:00:00 2001 From: stanbrub Date: Fri, 24 Jan 2025 13:29:42 -0700 Subject: [PATCH 2/5] Reworked coverage merge into its own project --- ...o.deephaven.java-jacoco-conventions.gradle | 22 ++++----- .../io.deephaven.java-test-conventions.gradle | 1 - coverage/README.md | 47 +++++++++++++++++++ .../build.gradle | 18 +++++-- coverage/exclude-packages.txt | 5 ++ coverage/gather-coverage.py | 46 ++++++++++++++++++ coverage/gradle.properties | 1 + settings.gradle | 1 + 8 files changed, 123 insertions(+), 18 deletions(-) create mode 100644 coverage/README.md rename buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle => coverage/build.gradle (50%) create mode 100644 coverage/exclude-packages.txt create mode 100644 coverage/gather-coverage.py create mode 100644 coverage/gradle.properties diff --git a/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle index 903181a5759..be609e85c62 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle @@ -7,21 +7,17 @@ jacoco { toolVersion = '0.8.12' } -test { - jacoco { - destinationFile = layout.buildDirectory.file('jacoco/jacoco.exec').get().asFile - } - finalizedBy jacocoTestReport -} - jacocoTestReport { - dependsOn test + sourceSets sourceSets.main + executionData = fileTree(buildDir).include("/jacoco/*.exec") reports { - csv.required = true - csv.destination = layout.buildDirectory.file('reports/jacoco/java-coverage.csv').get().asFile - xml.required = true - xml.outputLocation = layout.buildDirectory.file('reports/jacoco/java-coverage.xml').get().asFile - html.outputLocation = layout.buildDirectory.dir('reports/jacoco/html') + csv.required = true + xml.required = true + html.required = true } } +task coverage { + dependsOn jacocoTestReport +} + diff --git a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle index dbc209e582f..9ea0f846031 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle @@ -5,7 +5,6 @@ plugins { // Apply Jacoco instrumentation and coverage reporting if (project.findProperty('coverage.enabled') == 'true') { project.apply plugin: 'io.deephaven.java-jacoco-conventions' - rootProject.apply plugin: 'io.deephaven.coverage-merge' } def testJar = project.tasks.register 'testJar', Jar, { Jar jar -> diff --git a/coverage/README.md b/coverage/README.md new file mode 100644 index 00000000000..b2addb5faf0 --- /dev/null +++ b/coverage/README.md @@ -0,0 +1,47 @@ +# Overview + +Code coverage for Deephaven Community Core manages multiple languages like Java, Python, R, Go and C++. This is handled in the gradle build at the individual project level but also supports collection of normalized results rolled up to the top level. For convenience, both top-level Java HTML and a top-level all-language CSV are created. + +## Running for Coverage + +A typical run looks like the following that is run from the root of the multi-project build +``` +./gradlew -Pcoverage.enabled=true check +./gradlew -Pcoverage.enabled=true testSerial +./gradlew -Pcoverage.enabled=true testParallel +./gradlew -Pcoverage.enabled=true testOutOfBand +./gradlew -Pcoverage.enabled=true coverage +./gradlew -Pcoverage.enabled=true coverage-merge +``` +Running the second command is not contingent upon the first command succeeding. It merely collects what coverage is available. + +## Result Files + +Results for individual project coverage are stored in the project's _build_ output directory. Depending on the language and coverage tools, there will be different result files with slightly different locations and names. For example, Java coverage could produce a binary _jacoco.exec_ file, while python coverage produces a tabbed text file. + +Aggregated results produce a merged CSV file for each language under the top-level _build_ directory. Those CSV files are further merged into one _all-coverage.csv_. + +## Exclusion Filters + +In some cases, there may be a need to exclude some packages from coverage, even though they may be used during testing. For example, some Java classes used in GRPC are generated. The expectation is that the generator mechanism has already been tested and should produce viable classes. Including coverage for those classes in the results as zero coverage causes unnecessary noise and makes it harder to track coverage overall. + +To avoid unneeded coverage, the file _exclude-packages.txt_ can be used. This is a list of values to be excluded if they match the "Package" column in the coverage CSV. These are exact values and not wildcards. + +## File Layout + +Top-level Build Directory (Some languages TBD) +- `coverage/` This project's directory + - `gather-coverage.py` Gather and normalize coverage for all languages + - `exclude-packages.txt` A list of packages to exclude from aggregated results +- `buildSrc/src/main/groovy/` + - `io.deephaven.java-jacoco-conventions.gradle` Applied to run coverage on Java projects + - `io.deephaven.java-test-conventions.gradle` Applies the above conditionally base on the _coverage.enabled_ property +- `coverage/build/reports/coverage/` + - `java-coverage.csv` Normalized coverage from all Java projects + - `python-coverage.py` Normalized coverage from all Python projects + - `cplus-coverage.py` Normalized coverage from all C++ projects + - `r-coverage.py` Normalized coverage from all R projects + - `go-coverage.oy` Normalized coverage from all Go projects + - `all-coverage.csv` Normalized and filtered coverage from all covered projects +- `coverage/build/reports/jacoco/jacoco-merge/html/` + - `index.html` Root file to view Java coverage down to the branch level (not filtered) diff --git a/buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle b/coverage/build.gradle similarity index 50% rename from buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle rename to coverage/build.gradle index 3d9c83d6c1f..4c12bf5cdb9 100644 --- a/buildSrc/src/main/groovy/io.deephaven.coverage-merge.gradle +++ b/coverage/build.gradle @@ -1,5 +1,6 @@ plugins { - id 'base' + id 'io.deephaven.project.register' + id 'java' id 'jacoco-report-aggregation' } @@ -7,8 +8,8 @@ jacoco { toolVersion = '0.8.12' } -tasks.register("coverage-merge", JacocoReport) { - def jprojects = allprojects.findAll { p-> p.plugins.hasPlugin('java') } +tasks.register("jacoco-merge", JacocoReport) { + def jprojects = rootProject.allprojects.findAll { p-> p.plugins.hasPlugin('java') } additionalSourceDirs = files(jprojects.sourceSets.main.allSource.srcDirs) sourceDirectories = files(jprojects.sourceSets.main.allSource.srcDirs) classDirectories = files(jprojects.sourceSets.main.output) @@ -17,6 +18,15 @@ tasks.register("coverage-merge", JacocoReport) { csv.required = true xml.required = false } - def projRootDir = project.rootDir.absolutePath + def projRootDir = rootProject.rootDir.absolutePath executionData fileTree(projRootDir).include("**/build/jacoco/*.exec") } + +tasks.register("coverage-merge", Exec) { + dependsOn("jacoco-merge") + def projDir = projectDir.absolutePath + def script = projDir + '/gather-coverage.py' + commandLine 'python', script, projDir + standardOutput = System.out +} + diff --git a/coverage/exclude-packages.txt b/coverage/exclude-packages.txt new file mode 100644 index 00000000000..02a550e5024 --- /dev/null +++ b/coverage/exclude-packages.txt @@ -0,0 +1,5 @@ +io.deephaven.tuple.generated +io.deephaven.engine.table.impl.tuplesource.generated +io.deephaven.proto.backplane.grpc +io.deephaven.proto.backplane.script.grpc +io.deephaven.proto diff --git a/coverage/gather-coverage.py b/coverage/gather-coverage.py new file mode 100644 index 00000000000..76daa4a38d9 --- /dev/null +++ b/coverage/gather-coverage.py @@ -0,0 +1,46 @@ +# +# Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending +# +import sys, glob, csv, os, shutil + +# Aggregate coverage data for all languages. Each language has a different way of doing +# coverage and each normalization mechanism is used here. Class/file exclusions are +# handled here, since coverage tools are inconsistent or non-functional in that regard. + +proj_root_dir = sys.argv[1] +script_dir = os.path.dirname(os.path.abspath(__file__)) +coverage_dir = proj_root_dir + '/build/reports/coverage' +coverage_output_path = coverage_dir + '/all-coverage.csv' +coverage_input_glob = coverage_dir + '/*-coverage.csv' +exclude_path = script_dir + '/exclude-packages.txt' + +if os.path.exists(coverage_dir): + shutil.rmtree(coverage_dir) +os.makedirs(coverage_dir) + +# Aggregate and normalize coverage for java projects +input_glob = proj_root_dir + '/build/reports/jacoco/jacoco-merge/jacoco-merge.csv' +with open(f'{coverage_dir}/java-coverage.csv', 'w', newline='') as outfile: + csv_writer = csv.writer(outfile) + csv_writer.writerow(['Language','Project','Package','Class','Missed','Covered']) + for filename in glob.glob(input_glob, recursive = True): + with open(filename, 'r') as csv_in: + csv_reader = csv.reader(csv_in) + next(csv_reader, None) + for row in csv_reader: + new_row = ['java',row[0],row[1],row[2],row[3],row[4]] + csv_writer.writerow(new_row) + +# Load packages to be excluded from the aggregated coverage CSV +with open(exclude_path) as f: + excludes = [line.strip() for line in f] + +# Collect coverage CSVs into a single CSV without lines containing exclusions +with open(coverage_output_path, 'w', newline='') as outfile: + csv_writer = csv.writer(outfile) + for csv_file in glob.glob(coverage_input_glob): + with open(csv_file, 'r') as csv_in: + for row in csv.reader(csv_in): + if row[2] in excludes: continue + new_row = [row[0],row[1],row[2],row[3],row[4],row[5]] + csv_writer.writerow(new_row) diff --git a/coverage/gradle.properties b/coverage/gradle.properties new file mode 100644 index 00000000000..8f42cc0940f --- /dev/null +++ b/coverage/gradle.properties @@ -0,0 +1 @@ +io.deephaven.project.ProjectType=BASIC diff --git a/settings.gradle b/settings.gradle index e175c52b279..36a91a285b9 100644 --- a/settings.gradle +++ b/settings.gradle @@ -51,6 +51,7 @@ project(':configs').projectDir = file('props/configs') include(':test-configs') project(':test-configs').projectDir = file('props/test-configs') +include 'coverage' include 'combined-javadoc' include 'grpc-java:grpc-servlet-jakarta' From 9968609e8370e704c1dea23603e873c3791d02b4 Mon Sep 17 00:00:00 2001 From: stanbrub Date: Thu, 6 Feb 2025 11:54:41 -0700 Subject: [PATCH 3/5] Apply jacoco plugin always, but turn functionality on and off --- .../main/groovy/io.deephaven.java-jacoco-conventions.gradle | 5 ++--- .../main/groovy/io.deephaven.java-test-conventions.gradle | 6 ++---- coverage/build.gradle | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle index be609e85c62..8e4391a84a3 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-jacoco-conventions.gradle @@ -17,7 +17,6 @@ jacocoTestReport { } } -task coverage { - dependsOn jacocoTestReport +project.tasks.withType(Test).configureEach { Test t -> + t.jacoco.enabled = (project.findProperty('coverage.enabled') == 'true') } - diff --git a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle index 9ea0f846031..dec19c2b04b 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle @@ -2,10 +2,8 @@ plugins { id 'java' } -// Apply Jacoco instrumentation and coverage reporting -if (project.findProperty('coverage.enabled') == 'true') { - project.apply plugin: 'io.deephaven.java-jacoco-conventions' -} +// Apply Jacoco coverage instrumentation (This can be disabled with -Pcoverage.enabled=false) +project.apply plugin: 'io.deephaven.java-jacoco-conventions' def testJar = project.tasks.register 'testJar', Jar, { Jar jar -> jar.from project.sourceSets.test.output diff --git a/coverage/build.gradle b/coverage/build.gradle index 4c12bf5cdb9..c9394694888 100644 --- a/coverage/build.gradle +++ b/coverage/build.gradle @@ -1,7 +1,7 @@ plugins { id 'io.deephaven.project.register' id 'java' - id 'jacoco-report-aggregation' + id 'jacoco' } jacoco { From 164ad41aa42f19a3b41efaaa15bc1efb03afa3d1 Mon Sep 17 00:00:00 2001 From: stanbrub Date: Thu, 6 Feb 2025 14:38:28 -0700 Subject: [PATCH 4/5] Updated test coverage task name --- coverage/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coverage/README.md b/coverage/README.md index b2addb5faf0..19dd1ee042c 100644 --- a/coverage/README.md +++ b/coverage/README.md @@ -10,7 +10,7 @@ A typical run looks like the following that is run from the root of the multi-pr ./gradlew -Pcoverage.enabled=true testSerial ./gradlew -Pcoverage.enabled=true testParallel ./gradlew -Pcoverage.enabled=true testOutOfBand -./gradlew -Pcoverage.enabled=true coverage +./gradlew -Pcoverage.enabled=true jacocoTestReport ./gradlew -Pcoverage.enabled=true coverage-merge ``` Running the second command is not contingent upon the first command succeeding. It merely collects what coverage is available. From 4c5cebd82e0f6ae73d9b862bcc9d268beedc7942 Mon Sep 17 00:00:00 2001 From: stanbrub Date: Thu, 6 Feb 2025 15:06:43 -0700 Subject: [PATCH 5/5] moved java-jacoco-conventions to plugins --- .../src/main/groovy/io.deephaven.java-test-conventions.gradle | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle index dec19c2b04b..4f274286623 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle @@ -1,10 +1,8 @@ plugins { id 'java' + id 'io.deephaven.java-jacoco-conventions' } -// Apply Jacoco coverage instrumentation (This can be disabled with -Pcoverage.enabled=false) -project.apply plugin: 'io.deephaven.java-jacoco-conventions' - def testJar = project.tasks.register 'testJar', Jar, { Jar jar -> jar.from project.sourceSets.test.output jar.archiveClassifier = 'test'