fix Jacoco integration issue by updating internal testing infrastruct…

…ure (#391)

* fix jacoco integration by updating internal testing infrastructure

build-tools/sqlplugin-coverage.gradle

@@ -39,7 +39,6 @@ task dummyTest(type: Test) {
     }
 }
 
-/*
 task dummyIntegTest(type: Test) {
     enabled = false
     workingDir = file("/") // Force absolute path to jacoco agent jar
@@ -51,18 +50,21 @@ task dummyIntegTest(type: Test) {
 }
 
 integTest.runner {
-    jvmArgs += " ${dummyIntegTest.jacoco.getAsJvmArg()}"
+    systemProperty 'jacoco.dir', "${buildDir}/jacoco"
+}
+
+testClusters.integTest {
+    jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}"
     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"
 }
-*/
 
 jacocoTestReport {
     dependsOn integTest, test
-    executionData.from = [dummyTest.jacoco.destinationFile/*, dummyIntegTest.jacoco.destinationFile*/]
+    executionData.from = [dummyTest.jacoco.destinationFile, dummyIntegTest.jacoco.destinationFile]
     sourceDirectories.from = sourceSets.main.java.sourceDirectories
     classDirectories.from = files(sourceSets.main.java.outputDir)
 
@@ -73,29 +75,6 @@ jacocoTestReport {
     }
 }
 
-/*
-// See https://www.eclemma.org/jacoco/trunk/doc/api/org/jacoco/agent/rt/IAgent.html
-task dumpCoverage {
-    onlyIf {
-        // ignore the integ test coverage result when integTestRunner failed.
-        integTest.runner.getState().getFailure() == null
-    }
-    doFirst () {
-        def serverUrl = "service:jmx:rmi:///jndi/rmi://127.0.0.1:7777/jmxrmi"
-        def connector = JMXConnectorFactory.connect(new JMXServiceURL(serverUrl))
-        try {
-            def jacocoMBean = new GroovyMBean(connector.MBeanServerConnection, "org.jacoco:type=Runtime")
-            byte[] data = jacocoMBean.getExecutionData(false)
-            file(dummyIntegTest.jacoco.destinationFile).setBytes(data)
-        } finally {
-            connector.close()
-        }
-    }
-}
-
 project.gradle.projectsEvaluated {
-    integTest.runner.finalizedBy dumpCoverage
-    tasks['integTest.runner#stop'].dependsOn dumpCoverage
-    jacocoTestReport.dependsOn dumpCoverage
-}
-*/
+    jacocoTestReport.dependsOn integTest.runner
+}

build.gradle

@@ -143,7 +143,7 @@ integTest.runner {
 
     // Tell the test JVM if the cluster JVM is running under a debugger so that tests can use longer timeouts for
     // requests. The 'doFirst' delays reading the debug setting on the cluster till execution time.
-    // doFirst { systemProperty 'cluster.debug', integTestCluster.debug }
+    doFirst { systemProperty 'cluster.debug', getDebug()}
 
     // The --debug-jvm command-line option makes the cluster debuggable; this makes the tests debuggable
     if (System.getProperty("test.debug") != null) {

src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLIntegTestCase.java

@@ -25,10 +25,19 @@
 import org.junit.Assert;
 import org.junit.Before;
 
+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.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Locale;
 
 import static com.amazon.opendistroforelasticsearch.sql.esintgtest.TestUtils.createIndexByRestClient;
@@ -83,6 +92,40 @@ protected boolean preserveClusterUponCompletion() {
         return true; // Preserve test index, template and settings between test cases
     }
 
+    /**
+     * 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 sqlplugin-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 (jacocoBuildPath.isEmpty()) {
+            return;
+        }
+
+        String serverUrl = "service:jmx:rmi:///jndi/rmi://127.0.0.1:7777/jmxrmi";
+        JMXConnector connector = JMXConnectorFactory.connect(new JMXServiceURL(serverUrl));
+        IProxy proxy = MBeanServerInvocationHandler.newProxyInstance(
+                connector.getMBeanServerConnection(), new ObjectName("org.jacoco:type=Runtime"), IProxy.class,
+                false);
+
+        Path path = Paths.get(jacocoBuildPath + "/integTest.exec");
+        Files.write(path, proxy.getExecutionData(false));
+        connector.close();
+    }
+
     /**
      * As JUnit JavaDoc says:
      *  "The @AfterClass methods declared in superclasses will be run after those of the current class."