Skip to content

Commit f6d8f9a

Browse files
committed
Polish tests and add more test cases to verify error summary trimming
Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
1 parent 483e803 commit f6d8f9a

17 files changed

+145
-86
lines changed

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public class BulkRequestActionListener implements ActionListener<BulkResponse> {
3333
* Creates a new BulkRequestActionListener.
3434
*
3535
* @param responseObserver The gRPC stream observer to send the response back to the client
36-
* @param params
36+
* @param params parameters that are going to change how responses and errors are handled
3737
*/
3838
public BulkRequestActionListener(
3939
StreamObserver<org.opensearch.protobufs.BulkResponse> responseObserver,

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/listeners/SearchRequestActionListener.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public class SearchRequestActionListener implements ActionListener<SearchRespons
3434
* Constructs a new SearchRequestActionListener.
3535
*
3636
* @param responseObserver the gRPC stream observer to send the search response to
37+
* @param params parameters that are going to change how responses and errors are handled
3738
*/
3839
public SearchRequestActionListener(
3940
StreamObserver<org.opensearch.protobufs.SearchResponse> responseObserver,

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/ResponseHandlingParams.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ public class ResponseHandlingParams {
1414

1515
private final TracingLevel errorTracingLevel;
1616

17+
public ResponseHandlingParams() {
18+
this.errorTracingLevel = TracingLevel.SUMMARY;
19+
}
20+
1721
public ResponseHandlingParams(boolean detailedErrorsEnabled, GlobalParams errorTracesRequested) {
1822
this.errorTracingLevel = getTracingLevel(detailedErrorsEnabled, errorTracesRequested);
1923
}

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/GrpcPluginTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void testGetSettings() {
140140
assertTrue("SETTING_GRPC_KEEPALIVE_TIMEOUT should be included", settings.contains(SETTING_GRPC_KEEPALIVE_TIMEOUT));
141141

142142
// Verify the number of settings
143-
assertEquals("Should return 13 settings", 13, settings.size());
143+
assertEquals("Should return 14 settings", 14, settings.size());
144144
}
145145

146146
private static class LoadableMockServiceFactory implements GrpcServiceFactory {

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListenerTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.grpc.stub.StreamObserver;
2323
import org.mockito.Mock;
2424
import org.mockito.MockitoAnnotations;
25+
import org.opensearch.transport.grpc.proto.response.exceptions.ResponseHandlingParams;
2526

2627
import static org.mockito.ArgumentMatchers.any;
2728
import static org.mockito.Mockito.verify;
@@ -37,7 +38,7 @@ public class BulkRequestActionListenerTests extends OpenSearchTestCase {
3738
public void setUp() throws Exception {
3839
super.setUp();
3940
MockitoAnnotations.openMocks(this);
40-
listener = new BulkRequestActionListener(responseObserver);
41+
listener = new BulkRequestActionListener(responseObserver, new ResponseHandlingParams());
4142
}
4243

4344
public void testOnResponseWithSuccessfulResponse() {

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/SearchRequestActionListenerTests.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import io.grpc.stub.StreamObserver;
1818
import org.mockito.Mock;
1919
import org.mockito.MockitoAnnotations;
20+
import org.opensearch.transport.grpc.proto.response.exceptions.ResponseHandlingParams;
2021

2122
import static org.mockito.ArgumentMatchers.any;
2223
import static org.mockito.Mockito.mock;
@@ -34,7 +35,7 @@ public class SearchRequestActionListenerTests extends OpenSearchTestCase {
3435
public void setUp() throws Exception {
3536
super.setUp();
3637
MockitoAnnotations.openMocks(this);
37-
listener = new SearchRequestActionListener(responseObserver);
38+
listener = new SearchRequestActionListener(responseObserver, new ResponseHandlingParams());
3839
}
3940

4041
public void testOnResponse() {
@@ -60,20 +61,13 @@ public void testOnResponse() {
6061
}
6162

6263
public void testOnFailure() {
63-
// Create a mock StreamObserver
64-
@SuppressWarnings("unchecked")
65-
StreamObserver<org.opensearch.protobufs.SearchResponse> mockResponseObserver = mock(StreamObserver.class);
66-
67-
// Create a SearchRequestActionListener
68-
SearchRequestActionListener listener = new SearchRequestActionListener(mockResponseObserver);
69-
7064
// Create an exception
7165
Exception exception = new Exception("Test exception");
7266

7367
// Call the method under test
7468
listener.onFailure(exception);
7569

7670
// Verify that onError was called with a StatusRuntimeException
77-
verify(mockResponseObserver, times(1)).onError(any(StatusRuntimeException.class));
71+
verify(responseObserver, times(1)).onError(any(StatusRuntimeException.class));
7872
}
7973
}

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/BulkResponseProtoUtilsTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.opensearch.core.index.shard.ShardId;
1818
import org.opensearch.test.OpenSearchTestCase;
1919
import org.opensearch.transport.grpc.proto.response.document.bulk.BulkResponseProtoUtils;
20+
import org.opensearch.transport.grpc.proto.response.exceptions.ResponseHandlingParams;
2021

2122
import java.io.IOException;
2223

@@ -37,7 +38,7 @@ public void testToProtoWithSuccessfulResponse() throws IOException {
3738
BulkResponse bulkResponse = new BulkResponse(responses, 100);
3839

3940
// Convert to Protocol Buffer
40-
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse);
41+
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse, new ResponseHandlingParams());
4142

4243
// Verify the conversion
4344
assertEquals("Should have the correct took time", 100, protoResponse.getTook());
@@ -65,7 +66,7 @@ public void testToProtoWithFailedResponse() throws IOException {
6566
BulkResponse bulkResponse = new BulkResponse(responses, 100);
6667

6768
// Convert to Protocol Buffer
68-
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse);
69+
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse, new ResponseHandlingParams());
6970

7071
// Verify the conversion
7172
assertEquals("Should have the correct took time", 100, protoResponse.getTook());
@@ -94,7 +95,7 @@ public void testToProtoWithIngestTook() throws IOException {
9495
BulkResponse bulkResponse = new BulkResponse(responses, 100, 50);
9596

9697
// Convert to Protocol Buffer
97-
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse);
98+
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse, new ResponseHandlingParams());
9899

99100
// Verify the conversion
100101
assertEquals("Should have the correct took time", 100, protoResponse.getTook());

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtilsTests.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.opensearch.core.rest.RestStatus;
2222
import org.opensearch.index.get.GetResult;
2323
import org.opensearch.test.OpenSearchTestCase;
24+
import org.opensearch.transport.grpc.proto.response.exceptions.ResponseHandlingParams;
2425

2526
import java.io.IOException;
2627
import java.nio.charset.StandardCharsets;
@@ -47,7 +48,7 @@ public void testToProtoWithIndexResponse() throws IOException {
4748
BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.INDEX, indexResponse);
4849

4950
// Convert to protobuf ResponseItem
50-
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse);
51+
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, new ResponseHandlingParams());
5152

5253
// Verify the result
5354
assertNotNull("ResponseItem should not be null", responseItem);
@@ -72,7 +73,7 @@ public void testToProtoWithCreateResponse() throws IOException {
7273
BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.CREATE, indexResponse);
7374

7475
// Convert to protobuf ResponseItem
75-
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse);
76+
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, new ResponseHandlingParams());
7677

7778
// Verify the result
7879
assertNotNull("ResponseItem should not be null", responseItem);
@@ -97,7 +98,7 @@ public void testToProtoWithDeleteResponse() throws IOException {
9798
BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.DELETE, deleteResponse);
9899

99100
// Convert to protobuf ResponseItem
100-
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse);
101+
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, new ResponseHandlingParams());
101102

