kfaraz commented on code in PR #16092:
URL: https://github.com/apache/druid/pull/16092#discussion_r1518744813
##########
services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java:
##########
@@ -278,28 +278,13 @@ public void testJDBCSqlProxy() throws Exception
public void testHandleExceptionWithFilterDisabled() throws Exception
{
String errorMessage = "test exception message";
- ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
- HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
- ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
- Mockito.when(response.getOutputStream()).thenReturn(outputStream);
- final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
- new MapQueryToolChestWarehouse(ImmutableMap.of()),
- mockMapper,
- TestHelper.makeSmileMapper(),
- null,
- null,
- null,
- new NoopServiceEmitter(),
- new NoopRequestLogger(),
- new DefaultGenericQueryMetricsFactory(),
- new AuthenticatorMapper(ImmutableMap.of()),
- new Properties(),
- new ServerConfig()
+ ServerConfig serverConfig = new ServerConfig();
+ ArgumentCaptor<Exception> captor = executeServletAction(
+ serverConfig,
+ errorMessage,
+ (servlet, request, response, mapper, exception)
Review Comment:
Looking at all the tests, I think we don't really need to pass the
`errorMessage` to `executeServletAction`, neither to the lambda.
##########
services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java:
##########
@@ -1063,4 +969,34 @@ public int getPort()
return port;
}
}
+ interface ServletTestAction {
+ void execute(AsyncQueryForwardingServlet servlet, HttpServletRequest
request, HttpServletResponse response, ObjectMapper mapper, String
errorMessage) throws Exception;
+ }
+ private ArgumentCaptor<Exception> executeServletAction(ServerConfig
serverConfig, String errorMessage, ServletTestAction action) throws Exception
Review Comment:
Rename the method to suggest what it is actually being used for:
```suggestion
private ArgumentCaptor<Exception>
captureExceptionHandledByServlet(ServerConfig serverConfig, String
errorMessage, ServletTestAction action) throws Exception
```
--
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]
