Skip to content

[Test] Fix MigrationDocumentationIT.testClusterHealth #27774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 13, 2017
Merged

[Test] Fix MigrationDocumentationIT.testClusterHealth #27774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions ...-level/src/test/java/org/elasticsearch/client/documentation/MigrationDocumentationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.delete.DeleteResponse;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.client.ESRestHighLevelClientTestCase;
Expand All @@ -43,6 +44,7 @@
import java.util.Map;

import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;

Expand All @@ -67,7 +69,7 @@ public class MigrationDocumentationIT extends ESRestHighLevelClientTestCase {
public void testCreateIndex() throws IOException {
RestHighLevelClient client = highLevelClient();
{
//tag::migration-create-inded
//tag::migration-create-index
Settings indexSettings = Settings.builder() // <1>
.put(SETTING_NUMBER_OF_SHARDS, 1)
.put(SETTING_NUMBER_OF_REPLICAS, 0)
Expand Down Expand Up @@ -95,7 +97,7 @@ public void testCreateIndex() throws IOException {
if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
// <7>
}
//end::migration-create-inded
//end::migration-create-index
assertEquals(200, response.getStatusLine().getStatusCode());
}
}
Expand All @@ -104,7 +106,8 @@ public void testClusterHealth() throws IOException {
RestHighLevelClient client = highLevelClient();
{
//tag::migration-cluster-health
Response response = client.getLowLevelClient().performRequest("GET", "/_cluster/health"); // <1>
Map<String, String> parameters = singletonMap("wait_for_status", "green");
Response response = client.getLowLevelClient().performRequest("GET", "/_cluster/health", parameters); // <1>

ClusterHealthStatus healthStatus;
try (InputStream is = response.getEntity().getContent()) { // <2>
Expand All @@ -120,7 +123,7 @@ public void testClusterHealth() throws IOException {
}
}

public void testRequests() throws IOException {
public void testRequests() throws Exception {
RestHighLevelClient client = highLevelClient();
{
//tag::migration-request-ctor
Expand All @@ -133,13 +136,6 @@ public void testRequests() throws IOException {
//end::migration-request-ctor-execution
assertEquals(RestStatus.CREATED, response.status());
}
{
//tag::migration-request-sync-execution
DeleteRequest request = new DeleteRequest("index", "doc", "id");
DeleteResponse response = client.delete(request); // <1>
//end::migration-request-sync-execution
assertEquals(RestStatus.OK, response.status());
}
{
//tag::migration-request-async-execution
DeleteRequest request = new DeleteRequest("index", "doc", "id"); // <1>
Expand All @@ -155,6 +151,14 @@ public void onFailure(Exception e) {
}
});
//end::migration-request-async-execution
assertBusy(() -> assertFalse(client.exists(new GetRequest("index", "doc", "id"))));
Copy link
Member

@javanna javanna Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this another problem? I wonder why this assertBusy is needed.

Copy link
Member Author

@tlrx tlrx Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, previously the last code snippet was an async deletion and the test sometimes fails (or warn a thread leak) because of ongoing operation when shutting down the cluster.

I swapped the async and syn deletion operation so that the async is executed first, and this assertBusy is here to wait for the async operation to finish. A CountDownLatch would have been better but since this is a documentation code snippet I didn't want the latch to appear in doc so I did it this way.

Copy link
Member

@javanna javanna Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, that makes sense. Maybe add a tiny comment on why the assertBusy is there (the wait could be elsewhere but that would also appear in the docs snippet which is not good)

}
{
//tag::migration-request-sync-execution
DeleteRequest request = new DeleteRequest("index", "doc", "id");
DeleteResponse response = client.delete(request); // <1>
//end::migration-request-sync-execution
assertEquals(RestStatus.NOT_FOUND, response.status());
}
}
}
6 changes: 3 additions & 3 deletions docs/java-rest/high-level/migration.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ The same operation executed with the low-level client could be:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/MigrationDocumentationIT.java[migration-create-inded]
include-tagged::{doc-tests}/MigrationDocumentationIT.java[migration-create-index]
--------------------------------------------------
<1> Define the settings of the index
<2> Define the body of the HTTP request using a `XContentBuilder` with JSON format
Expand Down Expand Up @@ -331,8 +331,8 @@ With the low-level client, the code can be changed to:
--------------------------------------------------
include-tagged::{doc-tests}/MigrationDocumentationIT.java[migration-cluster-health]
--------------------------------------------------
<1> Call the cluster's health REST endpoint using the default paramaters
and gets back a `Response` object.
<1> Call the cluster's health REST endpoint and wait for the cluster health to become green,
then get back a `Response` object.
<2> Retrieve an `InputStream` object in order to read the response's content
<3> Parse the response's content using Elasticsearch's helper class `XContentHelper`. This
helper requires the content type of the response to be passed as an argument and returns
Expand Down