Integ Test Refactoring (#1383) (#1393)

* Integ Test Refactoring Signed-off-by: Vamsi Manohar <[email protected]> (cherry picked from commit 8f6793b)
opensearch-project · Mar 7, 2023 · 0ccb20a · 0ccb20a
1 parent ecea812
commit 0ccb20a
Show file tree

Hide file tree

Showing 13 changed files with 146 additions and 95 deletions.
diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml
@@ -25,11 +25,11 @@ jobs:
       matrix:
         entry:
           - { os: ubuntu-latest, java: 11 }
-          - { os: windows-latest, java: 11, os_build_args: -x doctest -x integTest -x jacocoTestReport -PbuildPlatform=windows }
-          - { os: macos-latest, java: 11, os_build_args: -x doctest -x integTest -x jacocoTestReport  }
+          - { os: windows-latest, java: 11, os_build_args: -x doctest  -PbuildPlatform=windows }
+          - { os: macos-latest, java: 11}
           - { os: ubuntu-latest, java: 17 }
-          - { os: windows-latest, java: 17, os_build_args: -x doctest -x integTest -x jacocoTestReport  -PbuildPlatform=windows }
-          - { os: macos-latest, java: 17, os_build_args: -x doctest -x integTest -x jacocoTestReport  }
+          - { os: windows-latest, java: 17, os_build_args: -x doctest -PbuildPlatform=windows }
+          - { os: macos-latest, java: 17 }
     runs-on: ${{ matrix.entry.os }}
 
     steps:

diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,5 @@
 *.class
+*.http
 .settings/
 # Mobile Tools for Java (J2ME)
 .mtj.tmp/
@@ -33,7 +34,6 @@ gen/
 # git mergetool artifact
 *.orig
 gen
-*.tokens
 
 # Python
 */.venv

diff --git a/build-tools/sqlplugin-coverage.gradle b/build-tools/sqlplugin-coverage.gradle
@@ -16,6 +16,7 @@
  *  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
@@ -45,7 +46,12 @@ integTest.runner {
 }
 
 testClusters.integTest {
-    jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}"
+    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"

diff --git a/buildSrc/src/main/groovy/com/wiredforcode/spawn/KillProcessTask.groovy b/buildSrc/src/main/groovy/com/wiredforcode/spawn/KillProcessTask.groovy
@@ -1,5 +1,6 @@
 package com.wiredforcode.gradle.spawn
 
+import org.apache.tools.ant.taskdefs.condition.Os
 import org.gradle.api.tasks.TaskAction
 
 class KillProcessTask extends DefaultSpawnTask {
@@ -12,7 +13,13 @@ class KillProcessTask extends DefaultSpawnTask {
         }
 
         def pid = pidFile.text
-        def process = "kill $pid".execute()
+        def killCommandLine
+        if (Os.isFamily(Os.FAMILY_WINDOWS)) {
+            killCommandLine = Arrays.asList("taskkill", "/F", "/T", "/PID", "$pid")
+        } else {
+            killCommandLine = Arrays.asList("kill", "$pid")
+        }
+        def process = killCommandLine.execute()
 
         try {
             process.waitFor()

diff --git a/buildSrc/src/main/groovy/com/wiredforcode/spawn/SpawnProcessTask.groovy b/buildSrc/src/main/groovy/com/wiredforcode/spawn/SpawnProcessTask.groovy
@@ -95,9 +95,18 @@ class SpawnProcessTask extends DefaultSpawnTask {
     }
 
     private int extractPidFromProcess(Process process) {
-        def pidField = process.class.getDeclaredField('pid')
-        pidField.accessible = true
-
-        return pidField.getInt(process)
+        def pid
+        try {
+            // works since java 9
+            def pidMethod = process.class.getDeclaredMethod('pid')
+            pidMethod.setAccessible(true)
+            pid = pidMethod.invoke(process)
+        } catch (ignored) {
+            // fallback to UNIX-only implementation
+            def pidField = process.class.getDeclaredField('pid')
+            pidField.accessible = true
+            pid = pidField.getInt(process)
+        }
+        return pid
     }
 }
diff --git a/doctest/build.gradle b/doctest/build.gradle
@@ -56,7 +56,12 @@ task startPrometheus(type: SpawnProcessTask) {
 
 //evaluationDependsOn(':')
 task startOpenSearch(type: SpawnProcessTask) {
-    command "${path}/gradlew -p ${plugin_path} runRestTestCluster"
+    if( getOSFamilyType() == "windows") {
+        command "${path}\\gradlew.bat -p ${plugin_path} runRestTestCluster"
+    }
+    else {
+        command "${path}/gradlew -p ${plugin_path} runRestTestCluster"
+    }
     ready 'started'
 }
 
@@ -94,12 +99,13 @@ task stopPrometheus() {
         }
     }
 }
-
-stopPrometheus.mustRunAfter startPrometheus
+if(getOSFamilyType() != "windows") {
+    stopPrometheus.mustRunAfter startPrometheus
+    startOpenSearch.dependsOn startPrometheus
+    stopOpenSearch.finalizedBy stopPrometheus
+}
 doctest.dependsOn startOpenSearch
-startOpenSearch.dependsOn startPrometheus
 doctest.finalizedBy stopOpenSearch
-stopOpenSearch.finalizedBy stopPrometheus
 check.dependsOn doctest
 clean.dependsOn(cleanBootstrap)
 

diff --git a/integ-test/build.gradle b/integ-test/build.gradle
@@ -22,7 +22,6 @@
  * under the License.
  */
 
-import org.gradle.nativeplatform.platform.internal.DefaultNativePlatform
 import org.opensearch.gradle.test.RestIntegTestTask
 import org.opensearch.gradle.testclusters.StandaloneRestIntegTestTask
 
@@ -151,9 +150,10 @@ stopPrometheus.mustRunAfter startPrometheus
 // Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled
 integTest {
     dependsOn ':opensearch-sql-plugin:bundlePlugin'
-    dependsOn startPrometheus
-    finalizedBy stopPrometheus
-
+    if(getOSFamilyType() != "windows") {
+        dependsOn startPrometheus
+        finalizedBy stopPrometheus
+    }
     systemProperty 'tests.security.manager', 'false'
     systemProperty('project.root', project.projectDir.absolutePath)
 
@@ -178,6 +178,12 @@ integTest {
         }
     }
 
+    if(getOSFamilyType() == "windows") {
+        exclude 'org/opensearch/sql/ppl/PrometheusDataSourceCommandsIT.class'
+        exclude 'org/opensearch/sql/ppl/ShowDataSourcesCommandIT.class'
+        exclude 'org/opensearch/sql/ppl/InformationSchemaCommandIT.class'
+    }
+
     exclude 'org/opensearch/sql/doctest/**/*IT.class'
     exclude 'org/opensearch/sql/correctness/**'
 

diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/CsvFormatIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/CsvFormatIT.java
@@ -11,6 +11,7 @@
 import java.io.IOException;
 import java.util.Locale;
 import org.junit.Test;
+import org.opensearch.sql.common.utils.StringUtils;
 
 public class CsvFormatIT extends PPLIntegTestCase {
 
@@ -23,27 +24,27 @@ public void init() throws IOException {
   public void sanitizeTest() throws IOException {
     String result = executeCsvQuery(
         String.format(Locale.ROOT, "source=%s | fields firstname, lastname", TEST_INDEX_BANK_CSV_SANITIZE));
-    assertEquals(
-        "firstname,lastname\n"
-            + "'+Amber JOHnny,Duke Willmington+\n"
-            + "'-Hattie,Bond-\n"
-            + "'=Nanette,Bates=\n"
-            + "'@Dale,Adams@\n"
-            + "\",Elinor\",\"Ratliff,,,\"\n",
+    assertEquals(StringUtils.format(
+        "firstname,lastname%n"
+            + "'+Amber JOHnny,Duke Willmington+%n"
+            + "'-Hattie,Bond-%n"
+            + "'=Nanette,Bates=%n"
+            + "'@Dale,Adams@%n"
+            + "\",Elinor\",\"Ratliff,,,\"%n"),
         result);
   }
 
   @Test
   public void escapeSanitizeTest() throws IOException {
     String result = executeCsvQuery(
         String.format(Locale.ROOT, "source=%s | fields firstname, lastname", TEST_INDEX_BANK_CSV_SANITIZE), false);
-    assertEquals(
-        "firstname,lastname\n"
-            + "+Amber JOHnny,Duke Willmington+\n"
-            + "-Hattie,Bond-\n"
-            + "=Nanette,Bates=\n"
-            + "@Dale,Adams@\n"
-            + "\",Elinor\",\"Ratliff,,,\"\n",
+    assertEquals(StringUtils.format(
+        "firstname,lastname%n"
+            + "+Amber JOHnny,Duke Willmington+%n"
+            + "-Hattie,Bond-%n"
+            + "=Nanette,Bates=%n"
+            + "@Dale,Adams@%n"
+            + "\",Elinor\",\"Ratliff,,,\"%n"),
         result);
   }
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java
@@ -6,19 +6,17 @@
 
 package org.opensearch.sql.ppl;
 
-import org.json.JSONObject;
-import org.junit.jupiter.api.Test;
-import org.opensearch.client.Request;
-import org.opensearch.client.ResponseException;
-
-import java.io.IOException;
-
 import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG;
 import static org.opensearch.sql.util.MatcherUtils.columnName;
-import static org.opensearch.sql.util.MatcherUtils.rows;
 import static org.opensearch.sql.util.MatcherUtils.verifyColumn;
 import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
 
+import java.io.IOException;
+import org.json.JSONObject;
+import org.junit.jupiter.api.Test;
+import org.opensearch.client.Request;
+import org.opensearch.client.ResponseException;
+
 public class DescribeCommandIT extends PPLIntegTestCase {
 
   @Override
@@ -88,25 +86,4 @@ public void describeCommandWithoutIndexShouldFailToParse() throws IOException {
       assertTrue(e.getMessage().contains("Failed to parse query due to offending symbol"));
     }
   }
-
-  @Test
-  public void testDescribeCommandWithPrometheusCatalog() throws IOException {
-    JSONObject result = executeQuery("describe  my_prometheus.prometheus_http_requests_total");
-    verifyColumn(
-        result,
-        columnName("TABLE_CATALOG"),
-        columnName("TABLE_SCHEMA"),
-        columnName("TABLE_NAME"),
-        columnName("COLUMN_NAME"),
-        columnName("DATA_TYPE")
-    );
-    verifyDataRows(result,
-        rows("my_prometheus", "default", "prometheus_http_requests_total", "handler", "keyword"),
-        rows("my_prometheus", "default", "prometheus_http_requests_total", "code", "keyword"),
-        rows("my_prometheus", "default", "prometheus_http_requests_total", "instance", "keyword"),
-        rows("my_prometheus", "default", "prometheus_http_requests_total", "@value", "double"),
-        rows("my_prometheus", "default", "prometheus_http_requests_total", "@timestamp",
-            "timestamp"),
-        rows("my_prometheus", "default", "prometheus_http_requests_total", "job", "keyword"));
-  }
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/InformationSchemaCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/InformationSchemaCommandIT.java
@@ -71,4 +71,27 @@ public void testTablesFromPrometheusCatalog() throws IOException {
             "counter", "", "Counter of HTTP requests."));
   }
 
+
+  // Moved this IT from DescribeCommandIT to segregate Datasource Integ Tests.
+  @Test
+  public void testDescribeCommandWithPrometheusCatalog() throws IOException {
+    JSONObject result = executeQuery("describe  my_prometheus.prometheus_http_requests_total");
+    verifyColumn(
+        result,
+        columnName("TABLE_CATALOG"),
+        columnName("TABLE_SCHEMA"),
+        columnName("TABLE_NAME"),
+        columnName("COLUMN_NAME"),
+        columnName("DATA_TYPE")
+    );
+    verifyDataRows(result,
+        rows("my_prometheus", "default", "prometheus_http_requests_total", "handler", "keyword"),
+        rows("my_prometheus", "default", "prometheus_http_requests_total", "code", "keyword"),
+        rows("my_prometheus", "default", "prometheus_http_requests_total", "instance", "keyword"),
+        rows("my_prometheus", "default", "prometheus_http_requests_total", "@value", "double"),
+        rows("my_prometheus", "default", "prometheus_http_requests_total", "@timestamp",
+            "timestamp"),
+        rows("my_prometheus", "default", "prometheus_http_requests_total", "job", "keyword"));
+  }
+
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/CsvFormatIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/CsvFormatIT.java
@@ -11,6 +11,7 @@
 import java.io.IOException;
 import java.util.Locale;
 import org.junit.Test;
+import org.opensearch.sql.common.utils.StringUtils;
 import org.opensearch.sql.legacy.SQLIntegTestCase;
 
 public class CsvFormatIT extends SQLIntegTestCase {
@@ -24,13 +25,13 @@ public void init() throws IOException {
   public void sanitizeTest() {
     String result = executeQuery(
         String.format(Locale.ROOT, "SELECT firstname, lastname FROM %s", TEST_INDEX_BANK_CSV_SANITIZE), "csv");
-    assertEquals(
-        "firstname,lastname\n"
-            + "'+Amber JOHnny,Duke Willmington+\n"
-            + "'-Hattie,Bond-\n"
-            + "'=Nanette,Bates=\n"
-            + "'@Dale,Adams@\n"
-            + "\",Elinor\",\"Ratliff,,,\"\n",
+    assertEquals(StringUtils.format(
+        "firstname,lastname%n"
+            + "'+Amber JOHnny,Duke Willmington+%n"
+            + "'-Hattie,Bond-%n"
+            + "'=Nanette,Bates=%n"
+            + "'@Dale,Adams@%n"
+            + "\",Elinor\",\"Ratliff,,,\"%n"),
         result);
   }
 
@@ -39,13 +40,13 @@ public void escapeSanitizeTest() {
     String result = executeQuery(
         String.format(Locale.ROOT, "SELECT firstname, lastname FROM %s", TEST_INDEX_BANK_CSV_SANITIZE),
         "csv&sanitize=false");
-    assertEquals(
-        "firstname,lastname\n"
-            + "+Amber JOHnny,Duke Willmington+\n"
-            + "-Hattie,Bond-\n"
-            + "=Nanette,Bates=\n"
-            + "@Dale,Adams@\n"
-            + "\",Elinor\",\"Ratliff,,,\"\n",
+    assertEquals(StringUtils.format(
+        "firstname,lastname%n"
+            + "+Amber JOHnny,Duke Willmington+%n"
+            + "-Hattie,Bond-%n"
+            + "=Nanette,Bates=%n"
+            + "@Dale,Adams@%n"
+            + "\",Elinor\",\"Ratliff,,,\"%n"),
         result);
   }
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/RawFormatIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/RawFormatIT.java
@@ -11,6 +11,7 @@
 import java.io.IOException;
 import java.util.Locale;
 import org.junit.Test;
+import org.opensearch.sql.common.utils.StringUtils;
 import org.opensearch.sql.legacy.SQLIntegTestCase;
 
 public class RawFormatIT extends SQLIntegTestCase {
@@ -24,13 +25,13 @@ public void init() throws IOException {
   public void rawFormatWithPipeFieldTest() {
     String result = executeQuery(
         String.format(Locale.ROOT, "SELECT firstname, lastname FROM %s", TEST_INDEX_BANK_RAW_SANITIZE), "raw");
-    assertEquals(
-        "firstname|lastname\n"
-            + "+Amber JOHnny|Duke Willmington+\n"
-            + "-Hattie|Bond-\n"
-            + "=Nanette|Bates=\n"
-            + "@Dale|Adams@\n"
-            + "@Elinor|\"Ratliff|||\"\n",
+    assertEquals(StringUtils.format(
+        "firstname|lastname%n"
+            + "+Amber JOHnny|Duke Willmington+%n"
+            + "-Hattie|Bond-%n"
+            + "=Nanette|Bates=%n"
+            + "@Dale|Adams@%n"
+            + "@Elinor|\"Ratliff|||\"%n"),
         result);
   }