Skip to content

Commit 38068c9

Browse files
vamsimanoharvamsimanohar
committed
Integ Test Refactoring (#1383)
* Integ Test Refactoring Signed-off-by: Vamsi Manohar <reddyvam@amazon.com> (cherry picked from commit 8f6793b)
1 parent 259b001 commit 38068c9

13 files changed

Lines changed: 146 additions & 95 deletions

File tree

‎.github/workflows/sql-test-and-build-workflow.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ jobs:
2525
matrix:
2626
entry:
2727
- { os: ubuntu-latest, java: 11 }
28-
- { os: windows-latest, java: 11, os_build_args: -x doctest -x integTest -x jacocoTestReport -PbuildPlatform=windows }
29-
- { os: macos-latest, java: 11, os_build_args: -x doctest -x integTest -x jacocoTestReport }
28+
- { os: windows-latest, java: 11, os_build_args: -x doctest -PbuildPlatform=windows }
29+
- { os: macos-latest, java: 11}
3030
- { os: ubuntu-latest, java: 17 }
31-
- { os: windows-latest, java: 17, os_build_args: -x doctest -x integTest -x jacocoTestReport -PbuildPlatform=windows }
32-
- { os: macos-latest, java: 17, os_build_args: -x doctest -x integTest -x jacocoTestReport }
31+
- { os: windows-latest, java: 17, os_build_args: -x doctest -PbuildPlatform=windows }
32+
- { os: macos-latest, java: 17 }
3333
runs-on: ${{ matrix.entry.os }}
3434

3535
steps:

‎.gitignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
*.class
2+
*.http
23
.settings/
34
# Mobile Tools for Java (J2ME)
45
.mtj.tmp/
@@ -33,7 +34,6 @@ gen/
3334
# git mergetool artifact
3435
*.orig
3536
gen
36-
*.tokens
3737

3838
# Python
3939
*/.venv

‎build-tools/sqlplugin-coverage.gradle‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* cluster is stopped and dump it to a file. Luckily our current security policy seems to allow this. This will also probably
1717
* break if there are multiple nodes in the integTestCluster. But for now... it sorta works.
1818
*/
19+
import org.apache.tools.ant.taskdefs.condition.Os
1920
apply plugin: 'jacoco'
2021

2122
// 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 {
4546
}
4647

4748
testClusters.integTest {
48-
jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}"
49+
if (Os.isFamily(Os.FAMILY_WINDOWS)) {
50+
// Replacing build with absolute path to fix the error "error opening zip file or JAR manifest missing : /build/tmp/expandedArchives/..../jacocoagent.jar"
51+
jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('build',"${buildDir}")
52+
} else {
53+
jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('javaagent:','javaagent:/')
54+
}
4955
systemProperty 'com.sun.management.jmxremote', "true"
5056
systemProperty 'com.sun.management.jmxremote.authenticate', "false"
5157
systemProperty 'com.sun.management.jmxremote.port', "7777"

‎buildSrc/src/main/groovy/com/wiredforcode/spawn/KillProcessTask.groovy‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.wiredforcode.gradle.spawn
22

3+
import org.apache.tools.ant.taskdefs.condition.Os
34
import org.gradle.api.tasks.TaskAction
45

