You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/02/19 23:26:07 UTC
[GitHub] [kafka] jeff303 opened a new pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
jeff303 opened a new pull request #11792:
URL: https://github.com/apache/kafka/pull/11792
Adding mockitoJunitJupiter test dependency to :connect:runtime project
Converting class level mocks from EasyMock -> Mockito
Changing first test (testJoinAssignment) to use Mockito constructs, along with its assertion helper methods
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jeff303 commented on pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
Posted by GitBox <gi...@apache.org>.
jeff303 commented on pull request #11792:
URL: https://github.com/apache/kafka/pull/11792#issuecomment-1072841151
Made a bit more progress on this. Converting `testRebalance` took me a very long time (due to the different paradigm that Mockito uses versus Powermock). The lack of a Mockito equivalent of `PowerMock.verifyAll()` takes a while to work through (have to manually figure out the verifications). The next test - `testIncrementalCooperativeRebalanceForNewMember` - went much faster at least.
Also, had to put in a hacky workaround for mockito/mockito#2601 for now.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jeff303 commented on pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
Posted by GitBox <gi...@apache.org>.
jeff303 commented on pull request #11792:
URL: https://github.com/apache/kafka/pull/11792#issuecomment-1078447241
Something I've learned, that I feel is not [adequately](https://github.com/mockito/mockito/wiki/Mockito-vs-EasyMock) covered by this wiki:
EasyMock expectations seem to stack up, as you build them. Or perhaps more like a FIFO queue. Expectations are queued up, then as invocations happen, the front of the queue is removed to satisfy the invocation, and the next mock is moved to the front.
On the other hand, Mockito only seems to have a **single** current mock behavior for a particular set of argument matchers. This means that new expectations can't be set until after the actual code exercises the first one (ex: the calls to `expectRebalance` in our tests). Because EasyMock is broken into phases, it allows for the "stacking" of those expectations in one phase, followed by execution, followed by verification. But with Mockito, it's necessary to follow a more staggered approach (set expectation, exercise code, set new expectation, exercise again, verify at end).
Just an observation for anyone else (or myself) in the future that is hopefully useful.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jeff303 commented on a change in pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
Posted by GitBox <gi...@apache.org>.
jeff303 commented on a change in pull request #11792:
URL: https://github.com/apache/kafka/pull/11792#discussion_r819904687
##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java
##########
@@ -917,6 +955,7 @@ public void testDestroyConnector() throws Exception {
PowerMock.verifyAll();
}
+ @Disabled("Disabled until EasyMock->Mockito conversion is done")
Review comment:
This is only temporary, as a way to keep all the source code in place while iterating on the file (since it's a large file). I will be removing the annotation one-by-one as I fix each case. Just wanted to validate the basic approach before going to all the trouble.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jeff303 edited a comment on pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
Posted by GitBox <gi...@apache.org>.
jeff303 edited a comment on pull request #11792:
URL: https://github.com/apache/kafka/pull/11792#issuecomment-1078447241
Something I've learned, that I feel is not adequately covered by [this wiki](https://github.com/mockito/mockito/wiki/Mockito-vs-EasyMock):
EasyMock expectations seem to stack up, as you build them. Or perhaps more like a FIFO queue. Expectations are queued up, then as invocations happen, the front of the queue is removed to satisfy the invocation, and the next mock is moved to the front.
On the other hand, Mockito only seems to have a **single** current mock behavior for a particular set of argument matchers. This means that new expectations can't be set until after the actual code exercises the first one (ex: the calls to `expectRebalance` in our tests). Because EasyMock is broken into phases, it allows for the "stacking" of those expectations in one phase, followed by execution, followed by verification. But with Mockito, it's necessary to follow a more staggered approach (set expectation, exercise code, set new expectation, exercise again, verify at end).
Just an observation for anyone else (or myself) in the future that is hopefully useful.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
Posted by GitBox <gi...@apache.org>.
mimaison commented on pull request #11792:
URL: https://github.com/apache/kafka/pull/11792#issuecomment-1059352526
Thanks for the PR @jeff303! Let me know when it is ready for review.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #11792: Replace EasyMock/PowerMock with Mockito in DistributedHerderTest
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #11792:
URL: https://github.com/apache/kafka/pull/11792#discussion_r819568938
##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java
##########
@@ -917,6 +955,7 @@ public void testDestroyConnector() throws Exception {
PowerMock.verifyAll();
}
+ @Disabled("Disabled until EasyMock->Mockito conversion is done")
Review comment:
We cannot disable tests like this.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org