From 8f6793ba63cb8c0301447276ae65a3e3f46563bd Mon Sep 17 00:00:00 2001 From: vamsi-amazon Date: Thu, 2 Mar 2023 15:50:09 -0800 Subject: [PATCH] Integ Test Refactoring (#1383) * Integ Test Refactoring Signed-off-by: Vamsi Manohar --- .../workflows/sql-test-and-build-workflow.yml | 8 ++--- .gitignore | 2 +- build-tools/sqlplugin-coverage.gradle | 8 ++++- .../wiredforcode/spawn/KillProcessTask.groovy | 9 ++++- .../spawn/SpawnProcessTask.groovy | 17 ++++++--- doctest/build.gradle | 16 ++++++--- integ-test/build.gradle | 14 +++++--- .../org/opensearch/sql/ppl/CsvFormatIT.java | 29 +++++++-------- .../opensearch/sql/ppl/DescribeCommandIT.java | 35 ++++-------------- .../sql/ppl/InformationSchemaCommandIT.java | 23 ++++++++++++ .../org/opensearch/sql/sql/CsvFormatIT.java | 29 +++++++-------- .../org/opensearch/sql/sql/RawFormatIT.java | 15 ++++---- scripts/integtest.sh | 36 +++++++++++++------ 13 files changed, 146 insertions(+), 95 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index bf219874e9..08880826bc 100644 --- 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 index 67e5bb07e9..2e78185969 100644 --- 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 index 6f99ceac1f..1665637690 100644 --- 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 index 15c60b1aec..55bb76f1a1 100644 --- 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 index 1b3d0c4f00..a79bd13f60 100644 --- 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 index 6760d068a5..40747a0e83 100644 --- 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 index f9d0bf31f8..b792ecb18f 100644 --- 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 @@ -153,9 +152,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) @@ -180,6 +180,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 index 37c08742ab..430ae9a7b2 100644 --- 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,13 +24,13 @@ 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); } @@ -37,13 +38,13 @@ public void sanitizeTest() throws IOException { 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 index 77fd910f35..23bea69a52 100644 --- 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 index 3f9191c9c9..8e7c03777e 100644 --- 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 index 2c5005413e..782e2e22b5 100644 --- 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 index 43af66185e..8cba86647c 100644 --- 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); } diff --git a/scripts/integtest.sh b/scripts/integtest.sh index 5b468b5356..59f4e88c76 100755 --- a/scripts/integtest.sh +++ b/scripts/integtest.sh @@ -95,18 +95,32 @@ fi USERNAME=`echo $CREDENTIAL | awk -F ':' '{print $1}'` PASSWORD=`echo $CREDENTIAL | awk -F ':' '{print $2}'` -OPENSEARCH_HOME=`ps -ef | grep -o "[o]pensearch.path.home=\S\+" | cut -d= -f2- | head -n1` - -curl -SL https://raw.githubusercontent.com/opensearch-project/sql/main/integ-test/src/test/resources/datasource/datasources.json -o "$OPENSEARCH_HOME"/datasources.json - -yes | $OPENSEARCH_HOME/bin/opensearch-keystore add-file plugins.query.federation.datasources.config $OPENSEARCH_HOME/datasources.json - -if [ $SECURITY_ENABLED == "true" ] +OS="`uname`" +#Cygwin or MinGW packages should be preinstalled in the windows. +#This command doesn't work without bash +#https://stackoverflow.com/questions/3466166/how-to-check-if-running-in-cygwin-mac-or-linux +#Operating System uname -s +#Mac OS X Darwin +#Cygwin 32-bit (Win-XP) CYGWIN_NT-5.1 +#Cygwin 32-bit (Win-7 32-bit) CYGWIN_NT-6.1 +#Cygwin 32-bit (Win-7 64-bit) CYGWIN_NT-6.1-WOW64 +#Cygwin 64-bit (Win-7 64-bit) CYGWIN_NT-6.1 +#MinGW (Windows 7 32-bit) MINGW32_NT-6.1 +#MinGW (Windows 10 64-bit) MINGW64_NT-10.0 +#Interix (Services for UNIX) Interix +#MSYS MSYS_NT-6.1 +#MSYS2 MSYS_NT-10.0-17763 +if ! [[ $OS =~ CYGWIN*|MINGW*|MINGW32*|MSYS* ]] then - curl -k --request POST --url https://$BIND_ADDRESS:$BIND_PORT/_nodes/reload_secure_settings --header 'content-type: application/json' --data '{"secure_settings_password":""}' --user $CREDENTIAL -else - curl --request POST --url http://$BIND_ADDRESS:$BIND_PORT/_nodes/reload_secure_settings --header 'content-type: application/json' --data '{"secure_settings_password":""}' + OPENSEARCH_HOME=`ps -ef | grep -o "[o]pensearch.path.home=\S\+" | cut -d= -f2- | head -n1` + curl -SL https://raw.githubusercontent.com/opensearch-project/sql/main/integ-test/src/test/resources/datasource/datasources.json -o "$OPENSEARCH_HOME"/datasources.json + yes | $OPENSEARCH_HOME/bin/opensearch-keystore add-file plugins.query.federation.datasources.config $OPENSEARCH_HOME/datasources.json + if [ $SECURITY_ENABLED == "true" ] + then + curl -k --request POST --url https://$BIND_ADDRESS:$BIND_PORT/_nodes/reload_secure_settings --header 'content-type: application/json' --data '{"secure_settings_password":""}' --user $CREDENTIAL + else + curl --request POST --url http://$BIND_ADDRESS:$BIND_PORT/_nodes/reload_secure_settings --header 'content-type: application/json' --data '{"secure_settings_password":""}' + fi fi - ./gradlew integTest -Dopensearch.version=$OPENSEARCH_VERSION -Dbuild.snapshot=$SNAPSHOT -Dtests.rest.cluster="$BIND_ADDRESS:$BIND_PORT" -Dtests.cluster="$BIND_ADDRESS:$BIND_PORT" -Dtests.clustername="opensearch-integrationtest" -Dhttps=$SECURITY_ENABLED -Duser=$USERNAME -Dpassword=$PASSWORD --console=plain