diff --git a/build-tools/knnplugin-coverage.gradle b/build-tools/knnplugin-coverage.gradle index 8a5c5c73b..eb3582dab 100644 --- a/build-tools/knnplugin-coverage.gradle +++ b/build-tools/knnplugin-coverage.gradle @@ -3,12 +3,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +apply plugin: 'jacoco' + +jacoco { + toolVersion = "0.8.10" +} + /** - * ES Plugin build tools don't work with the Gradle Jacoco Plugin to report coverage out of the box. - * https://github.com/elastic/elasticsearch/issues/28867. - * - * This code sets up coverage reporting manually for ES plugin tests. This is complicated because: - * 1. The ES integTest Task doesn't implement Gradle's JavaForkOptions so we have to manually start the jacoco agent with the test JVM + * This code sets up coverage reporting manually for the k-NN plugin tests. This is complicated because: + * 1. The OS integTest Task doesn't implement Gradle's JavaForkOptions so we have to manually start the jacoco agent with the test JVM * 2. The cluster nodes are stopped using 'kill -9' which means jacoco can't dump it's execution output to a file on VM shutdown * 3. The Java Security Manager prevents JMX from writing execution output to the file. * @@ -16,44 +19,18 @@ * cluster is stopped and dump it to a file. Luckily our current security policy seems to allow this. This will also probably * break if there are multiple nodes in the integTestCluster. But for now... it sorta works. */ - -import org.apache.tools.ant.taskdefs.condition.Os -apply plugin: 'jacoco' - -// Get gradle to generate the required jvm agent arg for us using a dummy tasks of type Test. Unfortunately Elastic's -// testing tasks don't derive from Test so the jacoco plugin can't do this automatically. -def jacocoDir = "${buildDir}/jacoco" -task dummyTest(type: Test) { - enabled = false - workingDir = file("/") // Force absolute path to jacoco agent jar - jacoco { - destinationFile = file("${jacocoDir}/test.exec") - destinationFile.parentFile.mkdirs() - jmx = true - } -} - -task dummyIntegTest(type: Test) { - enabled = false - workingDir = file("/") // Force absolute path to jacoco agent jar +integTest { jacoco { - destinationFile = file("${jacocoDir}/integTest.exec") - destinationFile.parentFile.mkdirs() jmx = true } -} -integTest { - systemProperty 'jacoco.dir', "${jacocoDir}" + systemProperty 'jacoco.dir', project.layout.buildDirectory.get().file("jacoco").asFile.absolutePath systemProperty 'jmx.serviceUrl', "service:jmx:rmi:///jndi/rmi://127.0.0.1:7777/jmxrmi" } jacocoTestReport { dependsOn integTest, test - executionData.from = [dummyTest.jacoco.destinationFile, dummyIntegTest.jacoco.destinationFile] - sourceDirectories.from = sourceSets.main.java.sourceDirectories - classDirectories.from = files(sourceSets.main.java.classesDirectory) - + executionData.from = [integTest.jacoco.destinationFile, test.jacoco.destinationFile] reports { html.getRequired().set(true) // human readable csv.getRequired().set(true) @@ -61,21 +38,12 @@ jacocoTestReport { } } -afterEvaluate { - jacocoTestReport.dependsOn integTest +testClusters.integTest { + jvmArgs " ${integTest.jacoco.getAsJvmArg()}" - testClusters.integTest { - if (Os.isFamily(Os.FAMILY_WINDOWS)) { - // Replacing build with absolute path to fix the error "error opening zip file or JAR manifest missing : /build/tmp/expandedArchives/..../jacocoagent.jar" - jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('build',"${buildDir}") - } else { - jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('javaagent:','javaagent:/') - } - - systemProperty 'com.sun.management.jmxremote', "true" - systemProperty 'com.sun.management.jmxremote.authenticate', "false" - systemProperty 'com.sun.management.jmxremote.port', "7777" - systemProperty 'com.sun.management.jmxremote.ssl', "false" - systemProperty 'java.rmi.server.hostname', "127.0.0.1" - } + systemProperty 'com.sun.management.jmxremote', "true" + systemProperty 'com.sun.management.jmxremote.authenticate', "false" + systemProperty 'com.sun.management.jmxremote.port', "7777" + systemProperty 'com.sun.management.jmxremote.ssl', "false" + systemProperty 'java.rmi.server.hostname', "127.0.0.1" } diff --git a/build.gradle b/build.gradle index 1fec2d69f..9260c00f9 100644 --- a/build.gradle +++ b/build.gradle @@ -36,7 +36,6 @@ plugins { id 'java-library' id 'java-test-fixtures' id 'idea' - id 'jacoco' id "com.diffplug.spotless" version "6.3.0" apply false id 'io.freefair.lombok' version '8.4' } @@ -135,10 +134,6 @@ if (!usingRemoteCluster) { } } -jacoco { - toolVersion = "0.8.7" -} - check.dependsOn spotlessCheck check.dependsOn jacocoTestReport diff --git a/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java b/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java index 011ec712c..63ed17f2d 100644 --- a/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java +++ b/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java @@ -10,6 +10,8 @@ import com.google.common.primitives.Floats; import org.apache.commons.lang.StringUtils; import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.xcontent.DeprecationHandler; @@ -52,7 +54,6 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; @@ -118,6 +119,8 @@ public class KNNRestTestCase extends ODFERestTestCase { protected static final int NUM_OF_ATTEMPTS = 30; private static final String SYSTEM_INDEX_PREFIX = ".opendistro"; + private static final Logger logger = LogManager.getLogger(KNNRestTestCase.class); + @AfterClass public static void dumpCoverage() throws IOException, MalformedObjectNameException { // jacoco.dir is set in esplugin-coverage.gradle, if it doesn't exist we don't @@ -136,9 +139,10 @@ public static void dumpCoverage() throws IOException, MalformedObjectNameExcepti false ); - Path path = Paths.get(jacocoBuildPath + "/integTest.exec"); + Path path = Path.of(jacocoBuildPath, "integTest.exec"); Files.write(path, proxy.getExecutionData(false)); } catch (Exception ex) { + logger.error("Failed to dump coverage: ", ex); throw new RuntimeException("Failed to dump coverage: " + ex); } }