KAFKA-16921: Migrate test of connect module to Junit5 (Runtime direct) by gongxuanzhang · Pull Request #16351 · apache/kafka

gongxuanzhang · 2024-06-15T07:01:43Z

This PR is part of a task submodule, with the purpose of migrating from JUnit 4 to JUnit 5.
see

In this PR,AbstractWorkerSourceTaskTest test case has error.maybe other modules are involved.

# Conflicts: # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/ConnectMetricsTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/ConnectorConfigTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/ErrorHandlingTaskTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/ExactlyOnceWorkerSourceTaskTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/InternalSinkRecordTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/LoggersTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/RestartPlanTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/RestartRequestTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/SourceConnectorConfigTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/SourceTaskOffsetCommitterTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/SubmittedRecordsTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/TransformationConfigTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/TransformationStageTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerConfigTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerConfigTransformerTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerConnectorTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerMetricsGroupTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSinkTaskTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSinkTaskThreadedTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSourceTaskTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTaskTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTestUtils.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/IncrementalCooperativeAssignorTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/WorkerCoordinatorTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginScannerTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginsTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/SynchronizationTest.java # connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java

chia7712

@gongxuanzhang thanks for this patch. MethodSource is not readable, so we should take a look at both ValueSource and EnumSource first.

chia7712 · 2024-06-16T01:55:03Z

