From 13eb3d2d341516e2cd1fe900d191b5c03d1870ce Mon Sep 17 00:00:00 2001 From: Subhobrata Dey Date: Wed, 8 May 2024 18:33:28 -0700 Subject: [PATCH] [Backport 2.x] fix code coverage calculation --- .github/workflows/ci.yml | 2 +- README.md | 2 + build-tools/opensearchplugin-coverage.gradle | 57 +++++++++++++++---- build.gradle | 6 +- .../SecurityAnalyticsRestTestCase.java | 47 +++++++++++++++ 5 files changed, 102 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a4d0ca7d..1b27e28d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: os: [ windows-latest, macos-latest ] include: - os: windows-latest - os_build_args: -x jacocoTestReport + os_build_args: -x integTest working_directory: X:\ os_java_options: -Xmx4096M - os: macos-latest diff --git a/README.md b/README.md index a12c4ed4e..7e6b3dd8c 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,5 @@ +[![Test Workflow](https://github.com/opensearch-project/security-analytics/workflows/Test%20Workflow/badge.svg)](https://github.com/opensearch-project/security-analytics/actions) +[![codecov](https://codecov.io/gh/opensearch-project/security-analytics/branch/main/graph/badge.svg)](https://codecov.io/gh/opensearch-project/security-analytics) ![Documentation](https://img.shields.io/badge/api-reference-blue.svg) ![Chat](https://img.shields.io/badge/chat-on%20forums-blue) ![PRs welcome!](https://img.shields.io/badge/PRs-welcome!-success) diff --git a/build-tools/opensearchplugin-coverage.gradle b/build-tools/opensearchplugin-coverage.gradle index b5b176a39..55a5eebbc 100644 --- a/build-tools/opensearchplugin-coverage.gradle +++ b/build-tools/opensearchplugin-coverage.gradle @@ -3,9 +3,10 @@ * 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. - * 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 @@ -14,13 +15,13 @@ * * 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 +32,55 @@ 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 + +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" + } + } } -check.dependsOn jacocoTestReport +check.dependsOn jacocoTestReport \ No newline at end of file diff --git a/build.gradle b/build.gradle index d08ceb25e..90db45811 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 { @@ -310,6 +309,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 fcdef90ba..d2216415d 100644 --- a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java +++ b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java @@ -12,6 +12,7 @@ import org.apache.http.entity.StringEntity; import org.apache.http.message.BasicHeader; import org.junit.After; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse; @@ -62,9 +63,16 @@ import org.opensearch.securityanalytics.util.CorrelationIndices; 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; +import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; @@ -1783,4 +1791,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