Copilot commented on code in PR #101:
URL:
https://github.com/apache/skywalking-banyandb-java-client/pull/101#discussion_r2425509788
##########
src/test/java/org/apache/skywalking/banyandb/v1/client/ITTraceTest.java:
##########
@@ -136,14 +131,36 @@ public void testTraceQueryByTraceId() throws
BanyanDBException, ExecutionExcepti
.tag("start_time", Value.timestampTagValue(now.toEpochMilli()))
.span(spanData)
.version(1L);
-
- // Write the trace via bulk processor
- CompletableFuture<Void> writeFuture = processor.add(traceWrite);
- writeFuture.exceptionally(exp -> {
- Assert.fail("Write failed: " + exp.getMessage());
- return null;
+
+ StreamObserver<BanyandbTrace.WriteRequest> writeObserver
+ = client.getTraceServiceStub().write(new
StreamObserver<BanyandbTrace.WriteResponse>() {
+ @Override
+ public void onNext(BanyandbTrace.WriteResponse writeResponse) {
+ Assert.assertEquals("STATUS_SUCCEED",
writeResponse.getStatus());
Review Comment:
Using string comparison for status check is fragile. Consider using enum
comparison with BanyandbModel.Status.STATUS_SUCCEED for type safety.
##########
src/test/java/org/apache/skywalking/banyandb/v1/client/ITTraceTest.java:
##########
@@ -136,14 +131,36 @@ public void testTraceQueryByTraceId() throws
BanyanDBException, ExecutionExcepti
.tag("start_time", Value.timestampTagValue(now.toEpochMilli()))
.span(spanData)
.version(1L);
-
- // Write the trace via bulk processor
- CompletableFuture<Void> writeFuture = processor.add(traceWrite);
- writeFuture.exceptionally(exp -> {
- Assert.fail("Write failed: " + exp.getMessage());
- return null;
+
+ StreamObserver<BanyandbTrace.WriteRequest> writeObserver
+ = client.getTraceServiceStub().write(new
StreamObserver<BanyandbTrace.WriteResponse>() {
+ @Override
+ public void onNext(BanyandbTrace.WriteResponse writeResponse) {
+ Assert.assertEquals("STATUS_SUCCEED",
writeResponse.getStatus());
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ Assert.fail("write failed: " + throwable.getMessage());
+ }
+
+ @Override
+ public void onCompleted() {
+
+ }
});
- writeFuture.get(10, TimeUnit.SECONDS);
+ try {
+ writeObserver.onNext(traceWrite.build());
+ } finally {
+ writeObserver.onCompleted();
+ }
+ // Write the trace via bulk processor
+// CompletableFuture<Void> writeFuture = processor.add(traceWrite);
+// writeFuture.exceptionally(exp -> {
+// Assert.fail("Write failed: " + exp.getMessage());
+// return null;
+// });
+// writeFuture.get(10, TimeUnit.SECONDS);
Review Comment:
Remove commented out code rather than leaving it in the codebase. This
reduces clutter and maintains code cleanliness.
```suggestion
```
##########
src/test/java/org/apache/skywalking/banyandb/v1/client/ITBanyanDBStreamQueryTests.java:
##########
@@ -110,13 +106,28 @@ public void testStreamQuery_TraceID() throws
BanyanDBException, ExecutionExcepti
.tag("mq.topic", Value.stringTagValue(topic)) // 11
.tag("mq.queue", Value.stringTagValue(queue)); // 12
streamWrite.setTimestamp(now.toEpochMilli());
+ StreamObserver<BanyandbStream.WriteRequest> writeObserver
+ = client.getStreamServiceStub().write(new
StreamObserver<BanyandbStream.WriteResponse>() {
+ @Override
+ public void onNext(BanyandbStream.WriteResponse writeResponse) {
+ Assert.assertEquals("STATUS_SUCCEED",
writeResponse.getStatus());
Review Comment:
Using string comparison for status check is fragile. Consider using enum
comparison with BanyandbModel.Status.STATUS_SUCCEED for type safety.
```suggestion
Assert.assertEquals(BanyandbStream.WriteResponse.Status.STATUS_SUCCEED,
writeResponse.getStatus());
```
##########
src/test/java/org/apache/skywalking/banyandb/v1/client/ITBanyanDBMeasureQueryTests.java:
##########
@@ -81,13 +77,29 @@ public void testMeasureQuery() throws BanyanDBException,
ExecutionException, Int
MeasureWrite measureWrite = client.createMeasureWrite("sw_metric",
"service_cpm_minute", now.toEpochMilli());
measureWrite.tag("entity_id",
TagAndValue.stringTagValue("entity_1")).field("total",
TagAndValue.longFieldValue(100)).field("value", TagAndValue.longFieldValue(1));
+ StreamObserver<BanyandbMeasure.WriteRequest> writeObserver
+ = client.getMeasureServiceStub().write(new
StreamObserver<BanyandbMeasure.WriteResponse>() {
+ @Override
+ public void onNext(BanyandbMeasure.WriteResponse writeResponse) {
+ Assert.assertEquals("STATUS_SUCCEED",
writeResponse.getStatus());
Review Comment:
Using string comparison for status check is fragile. Consider using enum
comparison with BanyandbModel.Status.STATUS_SUCCEED for type safety.
```suggestion
Assert.assertEquals(BanyandbMeasure.WriteResponse.Status.STATUS_SUCCEED,
writeResponse.getStatus());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
