cadonna commented on code in PR #12524:
URL: https://github.com/apache/kafka/pull/12524#discussion_r976346195
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java:
##########
@@ -177,10 +181,7 @@ public void
shouldThrowLockExceptionIfFailedToLockStateDirectory() throws IOExce
@Test
public void shouldTransitToRunningAfterInitialization() {
-
EasyMock.expect(stateManager.changelogOffsets()).andStubReturn(Collections.emptyMap());
- stateManager.registerStateStores(EasyMock.anyObject(),
EasyMock.anyObject());
-
EasyMock.expect(stateManager.changelogPartitions()).andReturn(Collections.emptySet()).anyTimes();
- EasyMock.replay(stateManager);
+ stateManager.registerStateStores(any(), any());
Review Comment:
I meant that you should remove the call on the mock, because mocks should
not be directly called in the test code. Mocks should be called within the call
under test. In this case this is `task.initializeIfNeeded()`. Here it is really
important that `stateManager.registerStateStores()` is only called once, since
a second call to `task.initializeIfNeeded()` should not register the state
stores again. The test should be like this:
```
public void shouldTransitToRunningAfterInitialization() {
task = createStandbyTask();
assertEquals(CREATED, task.state());
task.initializeIfNeeded();
assertEquals(RUNNING, task.state());
// initialize should be idempotent
task.initializeIfNeeded();
assertEquals(RUNNING, task.state());
verify(stateManager).registerStateStores(any(), any());
}
```
You are right to say that the stubbings were unnecessary here since they
returned what is the default in Mockito.
--
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
