cadonna commented on code in PR #13932:
URL: https://github.com/apache/kafka/pull/13932#discussion_r1409103748


##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java:
##########
@@ -3494,32 +3323,27 @@ private StreamThread setUpThread(final Properties 
streamsConfigProps) {
     }
 
     private TaskManager mockTaskManager(final Task runningTask) {
-        final TaskManager taskManager = 
EasyMock.createNiceMock(TaskManager.class);
+        final TaskManager taskManager = mock(TaskManager.class);
         final TaskId taskId = new TaskId(0, 0);
 
-        expect(runningTask.state()).andStubReturn(Task.State.RUNNING);
-        expect(runningTask.id()).andStubReturn(taskId);
-        
expect(taskManager.allOwnedTasks()).andStubReturn(Collections.singletonMap(taskId,
 runningTask));
-        
expect(taskManager.commit(Collections.singleton(runningTask))).andStubReturn(1);
+        when(runningTask.state()).thenReturn(Task.State.RUNNING);
+        
when(taskManager.allOwnedTasks()).thenReturn(Collections.singletonMap(taskId, 
runningTask));
         return taskManager;
     }
 
     private TaskManager mockTaskManagerPurge(final int numberOfPurges) {
         final Task runningTask = mock(Task.class);
         final TaskManager taskManager = mockTaskManager(runningTask);
 
-        taskManager.maybePurgeCommittedRecords();
-        EasyMock.expectLastCall().times(numberOfPurges);

Review Comment:
   Adding
   ```java
   doNothing().when(taskManager).maybePurgeCommittedRecords();
   ```
   to this method was not what I meant.
   
   You should add 
   ```java
   verify(taskManager, times(2)).maybePurgeCommittedRecords();
   ```
   At the end of `shouldPurgeAfterPurgeInterval()` after line 1093.
   
   Probably you should inline the remaining code of this method where it is 
called. This method is called in two places: 
   - line 555: There you can remove `taskManager.maybePurgeCommittedRecords();` 
on line 556 because it does not make sense to call a method on a mock within a 
test method directly. Probably, it was kind of a hack to reuse the setup in 
this method.
   - line 1081: That is the method in which you should add the verification I 
mentioned before.



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

Reply via email to