You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/09/09 11:16:03 UTC

[GitHub] [kafka] yashmayya commented on a diff in pull request #12615: KAFKA-14132: Migrate some Connect tests from EasyMock/PowerMock to Mockito

yashmayya commented on code in PR #12615:
URL: https://github.com/apache/kafka/pull/12615#discussion_r966947727


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTaskTest.java:
##########
@@ -162,71 +121,53 @@ public void stopBeforeStarting() {
         // now run should not do anything
         workerTask.run();
 
-        verify(workerTask);
+        verify(workerTask).initialize(eq(TASK_CONFIG));
+        verify(workerTask).close();
+        verify(workerTask, never()).initializeAndStart();
+        verify(workerTask, never()).execute();
     }
 
     @Test
     public void cancelBeforeStopping() throws Exception {
         ConnectorTaskId taskId = new ConnectorTaskId("foo", 0);
 
-        WorkerTask workerTask = partialMockBuilder(WorkerTask.class)
-                .withConstructor(
-                        ConnectorTaskId.class,
-                        TaskStatus.Listener.class,
-                        TargetState.class,
-                        ClassLoader.class,
-                        ConnectMetrics.class,
-                        RetryWithToleranceOperator.class,
-                        Time.class,
-                        StatusBackingStore.class
-                )
-                .withArgs(taskId, statusListener, TargetState.STARTED, loader, metrics,
+        WorkerTask workerTask = mock(WorkerTask.class, withSettings()
+                .useConstructor(taskId, statusListener, TargetState.STARTED, loader, metrics,
                         retryWithToleranceOperator, Time.SYSTEM, statusBackingStore)
-                .addMockedMethod("initialize")
-                .addMockedMethod("initializeAndStart")
-                .addMockedMethod("execute")
-                .addMockedMethod("close")
-                .createStrictMock();
+                .defaultAnswer(CALLS_REAL_METHODS));
 
         final CountDownLatch stopped = new CountDownLatch(1);
-        final Thread thread = new Thread(() -> {
-            try {
-                stopped.await();
-            } catch (Exception e) {
-            }
-        });
 
-        workerTask.initialize(TASK_CONFIG);
-        EasyMock.expectLastCall();
+        doNothing().when(workerTask).initialize(any(TaskConfig.class));
 
-        workerTask.initializeAndStart();
-        EasyMock.expectLastCall();
-
-        workerTask.execute();
-        expectLastCall().andAnswer(() -> {
-            thread.start();
+        // Trigger task shutdown immediately after start. The task will block in it's execute() method
+        // until the stopped latch is counted down (i.e. it doesn't actually stop after stop is triggered).
+        doAnswer(i -> {
+            workerTask.stop();
             return null;
-        });
-
-        statusListener.onStartup(taskId);
-        expectLastCall();
-
-        workerTask.close();
-        expectLastCall();
-
-        // there should be no call to onShutdown()

Review Comment:
   This test implementation was buggy, and `statusListener.onShutdown` was actually being called (i.e. the task wasn't being cancelled before it fully stopped) - the only reason the test passed earlier was because interactions with `statusListener` weren't being verified.



##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/errors/ErrorReporterTest.java:
##########
@@ -139,42 +136,21 @@ public void testReportDLQTwice() {
 
         ProcessingContext context = processingContext();
 
-        EasyMock.expect(producer.send(EasyMock.anyObject(), EasyMock.anyObject())).andReturn(metadata).times(2);
-        replay(producer);
+        when(producer.send(any(), any())).thenReturn(metadata);
 
         deadLetterQueueReporter.report(context);
         deadLetterQueueReporter.report(context);
 
-        PowerMock.verifyAll();
-    }
-
-    @Test
-    public void testDLQReportAndReturnFuture() {

Review Comment:
   This test was identical to `testDLQConfigWithValidTopicName`



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org