From c19379ef31df3dce98d9d91d33c657eee7416f95 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Thu, 24 Oct 2019 09:04:57 -0500 Subject: [PATCH] Remove random when using HLRC sync and async calls (#48211) This commit removes the randomization used by every execute call in the high level rest tests. Previously every execute call, which can be many calls per single test, would rely on a random boolean to determine if they should use the sync or async methods provided to the execute method. This commit runs the tests twice, using two different clusters, both of them providing the value one time via a sysprop. This ensures that the whole suite of tests is run using the sync and async code paths. Closes #39667 --- client/rest-high-level/build.gradle | 23 +++++++++++++++---- .../client/ESRestHighLevelClientTestCase.java | 10 ++++---- .../LicensingDocumentationIT.java | 4 ++-- .../SnapshotClientDocumentationIT.java | 8 ++++++- modules/transport-netty4/build.gradle | 2 +- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle index 620268c9ec210..2eef1b4709bf4 100644 --- a/client/rest-high-level/build.gradle +++ b/client/rest-high-level/build.gradle @@ -1,3 +1,5 @@ +import org.elasticsearch.gradle.test.RestIntegTestTask + /* * Licensed to Elasticsearch under one or more contributor * license agreements. See the NOTICE file distributed with @@ -103,11 +105,22 @@ File nodeTrustStore = file("./testnode.jks") File pkiTrustCert = file("./src/test/resources/org/elasticsearch/client/security/delegate_pki/testRootCA.crt") integTest.runner { + systemProperty 'tests.rest.async', 'false' systemProperty 'tests.rest.cluster.username', System.getProperty('tests.rest.cluster.username', 'test_user') systemProperty 'tests.rest.cluster.password', System.getProperty('tests.rest.cluster.password', 'test-password') } -testClusters.integTest { +RestIntegTestTask asyncIntegTest = tasks.create("asyncIntegTest", RestIntegTestTask) { + runner { + systemProperty 'tests.rest.async', 'true' + systemProperty 'tests.rest.cluster.username', System.getProperty('tests.rest.cluster.username', 'test_user') + systemProperty 'tests.rest.cluster.password', System.getProperty('tests.rest.cluster.password', 'test-password') + } +} + +check.dependsOn(asyncIntegTest) + +testClusters.all { testDistribution = 'DEFAULT' systemProperty 'es.scripting.update.ctx_in_params', 'false' setting 'reindex.remote.whitelist', '[ "[::1]:*", "127.0.0.1:*" ]' @@ -127,10 +140,10 @@ testClusters.integTest { setting 'indices.lifecycle.poll_interval', '1000ms' keystore 'xpack.security.transport.ssl.truststore.secure_password', 'testnode' extraConfigFile 'roles.yml', file('roles.yml') - user username: System.getProperty('tests.rest.cluster.username', 'test_user'), - password: System.getProperty('tests.rest.cluster.password', 'test-password'), - role: System.getProperty('tests.rest.cluster.role', 'admin') - user username: 'admin_user', password: 'admin-password' + user username: System.getProperty('tests.rest.cluster.username', 'test_user'), + password: System.getProperty('tests.rest.cluster.password', 'test-password'), + role: System.getProperty('tests.rest.cluster.role', 'admin') + user username: 'admin_user', password: 'admin-password' extraConfigFile nodeCert.name, nodeCert extraConfigFile nodeTrustStore.name, nodeTrustStore diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java index f758156c222a8..b54e4f2434ec7 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.client.indices.CreateIndexRequest; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -52,6 +53,7 @@ public abstract class ESRestHighLevelClientTestCase extends ESRestTestCase { private static RestHighLevelClient restHighLevelClient; + private static boolean async = Booleans.parseBoolean(System.getProperty("tests.rest.async", "false")); @Before public void initHighLevelClient() throws IOException { @@ -78,20 +80,20 @@ protected static Resp execute(Req request, SyncMethod syn AsyncMethod asyncMethod) throws IOException { return execute(request, syncMethod, asyncMethod, RequestOptions.DEFAULT); } - + /** * Executes the provided request using either the sync method or its async variant, both provided as functions */ protected static Resp execute(Req request, SyncMethod syncMethod, AsyncMethod asyncMethod, RequestOptions options) throws IOException { - if (randomBoolean()) { + if (async == false) { return syncMethod.execute(request, options); } else { PlainActionFuture future = PlainActionFuture.newFuture(); asyncMethod.execute(request, options, future); return future.actionGet(); } - } + } /** * Executes the provided request using either the sync method or its async @@ -100,7 +102,7 @@ protected static Resp execute(Req request, SyncMethod syn */ protected static Resp execute(SyncMethodNoRequest syncMethodNoRequest, AsyncMethodNoRequest asyncMethodNoRequest, RequestOptions requestOptions) throws IOException { - if (randomBoolean()) { + if (async == false) { return syncMethodNoRequest.execute(requestOptions); } else { PlainActionFuture future = PlainActionFuture.newFuture(); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/LicensingDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/LicensingDocumentationIT.java index 0d8fe0a5cd9a7..c181fca44c64b 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/LicensingDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/LicensingDocumentationIT.java @@ -194,7 +194,7 @@ public void testGetLicense() throws Exception { //end::get-license-response assertThat(currentLicense, containsString("trial")); - assertThat(currentLicense, containsString("integTest")); + assertThat(currentLicense, containsString("ntegTest")); } { GetLicenseRequest request = new GetLicenseRequest(); @@ -233,7 +233,7 @@ public void onFailure(Exception e) { String currentLicense = response.getLicenseDefinition(); assertThat(currentLicense, startsWith("{")); assertThat(currentLicense, containsString("trial")); - assertThat(currentLicense, containsString("integTest")); + assertThat(currentLicense, containsString("ntegTest")); assertThat(currentLicense, endsWith("}")); } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java index 0213f32068c1d..5ac1222d7f175 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java @@ -48,6 +48,7 @@ import org.elasticsearch.client.indices.CreateIndexRequest; import org.elasticsearch.cluster.SnapshotsInProgress; import org.elasticsearch.cluster.metadata.RepositoryMetaData; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentType; @@ -457,7 +458,12 @@ public void testSnapshotVerifyRepository() throws IOException { List repositoryMetaDataResponse = response.getNodes(); // end::verify-repository-response assertThat(1, equalTo(repositoryMetaDataResponse.size())); - assertThat("integTest-0", equalTo(repositoryMetaDataResponse.get(0).getName())); + final boolean async = Booleans.parseBoolean(System.getProperty("tests.rest.async", "false")); + if (async) { + assertThat("asyncIntegTest-0", equalTo(repositoryMetaDataResponse.get(0).getName())); + } else { + assertThat("integTest-0", equalTo(repositoryMetaDataResponse.get(0).getName())); + } } public void testSnapshotVerifyRepositoryAsync() throws InterruptedException { diff --git a/modules/transport-netty4/build.gradle b/modules/transport-netty4/build.gradle index 8483a7d2c00ea..9da97bd997073 100644 --- a/modules/transport-netty4/build.gradle +++ b/modules/transport-netty4/build.gradle @@ -69,7 +69,7 @@ TaskProvider pooledTest = tasks.register("pooledTest", Test) { systemProperty 'es.set.netty.runtime.available.processors', 'false' systemProperty 'es.use_unpooled_allocator', 'false' } -// TODO: we can't use task avoidance here because RestIntegTestTask does the testcluster creation + RestIntegTestTask pooledIntegTest = tasks.create("pooledIntegTest", RestIntegTestTask) { runner { systemProperty 'es.set.netty.runtime.available.processors', 'false'