mdedetrich commented on code in PR #12524:
URL: https://github.com/apache/kafka/pull/12524#discussion_r975412340
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java:
##########
@@ -211,32 +205,22 @@ public void shouldThrowIfCommittingOnIllegalState() {
@Test
public void shouldAlwaysCheckpointStateIfEnforced() {
- stateManager.flush();
Review Comment:
Done.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java:
##########
@@ -177,11 +179,6 @@ public void
shouldThrowLockExceptionIfFailedToLockStateDirectory() throws IOExce
@Test
public void shouldTransitToRunningAfterInitialization() {
-
EasyMock.expect(stateManager.changelogOffsets()).andStubReturn(Collections.emptyMap());
- stateManager.registerStateStores(EasyMock.anyObject(),
EasyMock.anyObject());
Review Comment:
Yes this part of `EasyMock` I understand, the problem is that a lot of the
tests had stubbings that were unnecessary which was being picked up being
`MockitoJUnitRunner.StrictStubs.class`. Ontop of this
`Easymock.verify(stateManager)` has no equivalent in mockito since its
considered bad practice to just verify any interaction on a class (Mockito
forces you to specify which method is being called, not just
`Easymock.verify(someClass)`. Since the `Easymock.verify(someClass)` was
typically placed at the bottom of the test methods so it was hard to tell what
exactly was expected to mock and in some cases it was pointless.
Evidently I did some mistakes so I have put back
`stateManager.registerStateStores(any(), any());` however note that
`stateManager.registerStateStores()` is being called twice so I used
`atLeastOnce`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
