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 2021/06/14 17:41:42 UTC
[GitHub] [kafka] wycccccc opened a new pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for StreamsMe…
wycccccc opened a new pull request #10881:
URL: https://github.com/apache/kafka/pull/10881
Development of EasyMock and PowerMock has stagnated while Mockito continues to be actively developed. With the new Java cadence, it's a problem to depend on libraries that do bytecode generation and are not actively maintained. In addition, Mockito is also easier to use.[KAFKA-7438](https://issues.apache.org/jira/browse/KAFKA-7438)
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] wycccccc commented on pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
wycccccc commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-915451252
@ijuma Sure,I have resolved the problem.If there are other problems, I will solve them immediately.
--
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] ableegoldman commented on pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for StreamsMe…
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-884466044
cc also @cadonna who's familiar with these specific tests I think
--
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 pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for StreamsMe…
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-861474114
Thanks for the PR. Can we please update https://github.com/apache/kafka/blob/trunk/build.gradle#L374 as well?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] wycccccc commented on a change in pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
wycccccc commented on a change in pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#discussion_r704650439
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/StateManagerUtilTest.java
##########
@@ -42,77 +39,65 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
-import static org.easymock.EasyMock.createStrictControl;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.expectLastCall;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
-import static org.powermock.api.easymock.PowerMock.mockStatic;
-import static org.powermock.api.easymock.PowerMock.replayAll;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.doThrow;
-@RunWith(PowerMockRunner.class)
-@PrepareForTest(Utils.class)
public class StateManagerUtilTest {
- @Mock(type = MockType.NICE)
+ @Mock
private ProcessorStateManager stateManager;
- @Mock(type = MockType.NICE)
+ @Mock
private StateDirectory stateDirectory;
- @Mock(type = MockType.NICE)
+ @Mock
private ProcessorTopology topology;
- @Mock(type = MockType.NICE)
+ @Mock
private InternalProcessorContext processorContext;
- private IMocksControl ctrl;
private Logger logger = new LogContext("test").logger(AbstractTask.class);
private final TaskId taskId = new TaskId(0, 0);
@Before
public void setup() {
- ctrl = createStrictControl();
- topology = ctrl.createMock(ProcessorTopology.class);
- processorContext = ctrl.createMock(InternalProcessorContext.class);
+ topology = mock(ProcessorTopology.class);
+ processorContext = mock(InternalProcessorContext.class);
- stateManager = ctrl.createMock(ProcessorStateManager.class);
- stateDirectory = ctrl.createMock(StateDirectory.class);
+ stateManager = mock(ProcessorStateManager.class);
+ stateDirectory = mock(StateDirectory.class);
}
@Test
public void testRegisterStateStoreWhenTopologyEmpty() {
- expect(topology.stateStores()).andReturn(emptyList());
-
- ctrl.checkOrder(true);
- ctrl.replay();
+ when(topology.stateStores()).thenReturn(emptyList());
+ inOrder(topology);
StateManagerUtil.registerStateStores(logger,
"logPrefix:", topology, stateManager, stateDirectory, processorContext);
-
- ctrl.verify();
}
@Test
public void testRegisterStateStoreFailToLockStateDirectory() {
- expect(topology.stateStores()).andReturn(singletonList(new MockKeyValueStore("store", false)));
+ when(topology.stateStores()).thenReturn(singletonList(new MockKeyValueStore("store", false)));
+
+ when(stateManager.taskId()).thenReturn(taskId);
- expect(stateManager.taskId()).andReturn(taskId);
+ when(stateDirectory.lock(taskId)).thenReturn(false);
- expect(stateDirectory.lock(taskId)).andReturn(false);
-
- ctrl.checkOrder(true);
- ctrl.replay();
+ inOrder(topology, stateManager, stateDirectory);
Review comment:
I have gotten the method how to use inorder and the problem has been solved.
--
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] wycccccc commented on a change in pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
wycccccc commented on a change in pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#discussion_r675973748
##########
File path: streams/src/test/java/org/apache/kafka/streams/kstream/internals/graph/TableSourceNodeTest.java
##########
@@ -23,43 +23,32 @@
import org.apache.kafka.streams.kstream.internals.MaterializedInternal;
import org.apache.kafka.streams.kstream.internals.graph.TableSourceNode.TableSourceNodeBuilder;
import org.apache.kafka.streams.processor.internals.InternalTopologyBuilder;
-import org.easymock.EasyMock;
import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.powermock.api.easymock.PowerMock;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
import java.util.Properties;
-@RunWith(PowerMockRunner.class)
-@PrepareForTest({InternalTopologyBuilder.class})
+import static org.mockito.Mockito.mock;
+
public class TableSourceNodeTest {
private static final String STORE_NAME = "store-name";
private static final String TOPIC = "input-topic";
- private final InternalTopologyBuilder topologyBuilder = PowerMock.createNiceMock(InternalTopologyBuilder.class);
+ private final InternalTopologyBuilder topologyBuilder = mock(InternalTopologyBuilder.class);
@Test
public void shouldConnectStateStoreToInputTopicIfInputTopicIsUsedAsChangelog() {
final boolean shouldReuseSourceTopicForChangelog = true;
topologyBuilder.connectSourceStoreAndTopic(STORE_NAME, TOPIC);
- EasyMock.replay(topologyBuilder);
buildTableSourceNode(shouldReuseSourceTopicForChangelog);
-
- EasyMock.verify(topologyBuilder);
Review comment:
In my opinion, the `verfiy(object) `in easymock is to verify whether the stubbed method is actually called.
And this step will be automatically detected in mockito. The call form of verfiy method in mockito is `verfit(object).callmethod`, which is used for some specific detections, such as the number of times the method is called (atMostOnec, atLeastOnec) .
So unless it is a specific test, the general easymock verify is not required by mockito.
--
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 pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for StreamsMe…
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-869298381
@vvcephei @ableegoldman can one of you review this PR please? It gets us closer to disabling the JDK 15 builds.
--
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] chia7712 commented on a change in pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#discussion_r686709179
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/StateManagerUtilTest.java
##########
@@ -42,77 +39,65 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
-import static org.easymock.EasyMock.createStrictControl;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.expectLastCall;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
-import static org.powermock.api.easymock.PowerMock.mockStatic;
-import static org.powermock.api.easymock.PowerMock.replayAll;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.doThrow;
-@RunWith(PowerMockRunner.class)
-@PrepareForTest(Utils.class)
public class StateManagerUtilTest {
- @Mock(type = MockType.NICE)
+ @Mock
private ProcessorStateManager stateManager;
- @Mock(type = MockType.NICE)
+ @Mock
private StateDirectory stateDirectory;
- @Mock(type = MockType.NICE)
+ @Mock
private ProcessorTopology topology;
- @Mock(type = MockType.NICE)
+ @Mock
private InternalProcessorContext processorContext;
- private IMocksControl ctrl;
private Logger logger = new LogContext("test").logger(AbstractTask.class);
private final TaskId taskId = new TaskId(0, 0);
@Before
public void setup() {
- ctrl = createStrictControl();
- topology = ctrl.createMock(ProcessorTopology.class);
- processorContext = ctrl.createMock(InternalProcessorContext.class);
+ topology = mock(ProcessorTopology.class);
+ processorContext = mock(InternalProcessorContext.class);
- stateManager = ctrl.createMock(ProcessorStateManager.class);
- stateDirectory = ctrl.createMock(StateDirectory.class);
+ stateManager = mock(ProcessorStateManager.class);
+ stateDirectory = mock(StateDirectory.class);
}
@Test
public void testRegisterStateStoreWhenTopologyEmpty() {
- expect(topology.stateStores()).andReturn(emptyList());
-
- ctrl.checkOrder(true);
- ctrl.replay();
+ when(topology.stateStores()).thenReturn(emptyList());
+ inOrder(topology);
StateManagerUtil.registerStateStores(logger,
"logPrefix:", topology, stateManager, stateDirectory, processorContext);
-
- ctrl.verify();
}
@Test
public void testRegisterStateStoreFailToLockStateDirectory() {
- expect(topology.stateStores()).andReturn(singletonList(new MockKeyValueStore("store", false)));
+ when(topology.stateStores()).thenReturn(singletonList(new MockKeyValueStore("store", false)));
+
+ when(stateManager.taskId()).thenReturn(taskId);
- expect(stateManager.taskId()).andReturn(taskId);
+ when(stateDirectory.lock(taskId)).thenReturn(false);
- expect(stateDirectory.lock(taskId)).andReturn(false);
-
- ctrl.checkOrder(true);
- ctrl.replay();
+ inOrder(topology, stateManager, stateDirectory);
Review comment:
you have to check the execution order through the return object from `inOrder`
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/StateManagerUtilTest.java
##########
@@ -42,77 +39,65 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
-import static org.easymock.EasyMock.createStrictControl;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.expectLastCall;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
-import static org.powermock.api.easymock.PowerMock.mockStatic;
-import static org.powermock.api.easymock.PowerMock.replayAll;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.doThrow;
-@RunWith(PowerMockRunner.class)
-@PrepareForTest(Utils.class)
public class StateManagerUtilTest {
- @Mock(type = MockType.NICE)
+ @Mock
private ProcessorStateManager stateManager;
- @Mock(type = MockType.NICE)
+ @Mock
private StateDirectory stateDirectory;
- @Mock(type = MockType.NICE)
+ @Mock
private ProcessorTopology topology;
- @Mock(type = MockType.NICE)
+ @Mock
private InternalProcessorContext processorContext;
- private IMocksControl ctrl;
private Logger logger = new LogContext("test").logger(AbstractTask.class);
private final TaskId taskId = new TaskId(0, 0);
@Before
public void setup() {
- ctrl = createStrictControl();
- topology = ctrl.createMock(ProcessorTopology.class);
- processorContext = ctrl.createMock(InternalProcessorContext.class);
+ topology = mock(ProcessorTopology.class);
+ processorContext = mock(InternalProcessorContext.class);
- stateManager = ctrl.createMock(ProcessorStateManager.class);
- stateDirectory = ctrl.createMock(StateDirectory.class);
+ stateManager = mock(ProcessorStateManager.class);
+ stateDirectory = mock(StateDirectory.class);
}
@Test
public void testRegisterStateStoreWhenTopologyEmpty() {
- expect(topology.stateStores()).andReturn(emptyList());
-
- ctrl.checkOrder(true);
- ctrl.replay();
+ when(topology.stateStores()).thenReturn(emptyList());
+ inOrder(topology);
Review comment:
there is a single mock method. Does it need to check order?
--
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] wycccccc commented on pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for StreamsMe…
Posted by GitBox <gi...@apache.org>.
wycccccc commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-861635281
@ijuma Thanks for the hint，have updated.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-888743634
I think this looks good but there's a merge conflict, can you fix that?
--
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] wycccccc commented on pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
wycccccc commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-889261189
@ableegoldman Have resloved conflict, thanks for your 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 pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for Streams…
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#issuecomment-913727723
@wycccccc Are you planning to address the outstanding review comments?
--
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] ableegoldman commented on a change in pull request #10881: KAFKA-12947 Replace EasyMock and PowerMock with Mockito for StreamsMe…
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10881:
URL: https://github.com/apache/kafka/pull/10881#discussion_r674307316
##########
File path: streams/src/test/java/org/apache/kafka/streams/kstream/internals/graph/TableSourceNodeTest.java
##########
@@ -23,43 +23,32 @@
import org.apache.kafka.streams.kstream.internals.MaterializedInternal;
import org.apache.kafka.streams.kstream.internals.graph.TableSourceNode.TableSourceNodeBuilder;
import org.apache.kafka.streams.processor.internals.InternalTopologyBuilder;
-import org.easymock.EasyMock;
import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.powermock.api.easymock.PowerMock;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
import java.util.Properties;
-@RunWith(PowerMockRunner.class)
-@PrepareForTest({InternalTopologyBuilder.class})
+import static org.mockito.Mockito.mock;
+
public class TableSourceNodeTest {
private static final String STORE_NAME = "store-name";
private static final String TOPIC = "input-topic";
- private final InternalTopologyBuilder topologyBuilder = PowerMock.createNiceMock(InternalTopologyBuilder.class);
+ private final InternalTopologyBuilder topologyBuilder = mock(InternalTopologyBuilder.class);
@Test
public void shouldConnectStateStoreToInputTopicIfInputTopicIsUsedAsChangelog() {
final boolean shouldReuseSourceTopicForChangelog = true;
topologyBuilder.connectSourceStoreAndTopic(STORE_NAME, TOPIC);
- EasyMock.replay(topologyBuilder);
buildTableSourceNode(shouldReuseSourceTopicForChangelog);
-
- EasyMock.verify(topologyBuilder);
Review comment:
Admittedly I don't know much about Mockito, but it looks like we're removing the one verification step that's actually testing something in this test without replacing it. Isn't there a `verify()` for Mockito?
--
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