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

[GitBox]

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


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTaskTest.java:
##########
@@ -78,82 +78,41 @@ public void tearDown() {
     public void standardStartup() {
         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()

Review Comment:
   Mocking the class we're testing is a bit of an anti-pattern.
   
   How about we introduce a `TestWorkerTask` class that defaults to no-ops for 
all the abstract `WorkerTask` methods, and can be overridden on a per-test-case 
basis to define other behavior? Thinking something like this, using 
`stopBeforeStarting` as an example:
   
   ```java
   @Test
   public void stopBeforeStarting() {
       ConnectorTaskId taskId = new ConnectorTaskId("foo", 0);
   
       WorkerTask workerTask = new TestWorkerTask(taskId, statusListener, 
TargetState.STARTED, loader, metrics,
                       retryWithToleranceOperator, Time.SYSTEM, 
statusBackingStore) {
           @Override
           protected void initializeAndStart() {
               fail("Should never be invoked");
           }
   
           @Override
           protected void execute() {
               fail("Should never be invoked");
           }
       };
   
       // Rest of test logic goes here
   }
   
   // ... Rest of test cases go here
   
   // At bottom of file
   private class TestWorkerTask extends WorkerTask {
       public TestWorkerTask(ConnectorTaskId id, TaskStatus.Listener 
statusListener, TargetState initialState, ClassLoader loader, ConnectMetrics 
connectMetrics, RetryWithToleranceOperator retryWithToleranceOperator, Time 
time, StatusBackingStore statusBackingStore) {
           super(id, statusListener, initialState, loader, connectMetrics, 
retryWithToleranceOperator, time, statusBackingStore);
       }
   
       @Override
       public void initialize(TaskConfig taskConfig) {
       }
   
       @Override
       protected void initializeAndStart() {
       }
   
       @Override
       protected void execute() {
       }
   
       @Override
       protected void close() {
       }
   }
   ```



-- 
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