yashmayya opened a new pull request, #14102: URL: https://github.com/apache/kafka/pull/14102
- https://issues.apache.org/jira/browse/KAFKA-13187 - Original / abandoned PR for this issue is - https://github.com/apache/kafka/pull/11792. - https://github.com/mockito/mockito/issues/2601 is resolved in the current version of Mockito that we're using (`4.11.0`) so the workaround hack is no longer needed. - The current / new patch takes a slightly different approach from the original one. Using a combination of Mockito's [strict stubs](https://www.javadoc.io/doc/org.mockito/mockito-core/4.11.0/org/mockito/quality/Strictness.html#STRICT_STUBS) functionality and [verifyNoMoreInteractions](https://javadoc.io/static/org.mockito/mockito-core/4.11.0/org/mockito/Mockito.html#verifyNoMoreInteractions(java.lang.Object...)), we get some nice properties - - Any unused stubbings will lead to test failures - Tests fail early when stubbed methods are invoked with different arguments - Stubbed invocations are automatically verified and don't need explicit verification. - The above properties allow us to make the test code significantly leaner and cleaner (as can be seen from the + / - SLOC in this diff). - One big caveat with this approach is that the number of invocations for stubbed methods isn't automatically verified for us since `Mockito` stubbings are applicable any number of times by default (unlike in `EasyMock` where the default is a single invocation being stubbed). - Hence, the older `EasyMock` way of stacking up expectations and then verifying them automatically took care of verifying the number of invocations for stubbed methods (assuming `anyTimes()` wasn't used in the expect calls). - While for a lot of cases this really isn't required (for instance, verifying the number of calls to `WorkerGroupMember::wakeup` in large tests that involve multiple calls to `DistributedHerder::tick`), there are some cases where it is crucial to the test's coverage and this patch attempts to retain that coverage wherever it seemed important by using the [times() verification mode](https://javadoc.io/doc/org.mockito/mockito-core/4.11.0/org/mockito/verification/VerificationMode.html) explicitly. - There was no refactoring required for the `DistributedHerder` class itself apart from bumping up the visibility of the `herderExecutor` for this specific test - https://github.com/apache/kafka/blob/ff390ab60a57100cc829be243ce525ac31523000/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java#L3906 ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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