102103
// Verify the result
103104
assertNotNull("ResponseItem should not be null", responseItem);
@@ -122,7 +123,7 @@ public void testToProtoWithUpdateResponse() throws IOException {
122123
BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.UPDATE, updateResponse);
123124

124125
// Convert to protobuf Item
125-
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse);
126+
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, new ResponseHandlingParams());
126127

127128
// Verify the result
128129
assertNotNull("ResponseItem should not be null", responseItem);
@@ -165,7 +166,7 @@ public void testToProtoWithUpdateResponseAndGetResult() throws IOException {
165166
BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.UPDATE, updateResponse);
166167

167168
// Convert to protobuf Item
168-
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse);
169+
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, new ResponseHandlingParams());
169170

170171
// Verify the result
171172
assertNotNull("ResponseItem should not be null", responseItem);
@@ -195,7 +196,7 @@ public void testToProtoWithFailure() throws IOException {
195196
BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.INDEX, failure);
196197

197198
// Convert to protobuf Item
198-
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse);
199+
org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, new ResponseHandlingParams());
199200

200201
// Verify the result
201202
assertNotNull("ResponseItem should not be null", responseItem);
@@ -210,6 +211,6 @@ public void testToProtoWithFailure() throws IOException {
210211

211212
public void testToProtoWithNullResponse() throws IOException {
212213
// Call toProto with null, should throw NullPointerException
213-
expectThrows(NullPointerException.class, () -> BulkItemResponseProtoUtils.toProto(null));
214+
expectThrows(NullPointerException.class, () -> BulkItemResponseProtoUtils.toProto(null, new ResponseHandlingParams()));
214215
}
215216
}

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtilsTests.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.opensearch.core.index.shard.ShardId;
1515
import org.opensearch.protobufs.ResponseItem;
1616
import org.opensearch.test.OpenSearchTestCase;
17+
import org.opensearch.transport.grpc.proto.response.exceptions.ResponseHandlingParams;
1718

