Hello! Following Divij's example I wanted to give an update on the progress being made. With a combined effort from Yash Mayya, Matthew de Detrich and myself we are 3/4 through providing pull requests for moving the remaining EasyMock tests to Mockito (https://issues.apache.org/jira/browse/KAFKA-14133). Across those and other pull requests we have received insightful feedback from Bruno Cadonna, Chris Egerton, Dalibor Plavcic and Ismael Juma on how things can be improved. Thank you very much! Current pull requests we are trying to get to a resolution from oldest to newest are: * https://github.com/apache/kafka/pull/12409* https://github.com/apache/kafka/pull/12418* https://github.com/apache/kafka/pull/12459* https://github.com/apache/kafka/pull/12465 * https://github.com/apache/kafka/pull/12473 * https://github.com/apache/kafka/pull/12492* https://github.com/apache/kafka/pull/12505* https://github.com/apache/kafka/pull/12509 Best,Christo
On Thursday, 4 August 2022, 18:27:17 BST, Divij Vaidya <divijvaidy...@gmail.com> wrote: Hi everyone To provide you with quick updates on the progress. Open PRs (pending review): 1. Streams - https://github.com/apache/kafka/pull/12449 2. Streams - https://github.com/apache/kafka/pull/12465 3. Streams - https://github.com/apache/kafka/pull/12459 4. Connect - https://github.com/apache/kafka/pull/12484 5. Connect - https://github.com/apache/kafka/pull/12473 6. Connect - https://github.com/apache/kafka/pull/12409 7. Connect - https://github.com/apache/kafka/pull/12472 Open tasks (pending an owners): 1. https://issues.apache.org/jira/browse/KAFKA-14132 (need owners for separate individual tests) 2. https://issues.apache.org/jira/browse/KAFKA-14133 General guidance to reduce code review churn when working on these test conversions: 1. Please use @RunWith(MockitoJUnitRunner.StrictStubs.class) since it provides many benefits. 2. Please do not perform JUnit 5 migration in the same PR as Mockito conversion to keep the changes few and easy to review. We will follow up with a blanket JUnit5 conversion (similar to this <https://github.com/apache/kafka/pull/12285>) when Mockito migration is complete. 3. Please use @Mock annotation to mock (Chris Egerton has added this comment on various PRs, hence calling it out) 4. Note that @RunWith(MockitoJUnitRunner.StrictStubs.class) verifies the invocation of declared stubs automatically. If the stubs are not invoked, the test throws a UnnecessaryStubbingException. Note that this doesn't seem to work for `mockStatic` and I would suggest to explicitly verify stub invocations over there. 5. As a reference, you can use the merged PR from Chris Egerton here: https://github.com/apache/kafka/pull/12409 6. Add a verification step in the description that the test has successfully run with the command `./gradlew connect:runtime:unitTest` (or equivalent for the module you are changing the test for). Additionally, you can add the code coverage report using `./gradlew streams:reportCoverage -PenableTestCoverage=true -Dorg.gradle.parallel=false` to verify that no test assertion has been accidentally removed during the change. *Chris*, would you like to add anything else to the general guidance above which would help reduce the code review churn? -- Divij Vaidya On Mon, Aug 1, 2022 at 6:49 PM Divij Vaidya <divijvaidy...@gmail.com> wrote: > Hi folks > > We have been trying to replace EasyMock/Powermock with Mockito > <https://issues.apache.org/jira/browse/KAFKA-7438> for quite a while. > This adds complications for migrating to JDK 17 & Junit5. Significant > contributions have been made by various folks towards this goal and the > finish line is almost in sight. > > Let's join forces this week and get the task done! > > I and Christo(cc'ed) will be spending time converting the straggler tests > during this week. > > At this stage, we are missing a shepherd to help us wrap up this task. *Could > we please solicit some code review bandwidth from a committer for this week > to help us reach the finish line?* > > Current pending PR requests: > 1. KAFKA-13036: Replace EasyMock and PowerMock with Mockito for > RocksDBMetricsRecorderTest by divijvaidya · Pull Request #12459 · apache/kafka > 2. KAFKA-12950: Replace EasyMock and PowerMock with Mockito for > KafkaStreamsTest by divijvaidya · Pull Request #12465 · apache/kafka > 3. KAFKA-13414: Replace PowerMock/EasyMock with Mockito in > connect.storage.KafkaOffsetBackingStoreTest by clolov · Pull Request #12418 · > apache/kafka > > Regards, > Divij Vaidya > >