fix code coverage calculation (opensearch-project#980)

Signed-off-by: Subhobrata Dey <[email protected]>

.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

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)

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

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 {

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);
+        }
+    }
 }