-    @Test
-    public void testRemoveMetrics() {
+    @ParameterizedTest
+    @MethodSource("parameters")


Could you please use @ValueSource(booleans = {true, false}) instead? that is more readable to me

chia7712 · 2024-06-16T01:55:12Z

-    @Test
-    public void testStartPaused() throws Exception {
+    @ParameterizedTest
+    @MethodSource("parameters")


chia7712 · 2024-06-16T01:55:16Z

-    @Test
-    public void testPause() throws Exception {
+    @ParameterizedTest
+    @MethodSource("parameters")


chia7712 · 2024-06-16T01:55:18Z

-    @Test
-    public void testFailureInPreProducerCheck() throws Exception {
+    @ParameterizedTest
+    @MethodSource("parameters")


chia7712 · 2024-06-16T01:55:21Z


-    @Test
-    public void testFailureInProducerInitialization() throws Exception {
+    @ParameterizedTest


chia7712 · 2024-06-16T01:59:33Z

-    @Test
-    public void testTransitionStartedToStarted() {
+    @ParameterizedTest
+    @MethodSource("parameters")


Could you please use @EnumSource(value = ConnectorType.class, names = {"SOURCE", "SINK"}) instead? it is more readable

chia7712 · 2024-06-16T01:59:45Z

-    @Test
-    public void testFailureInPoll() throws Exception {
+    @ParameterizedTest
+    @MethodSource("parameters")


gongxuanzhang · 2024-06-16T07:10:33Z

@chia7712 I update it

chia7712

@gongxuanzhang thanks for updated PR

chia7712 · 2024-06-17T01:31:20Z

+    private boolean enableTopicCreation;

-    @Parameterized.Parameters
    public static Collection<Boolean> parameters() {


this is unused now, right?

chia7712 · 2024-06-17T01:32:10Z

-    public static Collection<Boolean> parameters() {
-        return Arrays.asList(false, true);
+
+    public static Stream<Boolean> parameters() {


chia7712 · 2024-06-17T01:34:05Z

+    private ConnectorOffsetBackingStore offsetStore;

-    @Parameterized.Parameters
    public static Collection<ConnectorType> parameters() {


gongxuanzhang · 2024-06-17T02:12:36Z

plz take a look @chia7712

chia7712 · 2024-06-17T08:18:23Z

@gongxuanzhang please fix those failed tests

gongxuanzhang · 2024-06-17T10:28:27Z

I have fixed all the test cases that failed due to the changes.
@chia7712

chia7712

@gongxuanzhang thanks for updated PR

chia7712 · 2024-06-17T14:59:15Z

            }
        }).when(sinkTask).preCommit(anyMap());
-
+        //  if the only command has error, consumer don't commit, so we don't need to mock commitAsync.


Could we have two methods?

private void expectPreCommit(ExpectOffsetCommitCommand... commands) { doAnswer(new Answer<Object>() { int index = 0; @Override public Object answer(InvocationOnMock invocation) { ExpectOffsetCommitCommand commitCommand = commands[index++]; // All assigned partitions will have offsets committed, but we've only processed messages/updated offsets for one final Map<TopicPartition, OffsetAndMetadata> offsetsToCommit = offsetsToCommitFn.apply(commitCommand.expectedMessages); if (commitCommand.error != null) { throw commitCommand.error; } else { return offsetsToCommit; } } }).when(sinkTask).preCommit(anyMap()); } private void expectOffsetCommit(ExpectOffsetCommitCommand... commands) { expectPreCommit(commands); ... }

with that changes, each test case can call specific "expect"

chia7712 · 2024-06-17T15:00:33Z

-    public static Collection<Boolean> parameters() {
-        return Arrays.asList(false, true);
-    }
+    private boolean enableTopicCreation;


we should remove this, and each test case should call expectTopicCreation when enableTopicCreation=true

gongxuanzhang · 2024-06-18T01:45:45Z

I update it. plz take a look @chia7712

chia7712

LGTM

…irect) (apache#16351) Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

gongxuanzhang and others added 9 commits June 14, 2024 17:52

first upload,remain RunWith

9d59fba

Merge branch 'apache:trunk' into junit5-runtime

8a9b18a

Merge branch 'apache:trunk' into junit5-runtime

d8d5f00

upgrade all junit 4 to junit5 in connect:runtime

b923a28

upgrade all junit 4 to junit5 in connect:runtime

ffdc645

revert other dir

f85e837

update mokito strictness

6a08838

revert mockito strictness

ace892c

chia7712 reviewed Jun 16, 2024

View reviewed changes

gongxuanzhang and others added 4 commits June 16, 2024 13:06

update @MethodSource("parameters") to ValueSource and EnumSource

d44c989

Merge branch 'apache:trunk' into junit5-runtime

5cbae4a

update import

45256b8

Merge remote-tracking branch 'origin/junit5-runtime' into junit5-runtime

36142c2

Merge branch 'apache:trunk' into junit5-runtime

3b240c6

chia7712 reviewed Jun 17, 2024

View reviewed changes

gongxuanzhang added 2 commits June 17, 2024 10:06

delete redundance

d082572

delete redundance

9882ebc

gongxuanzhang and others added 6 commits June 17, 2024 11:33

revert standalone

d507126

revert rest

4dc8bc7

revert isolation

b1c3253

revert distributed

ff17c53

Update DistributedHerderTest.java

a06aac9

Update StandaloneHerderTest.java

1b3f944

gongxuanzhang added 2 commits June 17, 2024 18:27

fix test case

c452750

Merge remote-tracking branch 'origin/junit5-runtime' into junit5-runtime

697b6d0

Merge branch 'apache:trunk' into junit5-runtime

3497fc2

chia7712 reviewed Jun 17, 2024

View reviewed changes

refactor code

c2609bd

chia7712 approved these changes Jun 18, 2024

View reviewed changes

chia7712 merged commit 3a3f9ce into apache:trunk Jun 18, 2024

TaiJuWu pushed a commit to TaiJuWu/kafka that referenced this pull request Jul 4, 2024

KAFKA-16921 [4/N] Migrate test of connect module to Junit5 (Runtime d…

d3687c2

…irect) (apache#16351) Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

Conversation

gongxuanzhang commented Jun 15, 2024

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gongxuanzhang commented Jun 16, 2024

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gongxuanzhang commented Jun 17, 2024

Uh oh!

chia7712 commented Jun 17, 2024

Uh oh!

gongxuanzhang commented Jun 17, 2024

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gongxuanzhang commented Jun 18, 2024

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants