From ff2f3f762ce5504a41e268e088245a3b109cc671 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 21 Feb 2024 16:02:30 +0100 Subject: [PATCH 1/8] Load csv data as class rule in single/multi-node This avoids running a setup method before each and every test case, which also obscures any errors happening during csv loading. --- x-pack/plugin/esql/qa/server/build.gradle | 1 + .../esql/qa/mixed/MixedClusterEsqlSpecIT.java | 12 +++ .../xpack/esql/ccq/MultiClusterSpecIT.java | 10 ++ .../xpack/esql/qa/multi_node/EsqlSpecIT.java | 10 +- .../xpack/esql/qa/single_node/EsqlSpecIT.java | 6 +- .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 96 +++++++++++++++++-- 6 files changed, 123 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/build.gradle b/x-pack/plugin/esql/qa/server/build.gradle index fe5e08cda32f7..746401b8f4dad 100644 --- a/x-pack/plugin/esql/qa/server/build.gradle +++ b/x-pack/plugin/esql/qa/server/build.gradle @@ -10,4 +10,5 @@ dependencies { // Requirement for some ESQL-specific utilities implementation project(':x-pack:plugin:esql') api project(xpackModule('esql:qa:testFixtures')) + api project(':test:test-clusters') } diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java index 9d22045522d19..28edea52dad29 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java @@ -11,9 +11,14 @@ import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; +import org.junit.Before; import org.junit.ClassRule; +import java.io.IOException; + import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; +import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.CSV_DATASET_MAP; +import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.loadDataSetIntoEs; import static org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase.Mode.ASYNC; public class MixedClusterEsqlSpecIT extends EsqlSpecTestCase { @@ -25,6 +30,13 @@ protected String getTestRestCluster() { return cluster.getHttpAddresses(); } + @Before + public void setup() throws IOException { + if (indexExists(CSV_DATASET_MAP.keySet().iterator().next()) == false) { + loadDataSetIntoEs(client()); + } + } + static final Version bwcVersion = Version.fromString(System.getProperty("tests.old_cluster_version")); public MixedClusterEsqlSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase, Mode mode) { diff --git a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java index e5e53f34df312..3ca5bdcdeda02 100644 --- a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java @@ -23,6 +23,7 @@ import org.elasticsearch.xpack.ql.CsvSpecReader; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; import org.elasticsearch.xpack.ql.SpecReader; +import org.junit.Before; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; @@ -39,7 +40,9 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; +import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.CSV_DATASET_MAP; import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.ENRICH_SOURCE_INDICES; +import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.loadDataSetIntoEs; import static org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase.Mode.SYNC; import static org.elasticsearch.xpack.ql.CsvSpecReader.specParser; import static org.elasticsearch.xpack.ql.TestUtils.classpathResources; @@ -62,6 +65,13 @@ public class MultiClusterSpecIT extends EsqlSpecTestCase { @ClassRule public static TestRule clusterRule = RuleChain.outerRule(remoteCluster).around(localCluster); + @Before + public void setup() throws IOException { + if (indexExists(CSV_DATASET_MAP.keySet().iterator().next()) == false) { + loadDataSetIntoEs(client()); + } + } + @ParametersFactory(argumentFormatting = "%2$s.%3$s") public static List readScriptSpec() throws Exception { List urls = classpathResources("/*.csv-spec"); diff --git a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java index 67b916a815819..6a4e4b544e3e5 100644 --- a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java @@ -14,7 +14,15 @@ public class EsqlSpecIT extends EsqlSpecTestCase { @ClassRule - public static ElasticsearchCluster cluster = Clusters.testCluster(); + public static ClusterWithCsvData cluster = new ClusterWithCsvData( + ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .nodes(2) + .setting("xpack.security.enabled", "false") + .setting("xpack.license.self_generated.type", "trial") + .build(), + Settings.builder().build() + ); @Override protected String getTestRestCluster() { diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java index db737e3678752..c6ad3ac79b089 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java @@ -9,8 +9,8 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.TestClustersThreadFilter; -import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; import org.junit.ClassRule; @@ -18,11 +18,11 @@ @ThreadLeakFilters(filters = TestClustersThreadFilter.class) public class EsqlSpecIT extends EsqlSpecTestCase { @ClassRule - public static ElasticsearchCluster cluster = Clusters.testCluster(); + public static ClusterWithCsvData cluster = new ClusterWithCsvData(Clusters.testCluster(), Settings.builder().build()); @Override protected String getTestRestCluster() { - return cluster.getHttpAddresses(); + return cluster.cluster().getHttpAddresses(); } public EsqlSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase, Mode mode) { diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index 18b9206f9b89e..306723ea8000a 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -9,16 +9,23 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.apache.http.HttpEntity; +import org.apache.http.HttpHost; import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.RestClientBuilder; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.IOUtils; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; import org.elasticsearch.geometry.utils.GeometryValidator; import org.elasticsearch.geometry.utils.WellKnownText; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; +import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.esql.CsvTestUtils; @@ -28,6 +35,9 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; import java.io.IOException; import java.net.URL; @@ -47,7 +57,6 @@ import static org.elasticsearch.xpack.esql.CsvTestUtils.ExpectedResults; import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; import static org.elasticsearch.xpack.esql.CsvTestUtils.loadCsvSpecValues; -import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.CSV_DATASET_MAP; import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.loadDataSetIntoEs; import static org.elasticsearch.xpack.ql.CsvSpecReader.specParser; import static org.elasticsearch.xpack.ql.TestUtils.classpathResources; @@ -95,18 +104,12 @@ protected EsqlSpecTestCase(String fileName, String groupName, String testName, I this.mode = mode; } - @Before - public void setup() throws IOException { - if (indexExists(CSV_DATASET_MAP.keySet().iterator().next()) == false) { - loadDataSetIntoEs(client()); - } - } - protected boolean supportsAsync() { return Version.CURRENT.onOrAfter(Version.V_8_13_0); // the Async API was introduced in 8.13.0 } @AfterClass + // TODO: This should become of a classrule for consistency. public static void wipeTestData() throws IOException { try { adminClient().performRequest(new Request("DELETE", "/*")); @@ -239,4 +242,81 @@ public static void assertRequestBreakerEmpty() throws Exception { } }); } + + public static class ClusterWithCsvData implements TestRule { + private static final long CLIENT_TIMEOUT = 40L; + + ElasticsearchCluster cluster; + Settings clientAuthSettings; + RestClient client; + + public ClusterWithCsvData(ElasticsearchCluster cluster, Settings clientAuthSettings) { + this.cluster = cluster; + this.clientAuthSettings = clientAuthSettings; + } + + public ElasticsearchCluster cluster() { + return cluster; + } + + @Override + public Statement apply(Statement base, Description description) { + return cluster.apply(startClient(loadCsvData(base)), description); + } + + private Statement startClient(Statement base) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + try { + client = doStartClient(); + base.evaluate(); + } finally { + IOUtils.close(client); + } + } + }; + } + + private Statement loadCsvData(Statement base) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + loadDataSetIntoEs(client); + base.evaluate(); + } + }; + } + + private RestClient doStartClient() throws IOException { + String address = cluster.getHttpAddress(0); + int portSeparator = address.lastIndexOf(':'); + if (portSeparator < 0) { + throw new IllegalArgumentException("Illegal cluster url [" + address + "]"); + } + String host = address.substring(0, portSeparator); + int port = Integer.parseInt(address.substring(portSeparator + 1)); + HttpHost[] remoteHttpHosts = new HttpHost[] { new HttpHost(host, port) }; + + return clientBuilder(clientAuthSettings, remoteHttpHosts); + } + + protected static TimeValue timeout() { + return TimeValue.timeValueSeconds(CLIENT_TIMEOUT); + } + + private static RestClient clientBuilder(Settings settings, HttpHost[] hosts) throws IOException { + RestClientBuilder builder = RestClient.builder(hosts); + doConfigureClient(builder, settings); + + int timeout = Math.toIntExact(timeout().millis()); + builder.setRequestConfigCallback( + requestConfigBuilder -> requestConfigBuilder.setConnectTimeout(timeout) + .setConnectionRequestTimeout(timeout) + .setSocketTimeout(timeout) + ); + builder.setStrictDeprecationMode(true); + return builder.build(); + } + } } From 21b5d29015c774c589449cd39a8420cc2746b003 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 17:45:47 +0100 Subject: [PATCH 2/8] Simplify using a RuleChain --- .../xpack/esql/qa/multi_node/EsqlSpecIT.java | 15 +++++------- .../xpack/esql/qa/single_node/EsqlSpecIT.java | 11 ++++++--- .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 23 ++++--------------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java index 6a4e4b544e3e5..b1c1018a4f5f3 100644 --- a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java @@ -11,18 +11,15 @@ import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; public class EsqlSpecIT extends EsqlSpecTestCase { + public static ElasticsearchCluster cluster = Clusters.testCluster(); + public static CsvLoader loader = new CsvLoader(cluster); + @ClassRule - public static ClusterWithCsvData cluster = new ClusterWithCsvData( - ElasticsearchCluster.local() - .distribution(DistributionType.DEFAULT) - .nodes(2) - .setting("xpack.security.enabled", "false") - .setting("xpack.license.self_generated.type", "trial") - .build(), - Settings.builder().build() - ); + public static TestRule clusterRule = RuleChain.outerRule(cluster).around(loader); @Override protected String getTestRestCluster() { diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java index c6ad3ac79b089..de79a4115b1f1 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java @@ -9,20 +9,25 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.TestClustersThreadFilter; +import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; @ThreadLeakFilters(filters = TestClustersThreadFilter.class) public class EsqlSpecIT extends EsqlSpecTestCase { + public static ElasticsearchCluster cluster = Clusters.testCluster(); + public static CsvLoader loader = new CsvLoader(cluster); + @ClassRule - public static ClusterWithCsvData cluster = new ClusterWithCsvData(Clusters.testCluster(), Settings.builder().build()); + public static TestRule clusterRule = RuleChain.outerRule(cluster).around(loader); @Override protected String getTestRestCluster() { - return cluster.cluster().getHttpAddresses(); + return cluster.getHttpAddresses(); } public EsqlSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase, Mode mode) { diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index 306723ea8000a..01e51ee8ca703 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -243,33 +243,30 @@ public static void assertRequestBreakerEmpty() throws Exception { }); } - public static class ClusterWithCsvData implements TestRule { + public static class CsvLoader implements TestRule { private static final long CLIENT_TIMEOUT = 40L; ElasticsearchCluster cluster; Settings clientAuthSettings; RestClient client; - public ClusterWithCsvData(ElasticsearchCluster cluster, Settings clientAuthSettings) { + public CsvLoader(ElasticsearchCluster cluster, Settings clientAuthSettings) { this.cluster = cluster; this.clientAuthSettings = clientAuthSettings; } - public ElasticsearchCluster cluster() { - return cluster; + public CsvLoader(ElasticsearchCluster cluster) { + this(cluster, Settings.builder().build()); } @Override public Statement apply(Statement base, Description description) { - return cluster.apply(startClient(loadCsvData(base)), description); - } - - private Statement startClient(Statement base) { return new Statement() { @Override public void evaluate() throws Throwable { try { client = doStartClient(); + loadDataSetIntoEs(client); base.evaluate(); } finally { IOUtils.close(client); @@ -278,16 +275,6 @@ public void evaluate() throws Throwable { }; } - private Statement loadCsvData(Statement base) { - return new Statement() { - @Override - public void evaluate() throws Throwable { - loadDataSetIntoEs(client); - base.evaluate(); - } - }; - } - private RestClient doStartClient() throws IOException { String address = cluster.getHttpAddress(0); int portSeparator = address.lastIndexOf(':'); From 70595aaa354234774724c560c710e2678a0999f6 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 17:57:17 +0100 Subject: [PATCH 3/8] Use ClassRule for mixed cluster test --- .../esql/qa/mixed/MixedClusterEsqlSpecIT.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java index 28edea52dad29..bd93c9885e43e 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java @@ -11,32 +11,25 @@ import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; -import org.junit.Before; import org.junit.ClassRule; - -import java.io.IOException; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; -import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.CSV_DATASET_MAP; -import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.loadDataSetIntoEs; import static org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase.Mode.ASYNC; public class MixedClusterEsqlSpecIT extends EsqlSpecTestCase { - @ClassRule public static ElasticsearchCluster cluster = Clusters.mixedVersionCluster(); + public static CsvLoader loader = new CsvLoader(cluster); + + @ClassRule + public static TestRule clusterRule = RuleChain.outerRule(cluster).around(loader); @Override protected String getTestRestCluster() { return cluster.getHttpAddresses(); } - @Before - public void setup() throws IOException { - if (indexExists(CSV_DATASET_MAP.keySet().iterator().next()) == false) { - loadDataSetIntoEs(client()); - } - } - static final Version bwcVersion = Version.fromString(System.getProperty("tests.old_cluster_version")); public MixedClusterEsqlSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase, Mode mode) { From 83ed0ccf5dfcc83d203c32fc1b5134e4d9bc2ff5 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 17:59:18 +0100 Subject: [PATCH 4/8] Add comment --- .../org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index 01e51ee8ca703..4f57462485348 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -275,6 +275,7 @@ public void evaluate() throws Throwable { }; } + // TODO: See if we can use existing code for building the client private RestClient doStartClient() throws IOException { String address = cluster.getHttpAddress(0); int portSeparator = address.lastIndexOf(':'); From bbb51a591bcf848d10ed19817da5c0bbdbc023f7 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 18:05:53 +0100 Subject: [PATCH 5/8] Remove unused readScriptSpec --- .../xpack/esql/ccq/MultiClusterSpecIT.java | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java index 3ca5bdcdeda02..2654f4804786b 100644 --- a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.ccq; -import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import org.apache.http.HttpEntity; @@ -22,7 +21,6 @@ import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; -import org.elasticsearch.xpack.ql.SpecReader; import org.junit.Before; import org.junit.ClassRule; import org.junit.rules.RuleChain; @@ -30,10 +28,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; -import java.net.URL; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.Locale; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -43,9 +38,6 @@ import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.CSV_DATASET_MAP; import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.ENRICH_SOURCE_INDICES; import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.loadDataSetIntoEs; -import static org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase.Mode.SYNC; -import static org.elasticsearch.xpack.ql.CsvSpecReader.specParser; -import static org.elasticsearch.xpack.ql.TestUtils.classpathResources; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -72,25 +64,6 @@ public void setup() throws IOException { } } - @ParametersFactory(argumentFormatting = "%2$s.%3$s") - public static List readScriptSpec() throws Exception { - List urls = classpathResources("/*.csv-spec"); - assertTrue("Not enough specs found " + urls, urls.size() > 0); - List specs = SpecReader.readScriptSpec(urls, specParser()); - - int len = specs.get(0).length; - List testcases = new ArrayList<>(); - for (var spec : specs) { - for (Mode mode : List.of(SYNC)) { // No async, for now - Object[] obj = new Object[len + 1]; - System.arraycopy(spec, 0, obj, 0, len); - obj[len] = mode; - testcases.add(obj); - } - } - return testcases; - } - public MultiClusterSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase, Mode mode) { super(fileName, groupName, testName, lineNumber, convertToRemoteIndices(testCase), mode); } From eccf7faee6568f1553847244dc8035672abc8ceb Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 18:27:25 +0100 Subject: [PATCH 6/8] Separate client and csv loading into 2 class rules --- .../esql/qa/mixed/MixedClusterEsqlSpecIT.java | 5 ++- .../xpack/esql/qa/multi_node/EsqlSpecIT.java | 5 ++- .../xpack/esql/qa/single_node/EsqlSpecIT.java | 5 ++- .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 37 +++++++++++++++---- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java index bd93c9885e43e..558f63b8ccd5e 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java @@ -20,10 +20,11 @@ public class MixedClusterEsqlSpecIT extends EsqlSpecTestCase { public static ElasticsearchCluster cluster = Clusters.mixedVersionCluster(); - public static CsvLoader loader = new CsvLoader(cluster); + public static TestRuleRestClient client = new TestRuleRestClient(cluster); + public static CsvLoader loader = new CsvLoader(client); @ClassRule - public static TestRule clusterRule = RuleChain.outerRule(cluster).around(loader); + public static TestRule clusterRule = RuleChain.outerRule(cluster).around(client).around(loader); @Override protected String getTestRestCluster() { diff --git a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java index b1c1018a4f5f3..87b7976afc521 100644 --- a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java @@ -16,10 +16,11 @@ public class EsqlSpecIT extends EsqlSpecTestCase { public static ElasticsearchCluster cluster = Clusters.testCluster(); - public static CsvLoader loader = new CsvLoader(cluster); + public static TestRuleRestClient client = new TestRuleRestClient(cluster); + public static CsvLoader loader = new CsvLoader(client); @ClassRule - public static TestRule clusterRule = RuleChain.outerRule(cluster).around(loader); + public static TestRule clusterRule = RuleChain.outerRule(cluster).around(client).around(loader); @Override protected String getTestRestCluster() { diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java index de79a4115b1f1..7c7851078cee4 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java @@ -20,10 +20,11 @@ @ThreadLeakFilters(filters = TestClustersThreadFilter.class) public class EsqlSpecIT extends EsqlSpecTestCase { public static ElasticsearchCluster cluster = Clusters.testCluster(); - public static CsvLoader loader = new CsvLoader(cluster); + public static TestRuleRestClient client = new TestRuleRestClient(cluster); + public static CsvLoader loader = new CsvLoader(client); @ClassRule - public static TestRule clusterRule = RuleChain.outerRule(cluster).around(loader); + public static TestRule clusterRule = RuleChain.outerRule(cluster).around(client).around(loader); @Override protected String getTestRestCluster() { diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index 4f57462485348..525e8b2cf4728 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -109,7 +109,7 @@ protected boolean supportsAsync() { } @AfterClass - // TODO: This should become of a classrule for consistency. + // TODO: This should become part of a classrule for consistency. public static void wipeTestData() throws IOException { try { adminClient().performRequest(new Request("DELETE", "/*")); @@ -244,21 +244,43 @@ public static void assertRequestBreakerEmpty() throws Exception { } public static class CsvLoader implements TestRule { - private static final long CLIENT_TIMEOUT = 40L; + TestRuleRestClient client; + + public CsvLoader(TestRuleRestClient client) { + this.client = client; + } + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + loadDataSetIntoEs(client.client()); + base.evaluate(); + } + }; + } + } - ElasticsearchCluster cluster; - Settings clientAuthSettings; - RestClient client; + public static class TestRuleRestClient implements TestRule { + private static final long CLIENT_TIMEOUT = 40L; + private ElasticsearchCluster cluster; + private Settings clientAuthSettings; + private RestClient client; - public CsvLoader(ElasticsearchCluster cluster, Settings clientAuthSettings) { + public TestRuleRestClient(ElasticsearchCluster cluster, Settings clientAuthSettings) { this.cluster = cluster; this.clientAuthSettings = clientAuthSettings; } - public CsvLoader(ElasticsearchCluster cluster) { + public TestRuleRestClient(ElasticsearchCluster cluster) { this(cluster, Settings.builder().build()); } + public RestClient client() { + return client; + } + @Override public Statement apply(Statement base, Description description) { return new Statement() { @@ -266,7 +288,6 @@ public Statement apply(Statement base, Description description) { public void evaluate() throws Throwable { try { client = doStartClient(); - loadDataSetIntoEs(client); base.evaluate(); } finally { IOUtils.close(client); From 52f6789140ac035b83e597bb29e0fc6b00c50850 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 18:40:45 +0100 Subject: [PATCH 7/8] Align Rest client building with ESRestTestCase --- .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index 525e8b2cf4728..a72983e9e314d 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.IOUtils; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; import org.elasticsearch.geometry.utils.GeometryValidator; @@ -263,14 +262,13 @@ public void evaluate() throws Throwable { } public static class TestRuleRestClient implements TestRule { - private static final long CLIENT_TIMEOUT = 40L; private ElasticsearchCluster cluster; - private Settings clientAuthSettings; + private Settings clientSettings; private RestClient client; - public TestRuleRestClient(ElasticsearchCluster cluster, Settings clientAuthSettings) { + public TestRuleRestClient(ElasticsearchCluster cluster, Settings clientSettings) { this.cluster = cluster; - this.clientAuthSettings = clientAuthSettings; + this.clientSettings = clientSettings; } public TestRuleRestClient(ElasticsearchCluster cluster) { @@ -296,7 +294,6 @@ public void evaluate() throws Throwable { }; } - // TODO: See if we can use existing code for building the client private RestClient doStartClient() throws IOException { String address = cluster.getHttpAddress(0); int portSeparator = address.lastIndexOf(':'); @@ -305,25 +302,14 @@ private RestClient doStartClient() throws IOException { } String host = address.substring(0, portSeparator); int port = Integer.parseInt(address.substring(portSeparator + 1)); - HttpHost[] remoteHttpHosts = new HttpHost[] { new HttpHost(host, port) }; + HttpHost[] httpHosts = new HttpHost[] { new HttpHost(host, port) }; - return clientBuilder(clientAuthSettings, remoteHttpHosts); + return buildClient(clientSettings, httpHosts); } - protected static TimeValue timeout() { - return TimeValue.timeValueSeconds(CLIENT_TIMEOUT); - } - - private static RestClient clientBuilder(Settings settings, HttpHost[] hosts) throws IOException { + private static RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOException { RestClientBuilder builder = RestClient.builder(hosts); doConfigureClient(builder, settings); - - int timeout = Math.toIntExact(timeout().millis()); - builder.setRequestConfigCallback( - requestConfigBuilder -> requestConfigBuilder.setConnectTimeout(timeout) - .setConnectionRequestTimeout(timeout) - .setSocketTimeout(timeout) - ); builder.setStrictDeprecationMode(true); return builder.build(); } From 1432c79032c29100247528f3b2672b4ace2e6d14 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 1 Mar 2024 19:33:39 +0100 Subject: [PATCH 8/8] Csv loading in multi-cluster test via ClassRule --- .../esql/qa/mixed/MixedClusterEsqlSpecIT.java | 11 ++- .../xpack/esql/ccq/MultiClusterSpecIT.java | 30 +++--- .../xpack/esql/qa/multi_node/EsqlSpecIT.java | 11 ++- .../xpack/esql/qa/single_node/EsqlSpecIT.java | 11 ++- .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 92 +++++++++++-------- 5 files changed, 99 insertions(+), 56 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java index 558f63b8ccd5e..fab8c6547d283 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java @@ -8,6 +8,8 @@ package org.elasticsearch.xpack.esql.qa.mixed; import org.elasticsearch.Version; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; @@ -15,12 +17,19 @@ import org.junit.rules.RuleChain; import org.junit.rules.TestRule; +import java.io.IOException; + import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; import static org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase.Mode.ASYNC; public class MixedClusterEsqlSpecIT extends EsqlSpecTestCase { public static ElasticsearchCluster cluster = Clusters.mixedVersionCluster(); - public static TestRuleRestClient client = new TestRuleRestClient(cluster); + public static ClosingTestRule client = new ClosingTestRule<>() { + @Override + protected RestClient provideObject() throws IOException { + return startClient(cluster, Settings.builder().build()); + } + }; public static CsvLoader loader = new CsvLoader(client); @ClassRule diff --git a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java index 2654f4804786b..360eb5e77345c 100644 --- a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java @@ -21,7 +21,6 @@ import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; -import org.junit.Before; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; @@ -35,9 +34,7 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled; -import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.CSV_DATASET_MAP; import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.ENRICH_SOURCE_INDICES; -import static org.elasticsearch.xpack.esql.CsvTestsDataLoader.loadDataSetIntoEs; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -53,16 +50,17 @@ public class MultiClusterSpecIT extends EsqlSpecTestCase { static ElasticsearchCluster remoteCluster = Clusters.remoteCluster(); static ElasticsearchCluster localCluster = Clusters.localCluster(remoteCluster); + public static ClosingTestRule client = new ClosingTestRule<>() { + @Override + protected RestClient provideObject() throws IOException { + HttpHost[] localHosts = parseClusterHostsStatic(localCluster.getHttpAddresses()).toArray(HttpHost[]::new); + return doBuildClient(Settings.builder().build(), localHosts); + } + }; + public static CsvLoader loader = new CsvLoader(client); @ClassRule - public static TestRule clusterRule = RuleChain.outerRule(remoteCluster).around(localCluster); - - @Before - public void setup() throws IOException { - if (indexExists(CSV_DATASET_MAP.keySet().iterator().next()) == false) { - loadDataSetIntoEs(client()); - } - } + public static TestRule clusterRule = RuleChain.outerRule(remoteCluster).around(localCluster).around(client).around(loader); public MultiClusterSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase, Mode mode) { super(fileName, groupName, testName, lineNumber, convertToRemoteIndices(testCase), mode); @@ -82,9 +80,13 @@ protected String getTestRestCluster() { @Override protected RestClient buildClient(Settings settings, HttpHost[] localHosts) throws IOException { - RestClient localClient = super.buildClient(settings, localHosts); - HttpHost[] remoteHosts = parseClusterHosts(remoteCluster.getHttpAddresses()).toArray(HttpHost[]::new); - RestClient remoteClient = super.buildClient(settings, remoteHosts); + return doBuildClient(settings, localHosts); + } + + private static RestClient doBuildClient(Settings settings, HttpHost[] localHosts) throws IOException { + RestClient localClient = buildClientStatic(settings, localHosts); + HttpHost[] remoteHosts = parseClusterHostsStatic(remoteCluster.getHttpAddresses()).toArray(HttpHost[]::new); + RestClient remoteClient = buildClientStatic(settings, remoteHosts); return twoClients(localClient, remoteClient); } diff --git a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java index 87b7976afc521..10d47d54d6d8d 100644 --- a/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.esql.qa.multi_node; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase; @@ -14,9 +16,16 @@ import org.junit.rules.RuleChain; import org.junit.rules.TestRule; +import java.io.IOException; + public class EsqlSpecIT extends EsqlSpecTestCase { public static ElasticsearchCluster cluster = Clusters.testCluster(); - public static TestRuleRestClient client = new TestRuleRestClient(cluster); + public static ClosingTestRule client = new ClosingTestRule<>() { + @Override + protected RestClient provideObject() throws IOException { + return startClient(cluster, Settings.builder().build()); + } + }; public static CsvLoader loader = new CsvLoader(client); @ClassRule diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java index 7c7851078cee4..137d4f17ff30f 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java @@ -9,6 +9,8 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.TestClustersThreadFilter; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase; @@ -17,10 +19,17 @@ import org.junit.rules.RuleChain; import org.junit.rules.TestRule; +import java.io.IOException; + @ThreadLeakFilters(filters = TestClustersThreadFilter.class) public class EsqlSpecIT extends EsqlSpecTestCase { public static ElasticsearchCluster cluster = Clusters.testCluster(); - public static TestRuleRestClient client = new TestRuleRestClient(cluster); + public static ClosingTestRule client = new ClosingTestRule<>() { + @Override + protected RestClient provideObject() throws IOException { + return startClient(cluster, Settings.builder().build()); + } + }; public static CsvLoader loader = new CsvLoader(client); @ClassRule diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index a72983e9e314d..67a30a8dce221 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -38,6 +38,7 @@ import org.junit.runner.Description; import org.junit.runners.model.Statement; +import java.io.Closeable; import java.io.IOException; import java.net.URL; import java.util.ArrayList; @@ -45,6 +46,7 @@ import java.util.Locale; import java.util.Map; +import static java.util.Collections.unmodifiableList; import static org.apache.lucene.geo.GeoEncodingUtils.decodeLatitude; import static org.apache.lucene.geo.GeoEncodingUtils.decodeLongitude; import static org.apache.lucene.geo.GeoEncodingUtils.encodeLatitude; @@ -242,10 +244,51 @@ public static void assertRequestBreakerEmpty() throws Exception { }); } + public static RestClient startClient(ElasticsearchCluster cluster, Settings clientSettings) throws IOException { + String address = cluster.getHttpAddress(0); + int portSeparator = address.lastIndexOf(':'); + if (portSeparator < 0) { + throw new IllegalArgumentException("Illegal cluster url [" + address + "]"); + } + String host = address.substring(0, portSeparator); + int port = Integer.parseInt(address.substring(portSeparator + 1)); + HttpHost[] httpHosts = new HttpHost[] { new HttpHost(host, port) }; + + return buildClientStatic(clientSettings, httpHosts); + } + + /** + * Like {@link ESRestTestCase#buildClient(Settings, HttpHost[])} but static. + */ + public static RestClient buildClientStatic(Settings settings, HttpHost[] hosts) throws IOException { + RestClientBuilder builder = RestClient.builder(hosts); + doConfigureClient(builder, settings); + builder.setStrictDeprecationMode(true); + return builder.build(); + } + + /** + * Like {@link ESRestTestCase#parseClusterHosts(String)} but static. + */ + protected static List parseClusterHostsStatic(String hostsString) { + String[] stringUrls = hostsString.split(","); + List hosts = new ArrayList<>(stringUrls.length); + for (String stringUrl : stringUrls) { + int portSeparator = stringUrl.lastIndexOf(':'); + if (portSeparator < 0) { + throw new IllegalArgumentException("Illegal cluster url [" + stringUrl + "]"); + } + String host = stringUrl.substring(0, portSeparator); + int port = Integer.valueOf(stringUrl.substring(portSeparator + 1)); + hosts.add(new HttpHost(host, port, "http")); + } + return unmodifiableList(hosts); + } + public static class CsvLoader implements TestRule { - TestRuleRestClient client; + ClosingTestRule client; - public CsvLoader(TestRuleRestClient client) { + public CsvLoader(ClosingTestRule client) { this.client = client; } @@ -254,30 +297,21 @@ public Statement apply(Statement base, Description description) { return new Statement() { @Override public void evaluate() throws Throwable { - loadDataSetIntoEs(client.client()); + loadDataSetIntoEs(client.get()); base.evaluate(); } }; } } - public static class TestRuleRestClient implements TestRule { - private ElasticsearchCluster cluster; - private Settings clientSettings; - private RestClient client; + public abstract static class ClosingTestRule implements TestRule { + private T providedObject; - public TestRuleRestClient(ElasticsearchCluster cluster, Settings clientSettings) { - this.cluster = cluster; - this.clientSettings = clientSettings; + public T get() { + return providedObject; } - public TestRuleRestClient(ElasticsearchCluster cluster) { - this(cluster, Settings.builder().build()); - } - - public RestClient client() { - return client; - } + protected abstract T provideObject() throws IOException; @Override public Statement apply(Statement base, Description description) { @@ -285,33 +319,13 @@ public Statement apply(Statement base, Description description) { @Override public void evaluate() throws Throwable { try { - client = doStartClient(); + providedObject = provideObject(); base.evaluate(); } finally { - IOUtils.close(client); + IOUtils.close(providedObject); } } }; } - - private RestClient doStartClient() throws IOException { - String address = cluster.getHttpAddress(0); - int portSeparator = address.lastIndexOf(':'); - if (portSeparator < 0) { - throw new IllegalArgumentException("Illegal cluster url [" + address + "]"); - } - String host = address.substring(0, portSeparator); - int port = Integer.parseInt(address.substring(portSeparator + 1)); - HttpHost[] httpHosts = new HttpHost[] { new HttpHost(host, port) }; - - return buildClient(clientSettings, httpHosts); - } - - private static RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOException { - RestClientBuilder builder = RestClient.builder(hosts); - doConfigureClient(builder, settings); - builder.setStrictDeprecationMode(true); - return builder.build(); - } } }