1819
import java.io.IOException;
1920

@@ -32,7 +33,7 @@ public void testToProtoWithIndexResponse() throws IOException {
3233
indexResponse.setForcedRefresh(true);
3334

3435
// Convert to protobuf ResponseItem.Builder
35-
ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse);
36+
ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse, new ResponseHandlingParams());
3637

3738
// Verify the result
3839
assertNotNull("ResponseItem.Builder should not be null", responseItemBuilder);
@@ -70,7 +71,7 @@ public void testToProtoWithEmptyId() throws IOException {
7071
indexResponse.setShardInfo(shardInfo);
7172

7273
// Convert to protobuf ResponseItem.Builder
73-
ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse);
74+
ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse, new ResponseHandlingParams());
7475

7576
// Verify the result
7677
assertNotNull("ResponseItem.Builder should not be null", responseItemBuilder);
@@ -94,7 +95,7 @@ public void testToProtoWithNoSeqNo() throws IOException {
9495
indexResponse.setShardInfo(shardInfo);
9596

9697
// Convert to protobuf ResponseItem.Builder
97-
ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse);
98+
ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse, new ResponseHandlingParams());
9899

99100
// Verify the result
100101
assertNotNull("ResponseItem.Builder should not be null", responseItemBuilder);
@@ -109,6 +110,6 @@ public void testToProtoWithNoSeqNo() throws IOException {
109110

110111
public void testToProtoWithNullResponse() throws IOException {
111112
// Call toProto with null, should throw NullPointerException
112-
expectThrows(NullPointerException.class, () -> DocWriteResponseProtoUtils.toProto(null));
113+
expectThrows(NullPointerException.class, () -> DocWriteResponseProtoUtils.toProto(null, new ResponseHandlingParams()));
113114
}
114115
}

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtilsTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.opensearch.core.rest.RestStatus;
1515
import org.opensearch.protobufs.ShardInfo;
1616
import org.opensearch.test.OpenSearchTestCase;
17+
import org.opensearch.transport.grpc.proto.response.exceptions.ResponseHandlingParams;
1718

1819
import java.io.IOException;
1920
import java.util.ArrayList;
@@ -26,7 +27,7 @@ public void testToProtoWithNoFailures() throws IOException {
2627
ReplicationResponse.ShardInfo shardInfo = new ReplicationResponse.ShardInfo(5, 3, new ReplicationResponse.ShardInfo.Failure[0]);
2728

2829
// Convert to protobuf ShardInfo
29-
ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo);
30+
ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo, new ResponseHandlingParams());
3031

3132
// Verify the result
3233
assertNotNull("ShardInfo should not be null", protoShardInfo);
@@ -55,7 +56,7 @@ public void testToProtoWithFailures() throws IOException {
5556
ReplicationResponse.ShardInfo shardInfo = new ReplicationResponse.ShardInfo(5, 3, failures);
5657

5758
// Convert to protobuf ShardInfo
58-
ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo);
59+
ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo, new ResponseHandlingParams());
5960

6061
// Verify the result
6162
assertNotNull("ShardInfo should not be null", protoShardInfo);
@@ -87,6 +88,6 @@ public void testToProtoWithFailures() throws IOException {
8788

8889
public void testToProtoWithNullShardInfo() throws IOException {
8990
// Call toProto with null, should throw NullPointerException
90-
expectThrows(NullPointerException.class, () -> ShardInfoProtoUtils.toProto(null));
91+
expectThrows(NullPointerException.class, () -> ShardInfoProtoUtils.toProto(null, new ResponseHandlingParams()));
9192
}
9293
}

0 commit comments

Comments
 (0)