Skip to content

Commit 8bb4290

Browse files
Fix IngestServiceTests.testBulkRequestExecutionWithFailures (#14918) (#14927)
The test would previously fail if the randomness led to only a single indexing request being included in the bulk payload. This change guarantees multiple indexing requests in order to ensure the batch logic kicks in. Also replace some unneeded mocks with real classes. (cherry picked from commit 087355f) Signed-off-by: Andrew Ross <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 544b137 commit 8bb4290

File tree

1 file changed

+22
-25
lines changed

1 file changed

+22
-25
lines changed

server/src/test/java/org/opensearch/ingest/IngestServiceTests.java

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
import org.opensearch.test.OpenSearchTestCase;
7979
import org.opensearch.threadpool.ThreadPool;
8080
import org.opensearch.threadpool.ThreadPool.Names;
81+
import org.hamcrest.MatcherAssert;
8182
import org.junit.Before;
8283

8384
import java.nio.charset.StandardCharsets;
@@ -104,15 +105,16 @@
104105

105106
import static java.util.Collections.emptyMap;
106107
import static java.util.Collections.emptySet;
108+
import static org.hamcrest.Matchers.contains;
107109
import static org.hamcrest.Matchers.equalTo;
108110
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
111+
import static org.hamcrest.Matchers.hasSize;
109112
import static org.hamcrest.Matchers.instanceOf;
110113
import static org.hamcrest.Matchers.is;
111114
import static org.hamcrest.Matchers.notNullValue;
112115
import static org.hamcrest.Matchers.nullValue;
113116
import static org.hamcrest.Matchers.sameInstance;
114117
import static org.mockito.Mockito.any;
115-
import static org.mockito.Mockito.anyInt;
116118
import static org.mockito.Mockito.anyString;
117119
import static org.mockito.Mockito.argThat;
118120
import static org.mockito.Mockito.doAnswer;
@@ -1106,27 +1108,23 @@ public void testExecuteFailureWithNestedOnFailure() throws Exception {
11061108
verify(completionHandler, times(1)).accept(Thread.currentThread(), null);
11071109
}
11081110

1109-
public void testBulkRequestExecutionWithFailures() throws Exception {
1111+
public void testBulkRequestExecutionWithFailures() {
11101112
BulkRequest bulkRequest = new BulkRequest();
11111113
String pipelineId = "_id";
11121114

1113-
int numRequest = scaledRandomIntBetween(8, 64);
1114-
int numIndexRequests = 0;
1115-
for (int i = 0; i < numRequest; i++) {
1116-
DocWriteRequest request;
1115+
int numIndexRequests = scaledRandomIntBetween(4, 32);
1116+
for (int i = 0; i < numIndexRequests; i++) {
1117+
IndexRequest indexRequest = new IndexRequest("_index").id("_id").setPipeline(pipelineId).setFinalPipeline("_none");
1118+
indexRequest.source(Requests.INDEX_CONTENT_TYPE, "field1", "value1");
1119+
bulkRequest.add(indexRequest);
1120+
}
1121+
int numOtherRequests = scaledRandomIntBetween(4, 32);
1122+
for (int i = 0; i < numOtherRequests; i++) {
11171123
if (randomBoolean()) {
1118-
if (randomBoolean()) {
1119-
request = new DeleteRequest("_index", "_id");
1120-
} else {
1121-
request = new UpdateRequest("_index", "_id");
1122-
}
1124+
bulkRequest.add(new DeleteRequest("_index", "_id"));
11231125
} else {
1124-
IndexRequest indexRequest = new IndexRequest("_index").id("_id").setPipeline(pipelineId).setFinalPipeline("_none");
1125-
indexRequest.source(Requests.INDEX_CONTENT_TYPE, "field1", "value1");
1126-
request = indexRequest;
1127-
numIndexRequests++;
1126+
bulkRequest.add(new UpdateRequest("_index", "_id"));
11281127
}
1129-
bulkRequest.add(request);
11301128
}
11311129

11321130
CompoundProcessor processor = mock(CompoundProcessor.class);
@@ -1155,23 +1153,22 @@ public void testBulkRequestExecutionWithFailures() throws Exception {
11551153
clusterState = IngestService.innerPut(putRequest, clusterState);
11561154
ingestService.applyClusterState(new ClusterChangedEvent("", clusterState, previousClusterState));
11571155

1158-
@SuppressWarnings("unchecked")
1159-
BiConsumer<Integer, Exception> requestItemErrorHandler = mock(BiConsumer.class);
1160-
@SuppressWarnings("unchecked")
1161-
final BiConsumer<Thread, Exception> completionHandler = mock(BiConsumer.class);
1156+
final Map<Integer, Exception> errorHandler = new HashMap<>();
1157+
final Map<Thread, Exception> completionHandler = new HashMap<>();
11621158
ingestService.executeBulkRequest(
1163-
numRequest,
1159+
numIndexRequests + numOtherRequests,
11641160
bulkRequest.requests(),
1165-
requestItemErrorHandler,
1166-
completionHandler,
1161+
errorHandler::put,
1162+
completionHandler::put,
11671163
indexReq -> {},
11681164
Names.WRITE,
11691165
bulkRequest
11701166
);
11711167

1172-
verify(requestItemErrorHandler, times(numIndexRequests)).accept(anyInt(), argThat(o -> o.getCause().equals(error)));
1168+
MatcherAssert.assertThat(errorHandler.entrySet(), hasSize(numIndexRequests));
1169+
errorHandler.values().forEach(e -> assertEquals(e.getCause(), error));
11731170

1174-
verify(completionHandler, times(1)).accept(Thread.currentThread(), null);
1171+
MatcherAssert.assertThat(completionHandler.keySet(), contains(Thread.currentThread()));
11751172
}
11761173

11771174
public void testBulkRequestExecution() throws Exception {

0 commit comments

Comments
 (0)