Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
chia7712 merged PR #16350: URL: https://github.com/apache/kafka/pull/16350 -- 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
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
frankvicky commented on PR #16350: URL: https://github.com/apache/kafka/pull/16350#issuecomment-2173322889 Hi @chia7712, I have make some changes based on feedback, PTAL 🔹 -- 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
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
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 scannerMap;
-@Parameterized.Parameters
-public static Collection parameters() {
-List 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 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 RestClient.HttpResponse
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 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 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.Strict
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
frankvicky commented on PR #16350: URL: https://github.com/apache/kafka/pull/16350#issuecomment-2171124111 Hi @chia7712, I have make some changes based on your example to keep `StandaloneHerderTest` has minimum changes during migration, PTAL 😸 -- 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
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
chia7712 commented on code in PR #16350:
URL: https://github.com/apache/kafka/pull/16350#discussion_r1641453002
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java:
##
@@ -147,7 +147,7 @@
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.withSettings;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension .class)
Review Comment:
please add `@MockitoSettings(strictness = Strictness.STRICT_STUBS)`
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java:
##
@@ -147,7 +147,7 @@
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.withSettings;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension .class)
Review Comment:
also, please remove redundant "space"
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -110,7 +111,8 @@
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.withSettings;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.STRICT_STUBS)
@SuppressWarnings("unchecked")
public class StandaloneHerderTest {
Review Comment:
Could we reuse the existent code as much as possible? please take a look
this example:
[StandaloneHerderTest.txt](https://github.com/user-attachments/files/15855186/StandaloneHerderTest.txt)
--
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
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
frankvicky commented on PR #16350: URL: https://github.com/apache/kafka/pull/16350#issuecomment-2169426511 Hi @chia7712 , @m1a2st I have do some changes, PTAL 😄 -- 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
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
frankvicky commented on code in PR #16350:
URL: https://github.com/apache/kafka/pull/16350#discussion_r1640916109
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/SynchronizationTest.java:
##
@@ -113,19 +111,14 @@ public synchronized void clear() {
public synchronized void set(Predicate predicate) {
clear();
this.predicate = predicate;
-// As soon as the barrier is tripped, the barrier will be reset
for the next round.
barrier = new CyclicBarrier(2);
}
-/**
Review Comment:
by mistake 😢
will roll it back.
--
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
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
m1a2st commented on code in PR #16350:
URL: https://github.com/apache/kafka/pull/16350#discussion_r1640841663
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestClientTest.java:
##
@@ -352,4 +373,4 @@ public int hashCode() {
return Objects.hash(content);
}
}
-}
Review Comment:
Please revert this unrelated change
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/WorkerCoordinatorIncrementalTest.java:
##
@@ -116,19 +112,15 @@ public class WorkerCoordinatorIncrementalTest {
// Arguments are:
// - Protocol type
// - Expected metadata size
-@Parameters
-public static Iterable mode() {
-return Arrays.asList(new Object[][]{{COMPATIBLE, 2}, {SESSIONED, 3}});
+static Stream mode() {
+return Stream.of(
+Arguments.of(ConnectProtocolCompatibility.COMPATIBLE, 2),
+Arguments.of(ConnectProtocolCompatibility.SESSIONED, 3)
+);
}
-@Parameter
-public ConnectProtocolCompatibility compatibility;
-
-@Parameter(1)
-public int expectedMetadataSize;
-
-@Before
-public void setup() {
+public void init(ConnectProtocolCompatibility compatibility) {
+System.err.println("init:" + compatibility);
Review Comment:
Do there need to print compatibility value?
##
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 scannerMap;
-@Parameterized.Parameters
-public static Collection parameters() {
-List values = new ArrayList<>();
-for (ScannerType type : ScannerType.values()) {
-values.add(new Object[]{type});
-}
-return values;
+@BeforeEach
+public void setScanner() {
+scannerMap = new HashMap<>();
+scannerMap.put(ScannerType.Reflection, new ReflectionScanner());
+scannerMap.put(ScannerType.ServiceLoader, new ServiceLoaderScanner());
}
-public PluginScannerTest(ScannerType scannerType) {
-switch (scannerType) {
-case Reflection:
-this.scanner = new ReflectionScanner();
-break;
-case ServiceLoader:
-this.scanner = new ServiceLoaderScanner();
-break;
-default:
-throw new IllegalArgumentException("Unknown type " +
scannerType);
-}
+static Stream parameters() {
+return Stream.of(ScannerType.values());
}
-@BeforeClass
+@BeforeAll
public static void setUp() {
// Work around a circular-dependency in TestPlugins.
TestPlugins.pluginPath();
}
-@Test
-public void testScanningEmptyPluginPath() {
-PluginScanResult result = scan(
-Collections.emptySet()
-);
+@ParameterizedTest
+@MethodSource("parameters")
+public void testScanningEmptyPluginPath(ScannerType scannerType) {
+PluginScanResult result = scan(scannerType, Collections.emptySet());
assertTrue(result.isEmpty());
}
-@Test
-public void testScanningPluginClasses() {
-PluginScanResult result = scan(
-TestPlugins.pluginPath()
-);
+@ParameterizedTest
+@MethodSource("parameters")
+public void testScanningPluginClasses(ScannerType scannerType) {
+PluginScanResult result = scan(scannerType, TestPlugins.pluginPath());
Set classes = new HashSet<>();
Re: [PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
chia7712 commented on code in PR #16350:
URL: https://github.com/apache/kafka/pull/16350#discussion_r1640826948
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedConfigTest.java:
##
@@ -43,18 +43,18 @@
import static
org.apache.kafka.connect.runtime.distributed.DistributedConfig.INTER_WORKER_KEY_GENERATION_ALGORITHM_CONFIG;
import static
org.apache.kafka.connect.runtime.distributed.DistributedConfig.INTER_WORKER_SIGNATURE_ALGORITHM_CONFIG;
import static
org.apache.kafka.connect.runtime.distributed.DistributedConfig.INTER_WORKER_VERIFICATION_ALGORITHMS_CONFIG;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension.class)
Review Comment:
Please add `@MockitoSettings(strictness = Strictness.STRICT_STUBS)` too
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/WorkerGroupMemberTest.java:
##
@@ -27,25 +27,25 @@
import org.apache.kafka.connect.runtime.WorkerConfig;
import org.apache.kafka.connect.storage.ConfigBackingStore;
import org.junit.Test;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.junit.jupiter.MockitoExtension;
import javax.management.MBeanServer;
import javax.management.ObjectName;
import java.lang.management.ManagementFactory;
import java.util.HashMap;
import java.util.Map;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension.class)
Review Comment:
please add `@MockitoSettings(strictness = Strictness.STRICT_STUBS)`
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java:
##
@@ -97,7 +97,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension.class)
Review Comment:
please add `@MockitoSettings(strictness = Strictness.STRICT_STUBS)`
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -141,24 +141,39 @@ private enum SourceSink {
private final SampleConnectorClientConfigOverridePolicy
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
-@Before
+@BeforeEach
public void setup() throws ExecutionException, InterruptedException {
Review Comment:
Could you please adjust this method to accept a boolean to mock?
```java
public void setup(boolean mockTransform) {
herder = mock(StandaloneHerder.class, withSettings()
.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
if (mockTransform) when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
}
```
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/errors/RetryWithToleranceOperatorTest.java:
##
@@ -80,7 +80,7 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension.class)
Review Comment:
please add `@MockitoSettings(s
[PR] KAFKA-16921: Migrate test of connect module to Junit5 (Runtime subpackage) [kafka]
frankvicky opened a new pull request, #16350: URL: https://github.com/apache/kafka/pull/16350 Migrate test of connect module to Junit5 ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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
