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
<[email protected]> 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 <[email protected]> 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
>
>
