cbalci commented on code in PR #15271:
URL: https://github.com/apache/pinot/pull/15271#discussion_r1996143580
##########
pinot-core/src/test/java/org/apache/pinot/core/transport/grpc/GrpcQueryServerTest.java:
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.transport.grpc;
+
+import io.grpc.stub.StreamObserver;
+import org.apache.pinot.common.config.GrpcConfig;
+import org.apache.pinot.common.metrics.ServerMetrics;
+import org.apache.pinot.common.proto.Server;
+import org.apache.pinot.core.operator.blocks.InstanceResponseBlock;
+import org.apache.pinot.core.query.executor.QueryExecutor;
+import org.apache.pinot.server.access.AccessControl;
+import org.apache.pinot.spi.trace.Tracing;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.*;
+
+public class GrpcQueryServerTest {
+ private GrpcQueryServer _grpcQueryServer;
+ @Mock private QueryExecutor _queryExecutor;
+ @Mock private ServerMetrics _serverMetrics;
+ @Mock private AccessControl _accessControl;
+ @Mock private StreamObserver<Server.ServerResponse> _responseObserver;
+ @Mock private GrpcConfig _grpcConfig;
+
+ @BeforeMethod
+ public void setUp() {
+ _queryExecutor = mock(QueryExecutor.class);
+ _serverMetrics = mock(ServerMetrics.class);
+ _accessControl = mock(AccessControl.class);
+ _responseObserver = mock(StreamObserver.class);
+ _grpcConfig = mock(GrpcConfig.class);
+
+ _grpcQueryServer = new GrpcQueryServer(1234, _grpcConfig, null,
+ _queryExecutor, _serverMetrics, _accessControl);
+ _grpcQueryServer.start();
+ }
+
+ @Test
+ public void testEnsureGrpcQueryProcessingInitializesTracingContext() {
+ // Arrange
+ InstanceResponseBlock mockedInstanceResponseBlock =
mock(InstanceResponseBlock.class);
+ doNothing().when(_serverMetrics).addMeteredGlobalValue(any(),
anyLong());
+ when(_accessControl.hasDataAccess(any(), any())).thenReturn(false);
+ when(_queryExecutor.execute(any(), any(),
any())).thenReturn(mockedInstanceResponseBlock);
+
+ Server.ServerRequest serverRequest = Server.ServerRequest
+ .newBuilder()
+ .setSql("SELECT x FROM myTable")
+ .build();
+
+ try (MockedStatic<Tracing.ThreadAccountantOps> mockedStatic =
+ mockStatic(Tracing.ThreadAccountantOps.class)) {
+ // Act
+ _grpcQueryServer.submit(serverRequest, _responseObserver);
+
+ // Assert: ThreadAccountantOps.setupRunner() was called once.
+ mockedStatic.verify(() ->
Tracing.ThreadAccountantOps.setupRunner(anyString()), times(1));
Review Comment:
This test is fine, but can we write a test case which actually validates the
gRPC query server can execute queries?
The reason I'm insistent on the test is that the Grpc transport is secondary
to the the HttpServer in terms of importance and when folks do sweeping changes
(like the tracing setup), regressions to the gRPC server slip unnoticed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
