clolov commented on code in PR #14410:
URL: https://github.com/apache/kafka/pull/14410#discussion_r1331578412
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsProducerTest.java:
##########
@@ -53,21 +55,23 @@
import static org.apache.kafka.common.utils.Utils.mkEntry;
import static org.apache.kafka.common.utils.Utils.mkMap;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.expectLastCall;
-import static org.easymock.EasyMock.mock;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reset;
-import static org.easymock.EasyMock.verify;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@SuppressWarnings("unchecked")
Review Comment:
Can you scope this suppression to just the lines/test it is suppressing?
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsProducerTest.java:
##########
@@ -244,47 +247,41 @@ public void shouldCreateProducer() {
@Test
public void shouldForwardCallToPartitionsFor() {
final List<PartitionInfo> expectedPartitionInfo =
Collections.emptyList();
-
expect(mockedProducer.partitionsFor("topic")).andReturn(expectedPartitionInfo);
- replay(mockedProducer);
+
when(mockedProducer.partitionsFor(topic)).thenReturn(expectedPartitionInfo);
final List<PartitionInfo> partitionInfo =
streamsProducerWithMock.partitionsFor(topic);
assertThat(partitionInfo, sameInstance(expectedPartitionInfo));
- verify(mockedProducer);
+ verify(mockedProducer).partitionsFor(topic);
Review Comment:
This is not necessary, the StrictStubs make certain to check all `when`s you
have used have indeed been invoked.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsProducerTest.java:
##########
@@ -244,47 +247,41 @@ public void shouldCreateProducer() {
@Test
public void shouldForwardCallToPartitionsFor() {
final List<PartitionInfo> expectedPartitionInfo =
Collections.emptyList();
-
expect(mockedProducer.partitionsFor("topic")).andReturn(expectedPartitionInfo);
- replay(mockedProducer);
+
when(mockedProducer.partitionsFor(topic)).thenReturn(expectedPartitionInfo);
final List<PartitionInfo> partitionInfo =
streamsProducerWithMock.partitionsFor(topic);
assertThat(partitionInfo, sameInstance(expectedPartitionInfo));
- verify(mockedProducer);
+ verify(mockedProducer).partitionsFor(topic);
}
@Test
public void shouldForwardCallToFlush() {
mockedProducer.flush();
Review Comment:
```suggestion
doNothing().when(mockedProducer).flush();
```
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsProducerTest.java:
##########
@@ -398,6 +395,8 @@ public void shouldNotSetTransactionIdIfEosDisabled() {
logContext,
mockTime
);
+
+
assertFalse(producerConfig.containsKey(ProducerConfig.TRANSACTIONAL_ID_CONFIG));
Review Comment:
For my understanding, how did you come with this assertion?
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/TopologyMetadataTest.java:
##########
@@ -21,7 +21,8 @@
import org.junit.Assert;
import org.junit.Test;
-import static org.easymock.EasyMock.mock;
+import static org.mockito.Mockito.mock;
+
public class TopologyMetadataTest {
Review Comment:
```suggestion
@RunWith(MockitoJUnitRunner.StrictStubs.class)
public class TopologyMetadataTest {
```
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsProducerTest.java:
##########
@@ -244,47 +247,41 @@ public void shouldCreateProducer() {
@Test
public void shouldForwardCallToPartitionsFor() {
final List<PartitionInfo> expectedPartitionInfo =
Collections.emptyList();
-
expect(mockedProducer.partitionsFor("topic")).andReturn(expectedPartitionInfo);
- replay(mockedProducer);
+
when(mockedProducer.partitionsFor(topic)).thenReturn(expectedPartitionInfo);
final List<PartitionInfo> partitionInfo =
streamsProducerWithMock.partitionsFor(topic);
assertThat(partitionInfo, sameInstance(expectedPartitionInfo));
- verify(mockedProducer);
+ verify(mockedProducer).partitionsFor(topic);
}
@Test
public void shouldForwardCallToFlush() {
mockedProducer.flush();
- expectLastCall();
- replay(mockedProducer);
streamsProducerWithMock.flush();
- verify(mockedProducer);
+ verify(mockedProducer, times(2)).flush();
Review Comment:
This is not necessary as detailed before, I also think that you will find
one of those invocations is because of line 260
--
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
