Skip to content

Commit

Permalink
Remove random when using HLRC sync and async calls (#48211)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hub-cap committed Oct 24, 2019
1 parent b034153 commit c19379e
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 13 deletions.
23 changes: 18 additions & 5 deletions client/rest-high-level/build.gradle
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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:*" ]'
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -78,20 +80,20 @@ protected static <Req, Resp> Resp execute(Req request, SyncMethod<Req, Resp> syn
AsyncMethod<Req, Resp> 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 <Req, Resp> Resp execute(Req request, SyncMethod<Req, Resp> syncMethod,
AsyncMethod<Req, Resp> asyncMethod, RequestOptions options) throws IOException {
if (randomBoolean()) {
if (async == false) {
return syncMethod.execute(request, options);
} else {
PlainActionFuture<Resp> future = PlainActionFuture.newFuture();
asyncMethod.execute(request, options, future);
return future.actionGet();
}
}
}

/**
* Executes the provided request using either the sync method or its async
Expand All @@ -100,7 +102,7 @@ protected static <Req, Resp> Resp execute(Req request, SyncMethod<Req, Resp> syn
*/
protected static <Resp> Resp execute(SyncMethodNoRequest<Resp> syncMethodNoRequest, AsyncMethodNoRequest<Resp> asyncMethodNoRequest,
RequestOptions requestOptions) throws IOException {
if (randomBoolean()) {
if (async == false) {
return syncMethodNoRequest.execute(requestOptions);
} else {
PlainActionFuture<Resp> future = PlainActionFuture.newFuture();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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("}"));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -457,7 +458,12 @@ public void testSnapshotVerifyRepository() throws IOException {
List<VerifyRepositoryResponse.NodeView> 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 {
Expand Down
2 changes: 1 addition & 1 deletion modules/transport-netty4/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ TaskProvider<Test> 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'
Expand Down

0 comments on commit c19379e

Please sign in to comment.