56
class KillProcessTask extends DefaultSpawnTask {
@@ -12,7 +13,13 @@ class KillProcessTask extends DefaultSpawnTask {
1213
}
1314

1415
def pid = pidFile.text
15-
def process = "kill $pid".execute()
16+
def killCommandLine
17+
if (Os.isFamily(Os.FAMILY_WINDOWS)) {
18+
killCommandLine = Arrays.asList("taskkill", "/F", "/T", "/PID", "$pid")
19+
} else {
20+
killCommandLine = Arrays.asList("kill", "$pid")
21+
}
22+
def process = killCommandLine.execute()
1623

1724
try {
1825
process.waitFor()

‎buildSrc/src/main/groovy/com/wiredforcode/spawn/SpawnProcessTask.groovy‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,18 @@ class SpawnProcessTask extends DefaultSpawnTask {
9595
}
9696

9797
private int extractPidFromProcess(Process process) {
98-
def pidField = process.class.getDeclaredField('pid')
99-
pidField.accessible = true
100-
101-
return pidField.getInt(process)
98+
def pid
99+
try {
100+
// works since java 9
101+
def pidMethod = process.class.getDeclaredMethod('pid')
102+
pidMethod.setAccessible(true)
103+
pid = pidMethod.invoke(process)
104+
} catch (ignored) {
105+
// fallback to UNIX-only implementation
106+
def pidField = process.class.getDeclaredField('pid')
107+
pidField.accessible = true
108+
pid = pidField.getInt(process)
109+
}
110+
return pid
102111
}
103112
}

‎doctest/build.gradle‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,12 @@ task startPrometheus(type: SpawnProcessTask) {
5656

5757
//evaluationDependsOn(':')
5858
task startOpenSearch(type: SpawnProcessTask) {
59-
command "${path}/gradlew -p ${plugin_path} runRestTestCluster"
59+
if( getOSFamilyType() == "windows") {
60+
command "${path}\\gradlew.bat -p ${plugin_path} runRestTestCluster"
61+
}
62+
else {
63+
command "${path}/gradlew -p ${plugin_path} runRestTestCluster"
64+
}
6065
ready 'started'
6166
}
6267

@@ -94,12 +99,13 @@ task stopPrometheus() {
9499
}
95100
}
96101
}
97-
98-
stopPrometheus.mustRunAfter startPrometheus
102+
if(getOSFamilyType() != "windows") {
103+
stopPrometheus.mustRunAfter startPrometheus
104+
startOpenSearch.dependsOn startPrometheus
105+
stopOpenSearch.finalizedBy stopPrometheus
106+
}
99107
doctest.dependsOn startOpenSearch
100-
startOpenSearch.dependsOn startPrometheus
101108
doctest.finalizedBy stopOpenSearch
102-
stopOpenSearch.finalizedBy stopPrometheus
103109
check.dependsOn doctest
104110
clean.dependsOn(cleanBootstrap)
105111

‎integ-test/build.gradle‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
* under the License.
2323
*/
2424

25-
import org.gradle.nativeplatform.platform.internal.DefaultNativePlatform
2625
import org.opensearch.gradle.test.RestIntegTestTask
2726
import org.opensearch.gradle.testclusters.StandaloneRestIntegTestTask
2827

@@ -151,9 +150,10 @@ stopPrometheus.mustRunAfter startPrometheus
151150
// Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled
152151
integTest {
153152
dependsOn ':opensearch-sql-plugin:bundlePlugin'
154-
dependsOn startPrometheus
155-
finalizedBy stopPrometheus
156-
153+
if(getOSFamilyType() != "windows") {
154+
dependsOn startPrometheus
155+
finalizedBy stopPrometheus
156+
}
157157
systemProperty 'tests.security.manager', 'false'
158158
systemProperty('project.root', project.projectDir.absolutePath)
159159

@@ -178,6 +178,12 @@ integTest {
178178
}
179179
}
180180

181+
if(getOSFamilyType() == "windows") {
182+
exclude 'org/opensearch/sql/ppl/PrometheusDataSourceCommandsIT.class'
183+
exclude 'org/opensearch/sql/ppl/ShowDataSourcesCommandIT.class'
184+
exclude 'org/opensearch/sql/ppl/InformationSchemaCommandIT.class'
185+
}
186+
181187
exclude 'org/opensearch/sql/doctest/**/*IT.class'
182188
exclude 'org/opensearch/sql/correctness/**'
183189

‎integ-test/src/test/java/org/opensearch/sql/ppl/CsvFormatIT.java‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.io.IOException;
1212
import java.util.Locale;
1313
import org.junit.Test;
14+
import org.opensearch.sql.common.utils.StringUtils;
1415

