From 743e538d8014cf33db6d9b7cde2cb5073a595608 Mon Sep 17 00:00:00 2001 From: Subhobrata Dey Date: Fri, 12 Apr 2024 20:32:12 +0000 Subject: [PATCH] fix code coverage calculation Signed-off-by: Subhobrata Dey --- build-tools/opensearchplugin-coverage.gradle | 62 +++++++++++++++---- build.gradle | 6 +- .../SecurityAnalyticsRestTestCase.java | 47 ++++++++++++++ 3 files changed, 101 insertions(+), 14 deletions(-) diff --git a/build-tools/opensearchplugin-coverage.gradle b/build-tools/opensearchplugin-coverage.gradle index b5b176a39..b3bb32649 100644 --- a/build-tools/opensearchplugin-coverage.gradle +++ b/build-tools/opensearchplugin-coverage.gradle @@ -3,24 +3,26 @@ * SPDX-License-Identifier: Apache-2.0 */ +apply plugin: 'jacoco' + /** - * OpenSearch Plugin build tools don't work with the Gradle Jacoco Plugin to report coverage out of the box. + * 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 OpenSearch plugin tests. This is complicated because: - * 1. The OpenSearch 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 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 * 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. * * To workaround these we start the cluster with jmx enabled and then use Jacoco's JMX MBean to get the execution data before the * 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. + * break if there are multiple nodes in the integTestCluster. But for now... it sorta works. */ -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 @@ -31,19 +33,53 @@ task dummyTest(type: Test) { } } +task dummyIntegTest(type: Test) { + enabled = false + workingDir = file("/") // Force absolute path to jacoco agent jar + jacoco { + destinationFile = file("${jacocoDir}/integTest.exec") + destinationFile.parentFile.mkdirs() + jmx = true + } +} +task dummyIntegTestRunner(type: Test) { + enabled = false + workingDir = file("/") // Force absolute path to jacoco agent jar + jacoco { + destinationFile = file("${jacocoDir}/integTestRunner.exec") + destinationFile.parentFile.mkdirs() + jmx = true + } +} + +integTest { + systemProperty 'jacoco.dir', "${jacocoDir}" +} + jacocoTestReport { - dependsOn test - executionData dummyTest.jacoco.destinationFile - getSourceDirectories().from(sourceSets.main.allSource) - getClassDirectories().from(sourceSets.main.output) + dependsOn integTest, test + executionData.from dummyTest.jacoco.destinationFile, dummyIntegTest.jacoco.destinationFile, dummyIntegTestRunner.jacoco.destinationFile + sourceDirectories.from = "src/main/java" + classDirectories.from = sourceSets.main.output reports { html.required = true // human readable + csv.required = true xml.required = true // for coverlay } } -project.gradle.projectsEvaluated { - jacocoTestReport.dependsOn test -} -check.dependsOn jacocoTestReport +allprojects { + afterEvaluate { + jacocoTestReport.dependsOn integTest + + testClusters.integTest { + 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" + } + } +} \ No newline at end of file diff --git a/build.gradle b/build.gradle index 361d4120f..4fb5beebf 100644 --- a/build.gradle +++ b/build.gradle @@ -46,7 +46,6 @@ apply plugin: 'opensearch.opensearchplugin' apply plugin: 'opensearch.testclusters' apply plugin: 'opensearch.java-rest-test' apply plugin: 'opensearch.pluginzip' -apply from: 'build-tools/opensearchplugin-coverage.gradle' apply from: 'gradle/formatting.gradle' ext { @@ -306,6 +305,11 @@ testClusters.integTest { plugins.add(firstPlugin) } } +def usingRemoteCluster = System.properties.containsKey('tests.rest.cluster') || System.properties.containsKey('tests.cluster') +def usingMultiNode = project.properties.containsKey('numNodes') +if (!usingRemoteCluster && !usingMultiNode) { + apply from: 'build-tools/opensearchplugin-coverage.gradle' +} run { doFirst { diff --git a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java index 20e98c8db..e7da36705 100644 --- a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java +++ b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java @@ -4,6 +4,7 @@ */ package org.opensearch.securityanalytics; +import java.nio.file.Files; import java.util.Set; import java.util.ArrayList; import java.util.function.BiConsumer; @@ -16,6 +17,7 @@ import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.io.entity.StringEntity; import org.apache.hc.core5.http.message.BasicHeader; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.After; import org.junit.Before; @@ -69,6 +71,12 @@ import org.opensearch.test.rest.OpenSearchRestTestCase; +import javax.management.MBeanServerInvocationHandler; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectName; +import javax.management.remote.JMXConnector; +import javax.management.remote.JMXConnectorFactory; +import javax.management.remote.JMXServiceURL; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -1786,4 +1794,43 @@ public String getMatchAllSearchRequestString(int num) { " }\n" + "}"; } + + /** + * We need to be able to dump the jacoco coverage before cluster is shut down. + * The new internal testing framework removed some of the gradle tasks we were listening to + * to choose a good time to do it. This will dump the executionData to file after each test. + * TODO: This is also currently just overwriting integTest.exec with the updated execData without + * resetting after writing each time. This can be improved to either write an exec file per test + * or by letting jacoco append to the file + */ + public interface IProxy { + byte[] getExecutionData(boolean reset); + + void dump(boolean reset); + + void reset(); + } + + + @AfterClass + public static void dumpCoverage() throws IOException, MalformedObjectNameException { + // jacoco.dir is set in esplugin-coverage.gradle, if it doesn't exist we don't + // want to collect coverage so we can return early + String jacocoBuildPath = System.getProperty("jacoco.dir"); + if (Strings.isNullOrEmpty(jacocoBuildPath)) { + return; + } + + String serverUrl = "service:jmx:rmi:///jndi/rmi://127.0.0.1:7777/jmxrmi"; + try (JMXConnector connector = JMXConnectorFactory.connect(new JMXServiceURL(serverUrl))) { + IProxy proxy = MBeanServerInvocationHandler.newProxyInstance( + connector.getMBeanServerConnection(), new ObjectName("org.jacoco:type=Runtime"), IProxy.class, + false); + + Path path = org.opensearch.common.io.PathUtils.get(jacocoBuildPath + "/integTestRunner.exec"); + Files.write(path, proxy.getExecutionData(false)); + } catch (Exception ex) { + throw new RuntimeException("Failed to dump coverage: " + ex); + } + } } \ No newline at end of file