chia7712 commented on code in PR #16350:
URL: https://github.com/apache/kafka/pull/16350#discussion_r1642307332
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginScannerTest.java:
##########
@@ -17,161 +17,156 @@
package org.apache.kafka.connect.runtime.isolation;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
+import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.assertFalse;
-@RunWith(Parameterized.class)
public class PluginScannerTest {
private enum ScannerType { Reflection, ServiceLoader }
- @Rule
- public TemporaryFolder pluginDir = new TemporaryFolder();
+ @TempDir
+ File pluginDir;
- private final PluginScanner scanner;
+ private Map<ScannerType, PluginScanner> scannerMap;
- @Parameterized.Parameters
- public static Collection<Object[]> parameters() {
- List<Object[]> values = new ArrayList<>();
- for (ScannerType type : ScannerType.values()) {
- values.add(new Object[]{type});
- }
- return values;
+ @BeforeEach
+ public void setScanner() {
Review Comment:
Maybe we can parameterize scanner directly?
```java
static Stream<PluginScanner> parameters() {
return Stream.of(new ReflectionScanner(), new
ServiceLoaderScanner());
}
```
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/WorkerGroupMemberTest.java:
##########
@@ -27,25 +27,28 @@
import org.apache.kafka.connect.runtime.WorkerConfig;
import org.apache.kafka.connect.storage.ConfigBackingStore;
import org.junit.Test;
Review Comment:
please update this package
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestClientTest.java:
##########
@@ -105,228 +106,235 @@ private static <T> RestClient.HttpResponse<T>
httpRequest(
);
}
-
- @RunWith(Parameterized.class)
- public static class RequestFailureParameterizedTest {
-
- @Rule
- public MockitoRule initRule =
MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);
-
- @Mock
- private HttpClient httpClient;
-
- @Parameterized.Parameter
- public Throwable requestException;
-
- @Parameterized.Parameters
- public static Collection<Object[]> requestExceptions() {
- return Arrays.asList(new Object[][]{
- {new InterruptedException()},
- {new ExecutionException(null)},
- {new TimeoutException()}
- });
- }
-
- private static Request buildThrowingMockRequest(Throwable t) throws
ExecutionException, InterruptedException, TimeoutException {
- Request req = mock(Request.class);
- when(req.header(anyString(), anyString())).thenReturn(req);
- when(req.send()).thenThrow(t);
- return req;
- }
-
- @Test
- public void testFailureDuringRequestCausesInternalServerError() throws
Exception {
- Request request = buildThrowingMockRequest(requestException);
- when(httpClient.newRequest(anyString())).thenReturn(request);
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- assertIsInternalServerError(e);
- assertEquals(requestException, e.getCause());
- }
+ private static Stream<Arguments> requestExceptions() {
+ return Stream.of(
+ Arguments.of(new InterruptedException()),
+ Arguments.of(new ExecutionException(null)),
+ Arguments.of(new TimeoutException())
+ );
}
+ private static Request buildThrowingMockRequest(Throwable t) throws
ExecutionException, InterruptedException, TimeoutException {
+ Request req = mock(Request.class);
+ when(req.header(anyString(), anyString())).thenReturn(req);
+ when(req.send()).thenThrow(t);
+ return req;
+ }
- @RunWith(MockitoJUnitRunner.StrictStubs.class)
- public static class Tests {
- @Mock
- private HttpClient httpClient;
-
- private static String toJsonString(Object obj) {
- try {
- return OBJECT_MAPPER.writeValueAsString(obj);
- } catch (JsonProcessingException e) {
- throw new RuntimeException(e);
- }
- }
-
- private void setupHttpClient(int responseCode, String
responseJsonString) throws Exception {
- Request req = mock(Request.class);
- ContentResponse resp = mock(ContentResponse.class);
- when(resp.getStatus()).thenReturn(responseCode);
- when(resp.getContentAsString()).thenReturn(responseJsonString);
- when(req.send()).thenReturn(resp);
- when(req.header(anyString(), anyString())).thenReturn(req);
- when(httpClient.newRequest(anyString())).thenReturn(req);
- }
-
- @Test
- public void testNullUrl() throws Exception {
- int statusCode = Response.Status.OK.getStatusCode();
- setupHttpClient(statusCode, toJsonString(TEST_DTO));
-
- assertThrows(NullPointerException.class, () -> httpRequest(
- httpClient, null, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- }
-
- @Test
- public void testNullMethod() throws Exception {
- int statusCode = Response.Status.OK.getStatusCode();
- setupHttpClient(statusCode, toJsonString(TEST_DTO));
-
- assertThrows(NullPointerException.class, () -> httpRequest(
- httpClient, MOCK_URL, null, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- }
+ @ParameterizedTest
+ @MethodSource("requestExceptions")
+ public void testFailureDuringRequestCausesInternalServerError(Throwable
requestException) throws Exception {
+ Request request = buildThrowingMockRequest(requestException);
+ when(httpClient.newRequest(anyString())).thenReturn(request);
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ assertIsInternalServerError(e);
+ assertEquals(requestException, e.getCause());
+ }
- @Test
- public void testNullResponseType() throws Exception {
- int statusCode = Response.Status.OK.getStatusCode();
- setupHttpClient(statusCode, toJsonString(TEST_DTO));
+ @Test
+ public void testNullUrl() {
+ RestClient client = spy(new RestClient(null));
+ assertThrows(NullPointerException.class, () -> {
+ client.httpRequest(
+ null,
+ TEST_METHOD,
+ null,
+ TEST_DTO,
+ TEST_TYPE,
+ MOCK_SECRET_KEY,
+ TEST_SIGNATURE_ALGORITHM
+ );
+ });
+ }
- assertThrows(NullPointerException.class, () -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, null,
TEST_SIGNATURE_ALGORITHM
- ));
- }
+ @Test
+ public void testNullMethod() {
+ RestClient client = spy(new RestClient(null));
+ assertThrows(NullPointerException.class, () -> client.httpRequest(
+ MOCK_URL,
+ null,
+ null,
+ TEST_DTO,
+ TEST_TYPE,
+ MOCK_SECRET_KEY,
+ TEST_SIGNATURE_ALGORITHM
+ ));
+ }
- @Test
- public void testSuccess() throws Exception {
- int statusCode = Response.Status.OK.getStatusCode();
- setupHttpClient(statusCode, toJsonString(TEST_DTO));
+ @Test
+ public void testNullResponseType() {
+ RestClient client = spy(new RestClient(null));
+ assertThrows(NullPointerException.class, () -> client.httpRequest(
+ MOCK_URL,
+ TEST_METHOD,
+ null,
+ TEST_DTO,
+ null,
+ MOCK_SECRET_KEY,
+ TEST_SIGNATURE_ALGORITHM
+ ));
+ }
- RestClient.HttpResponse<TestDTO> httpResp = httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- );
- assertEquals(statusCode, httpResp.status());
- assertEquals(TEST_DTO, httpResp.body());
- }
+ @Test
+ public void testSuccess() throws Exception {
+ int statusCode = Response.Status.OK.getStatusCode();
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+ when(resp.getContentAsString()).thenReturn(toJsonString(TEST_DTO));
+ setupHttpClient(statusCode, req, resp);
- @Test
- public void testNoContent() throws Exception {
- int statusCode = Response.Status.NO_CONTENT.getStatusCode();
- setupHttpClient(statusCode, null);
+ RestClient.HttpResponse<TestDTO> httpResp = httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ );
+ assertEquals(statusCode, httpResp.status());
+ assertEquals(TEST_DTO, httpResp.body());
+ }
- RestClient.HttpResponse<TestDTO> httpResp = httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- );
- assertEquals(statusCode, httpResp.status());
- assertNull(httpResp.body());
- }
+ @Test
+ public void testNoContent() throws Exception {
+ int statusCode = Response.Status.NO_CONTENT.getStatusCode();
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+ setupHttpClient(statusCode, req, resp);
- @Test
- public void testStatusCodeAndErrorMessagePreserved() throws Exception {
- int statusCode = Response.Status.CONFLICT.getStatusCode();
- ErrorMessage errorMsg = new
ErrorMessage(Response.Status.GONE.getStatusCode(), "Some Error Message");
- setupHttpClient(statusCode, toJsonString(errorMsg));
-
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- assertEquals(statusCode, e.statusCode());
- assertEquals(errorMsg.errorCode(), e.errorCode());
- assertEquals(errorMsg.message(), e.getMessage());
- }
+ RestClient.HttpResponse<TestDTO> httpResp = httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ );
+ assertEquals(statusCode, httpResp.status());
+ assertNull(httpResp.body());
+ }
- @Test
- public void testNonEmptyResponseWithVoidResponseType() throws
Exception {
- int statusCode = Response.Status.OK.getStatusCode();
- setupHttpClient(statusCode, toJsonString(TEST_DTO));
+ @Test
+ public void testStatusCodeAndErrorMessagePreserved() throws Exception {
+ int statusCode = Response.Status.CONFLICT.getStatusCode();
+ ErrorMessage errorMsg = new
ErrorMessage(Response.Status.GONE.getStatusCode(), "Some Error Message");
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+ when(resp.getContentAsString()).thenReturn(toJsonString(errorMsg));
+ setupHttpClient(statusCode, req, resp);
+
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ assertEquals(statusCode, e.statusCode());
+ assertEquals(errorMsg.errorCode(), e.errorCode());
+ assertEquals(errorMsg.message(), e.getMessage());
+ }
- TypeReference<Void> voidResponse = new TypeReference<Void>() { };
- RestClient.HttpResponse<Void> httpResp = httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, voidResponse,
TEST_SIGNATURE_ALGORITHM
- );
- assertEquals(statusCode, httpResp.status());
- assertNull(httpResp.body());
- }
+ @Test
+ public void testNonEmptyResponseWithVoidResponseType() throws Exception {
+ int statusCode = Response.Status.OK.getStatusCode();
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+ when(resp.getContentAsString()).thenReturn(toJsonString(TEST_DTO));
+ setupHttpClient(statusCode, req, resp);
+
+ TypeReference<Void> voidResponse = new TypeReference<Void>() { };
+ RestClient.HttpResponse<Void> httpResp = httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, voidResponse,
TEST_SIGNATURE_ALGORITHM
+ );
+ assertEquals(statusCode, httpResp.status());
+ assertNull(httpResp.body());
+ }
- @Test
- public void testUnexpectedHttpResponseCausesInternalServerError()
throws Exception {
- int statusCode = Response.Status.NOT_MODIFIED.getStatusCode(); //
never thrown explicitly -
- // should be treated as an unexpected error and translated into
500 INTERNAL_SERVER_ERROR
+ @Test
+ public void testUnexpectedHttpResponseCausesInternalServerError() throws
Exception {
+ int statusCode = Response.Status.NOT_MODIFIED.getStatusCode();
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+
+ setupHttpClient(statusCode, req, resp);
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ assertIsInternalServerError(e);
+ }
- setupHttpClient(statusCode, null);
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- assertIsInternalServerError(e);
- }
+ @Test
+ public void testRuntimeExceptionCausesInternalServerError() {
+ when(httpClient.newRequest(anyString())).thenThrow(new
RuntimeException());
- @Test
- public void testRuntimeExceptionCausesInternalServerError() {
- when(httpClient.newRequest(anyString())).thenThrow(new
RuntimeException());
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ assertIsInternalServerError(e);
+ }
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- assertIsInternalServerError(e);
- }
+ @Test
+ public void testRequestSignatureFailureCausesInternalServerError() {
+ String invalidRequestSignatureAlgorithm = "Foo";
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
invalidRequestSignatureAlgorithm
+ ));
+ assertIsInternalServerError(e);
+ }
- @Test
- public void testRequestSignatureFailureCausesInternalServerError()
throws Exception {
- setupHttpClient(0, null);
+ @Test
+ public void testIOExceptionCausesInternalServerError() throws Exception {
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+ setupHttpClient(201, req, resp);
- String invalidRequestSignatureAlgorithm = "Foo";
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
invalidRequestSignatureAlgorithm
- ));
- assertIsInternalServerError(e);
- }
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ assertIsInternalServerError(e);
+ }
- @Test
- public void testIOExceptionCausesInternalServerError() throws
Exception {
- String invalidJsonString = "Invalid";
- setupHttpClient(201, invalidJsonString);
+ @Test
+ public void testUseSslConfigsOnlyWhenNecessary() throws Exception {
+ int statusCode = Response.Status.OK.getStatusCode();
+ Request req = mock(Request.class);
+ ContentResponse resp = mock(ContentResponse.class);
+ when(resp.getContentAsString()).thenReturn(toJsonString(TEST_DTO));
+ setupHttpClient(statusCode, req, resp);
+ assertDoesNotThrow(() -> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ String httpsUrl = "https://localhost:1234/api/endpoint";;
+ RestClient client = spy(new RestClient(null));
+ assertThrows(RuntimeException.class, () -> client.httpRequest(
+ httpsUrl,
+ TEST_METHOD,
+ null,
+ TEST_DTO,
+ TEST_TYPE,
+ MOCK_SECRET_KEY,
+ TEST_SIGNATURE_ALGORITHM
+ ));
+ }
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- assertIsInternalServerError(e);
- }
+ @Test
+ public void testHttpRequestInterrupted() throws ExecutionException,
InterruptedException, TimeoutException {
+ Request req = mock(Request.class);
+ doThrow(new InterruptedException()).when(req).send();
+ doReturn(req).when(req).header(anyString(), anyString());
+ doReturn(req).when(httpClient).newRequest(anyString());
+ ConnectRestException e = assertThrows(ConnectRestException.class, ()
-> httpRequest(
+ httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
+ ));
+ assertIsInternalServerError(e);
+ assertInstanceOf(InterruptedException.class, e.getCause());
+ assertTrue(Thread.interrupted());
+ }
- @Test
- public void testUseSslConfigsOnlyWhenNecessary() throws Exception {
- // See KAFKA-14816; we want to make sure that even if the worker
is configured with invalid SSL properties,
- // REST requests only fail if we try to contact a URL using HTTPS
(but not HTTP)
- int statusCode = Response.Status.OK.getStatusCode();
- setupHttpClient(statusCode, toJsonString(TEST_DTO));
-
- assertDoesNotThrow(() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- String httpsUrl = "https://localhost:1234/api/endpoint";;
- assertThrows(RuntimeException.class, () -> httpRequest(
- httpClient, httpsUrl, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- }
+ private void setupHttpClient(int responseCode, Request req,
ContentResponse resp) throws Exception {
+ when(resp.getStatus()).thenReturn(responseCode);
+ when(req.send()).thenReturn(resp);
+ when(req.header(anyString(), anyString())).thenReturn(req);
+ when(httpClient.newRequest(anyString())).thenReturn(req);
+ }
- @Test
- public void testHttpRequestInterrupted() throws ExecutionException,
InterruptedException, TimeoutException {
- Request req = mock(Request.class);
- doThrow(new InterruptedException()).when(req).send();
- doReturn(req).when(req).header(anyString(), anyString());
- doReturn(req).when(httpClient).newRequest(anyString());
- ConnectRestException e = assertThrows(ConnectRestException.class,
() -> httpRequest(
- httpClient, MOCK_URL, TEST_METHOD, TEST_TYPE,
TEST_SIGNATURE_ALGORITHM
- ));
- assertIsInternalServerError(e);
- assertInstanceOf(InterruptedException.class, e.getCause());
- assertTrue(Thread.interrupted());
+ private String toJsonString(Object obj) {
+ try {
+ return OBJECT_MAPPER.writeValueAsString(obj);
Review Comment:
we can wrap it by `assertDoesNotThrow`
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