1516
public class CsvFormatIT extends PPLIntegTestCase {
1617

@@ -23,27 +24,27 @@ public void init() throws IOException {
2324
public void sanitizeTest() throws IOException {
2425
String result = executeCsvQuery(
2526
String.format(Locale.ROOT, "source=%s | fields firstname, lastname", TEST_INDEX_BANK_CSV_SANITIZE));
26-
assertEquals(
27-
"firstname,lastname\n"
28-
+ "'+Amber JOHnny,Duke Willmington+\n"
29-
+ "'-Hattie,Bond-\n"
30-
+ "'=Nanette,Bates=\n"
31-
+ "'@Dale,Adams@\n"
32-
+ "\",Elinor\",\"Ratliff,,,\"\n",
27+
assertEquals(StringUtils.format(
28+
"firstname,lastname%n"
29+
+ "'+Amber JOHnny,Duke Willmington+%n"
30+
+ "'-Hattie,Bond-%n"
31+
+ "'=Nanette,Bates=%n"
32+
+ "'@Dale,Adams@%n"
33+
+ "\",Elinor\",\"Ratliff,,,\"%n"),
3334
result);
3435
}
3536

3637
@Test
3738
public void escapeSanitizeTest() throws IOException {
3839
String result = executeCsvQuery(
3940
String.format(Locale.ROOT, "source=%s | fields firstname, lastname", TEST_INDEX_BANK_CSV_SANITIZE), false);
40-
assertEquals(
41-
"firstname,lastname\n"
42-
+ "+Amber JOHnny,Duke Willmington+\n"
43-
+ "-Hattie,Bond-\n"
44-
+ "=Nanette,Bates=\n"
45-
+ "@Dale,Adams@\n"
46-
+ "\",Elinor\",\"Ratliff,,,\"\n",
41+
assertEquals(StringUtils.format(
42+
"firstname,lastname%n"
43+
+ "+Amber JOHnny,Duke Willmington+%n"
44+
+ "-Hattie,Bond-%n"
45+
+ "=Nanette,Bates=%n"
46+
+ "@Dale,Adams@%n"
47+
+ "\",Elinor\",\"Ratliff,,,\"%n"),
4748
result);
4849
}
4950
}

‎integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java‎

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,17 @@
66

77
package org.opensearch.sql.ppl;
88

9-
import org.json.JSONObject;
10-
import org.junit.jupiter.api.Test;
11-
import org.opensearch.client.Request;
12-
import org.opensearch.client.ResponseException;
13-
14-
import java.io.IOException;
15-
169
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG;
1710
import static org.opensearch.sql.util.MatcherUtils.columnName;
18-
import static org.opensearch.sql.util.MatcherUtils.rows;
1911
import static org.opensearch.sql.util.MatcherUtils.verifyColumn;
2012
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
2113

14+
import java.io.IOException;
15+
import org.json.JSONObject;
16+
import org.junit.jupiter.api.Test;
17+
import org.opensearch.client.Request;
18+
import org.opensearch.client.ResponseException;
19+
2220
public class DescribeCommandIT extends PPLIntegTestCase {
2321

2422
@Override
@@ -88,25 +86,4 @@ public void describeCommandWithoutIndexShouldFailToParse() throws IOException {
8886
assertTrue(e.getMessage().contains("Failed to parse query due to offending symbol"));
8987
}
9088
}
91-
92-
@Test
93-
public void testDescribeCommandWithPrometheusCatalog() throws IOException {
94-
JSONObject result = executeQuery("describe my_prometheus.prometheus_http_requests_total");
95-
verifyColumn(
96-
result,
97-
columnName("TABLE_CATALOG"),
98-
columnName("TABLE_SCHEMA"),
99-
columnName("TABLE_NAME"),
100-
columnName("COLUMN_NAME"),
101-
columnName("DATA_TYPE")
102-
);
103-
verifyDataRows(result,
104-
rows("my_prometheus", "default", "prometheus_http_requests_total", "handler", "keyword"),
105-
rows("my_prometheus", "default", "prometheus_http_requests_total", "code", "keyword"),
106-
rows("my_prometheus", "default", "prometheus_http_requests_total", "instance", "keyword"),
107-
rows("my_prometheus", "default", "prometheus_http_requests_total", "@value", "double"),
108-
rows("my_prometheus", "default", "prometheus_http_requests_total", "@timestamp",
109-
"timestamp"),
110-
rows("my_prometheus", "default", "prometheus_http_requests_total", "job", "keyword"));
111-
}
11289
}

‎integ-test/src/test/java/org/opensearch/sql/ppl/InformationSchemaCommandIT.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,27 @@ public void testTablesFromPrometheusCatalog() throws IOException {
7171
"counter", "", "Counter of HTTP requests."));
7272
}
7373

74+
75+
// Moved this IT from DescribeCommandIT to segregate Datasource Integ Tests.
76+
@Test
77+
public void testDescribeCommandWithPrometheusCatalog() throws IOException {
78+
JSONObject result = executeQuery("describe my_prometheus.prometheus_http_requests_total");
79+
verifyColumn(
80+
result,
81+
columnName("TABLE_CATALOG"),
82+
columnName("TABLE_SCHEMA"),
83+
columnName("TABLE_NAME"),
84+
columnName("COLUMN_NAME"),
85+
columnName("DATA_TYPE")
86+
);
87+
verifyDataRows(result,
88+
rows("my_prometheus", "default", "prometheus_http_requests_total", "handler", "keyword"),
89+
rows("my_prometheus", "default", "prometheus_http_requests_total", "code", "keyword"),
90+
rows("my_prometheus", "default", "prometheus_http_requests_total", "instance", "keyword"),
91+
rows("my_prometheus", "default", "prometheus_http_requests_total", "@value", "double"),
92+
rows("my_prometheus", "default", "prometheus_http_requests_total", "@timestamp",
93+
"timestamp"),
94+
rows("my_prometheus", "default", "prometheus_http_requests_total", "job", "keyword"));
95+
}
96+
7497
}

0 commit comments

Comments
 (0)