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/08/08 11:58:15 UTC
[GitHub] [kafka] clolov opened a new pull request, #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
clolov opened a new pull request, #12492:
URL: https://github.com/apache/kafka/pull/12492
Batch 1 of the tests detailed in https://issues.apache.org/jira/browse/KAFKA-14133 which use EasyMock and need to be moved to 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] cadonna commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946894212
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new KafkaException());
assertThrows(StreamsException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldRethrowTimeoutException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new TimeoutException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new TimeoutException());
assertThrows(TimeoutException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
assertTrue(fetchCommittedOffsets(emptySet(), consumer).isEmpty());
}
@Test
public void fetchEndOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
+ final Admin adminClient = mock(AdminClient.class);
assertTrue(fetchEndOffsets(emptySet(), adminClient).isEmpty());
}
@Test
public void fetchEndOffsetsShouldRethrowRuntimeExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new RuntimeException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new RuntimeException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
Review Comment:
I did not write the original test, but from the test it seems that verifying the admin client is important since only that mock is verified. I can imagine the original author wanted to make sure that the StreamsException is caused by the call to the admin client. I think in this case it might not be needed, since AFAIK we would run into a NPE if `adminClient.listOffsets` is not called.
--
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] clolov commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1215577425
Thank you @cadonna and @dplavcic for your comments. I will aim to address them as soon as possible.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946817091
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
That's fair and very well might be the reason.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r947594751
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java:
##########
@@ -148,34 +155,27 @@ private void createMockTaskManager(final Set<TaskId> activeTasks) {
}
private void createMockTaskManager(final Map<TaskId, Long> taskOffsetSums) {
- taskManager = EasyMock.createNiceMock(TaskManager.class);
- expect(taskManager.topologyMetadata()).andStubReturn(topologyMetadata);
- expect(taskManager.getTaskOffsetSums()).andReturn(taskOffsetSums).anyTimes();
Review Comment:
Yeah, after looking a bit more into this it just seems that this code path is not exercised. I deleted it from the original EasyMock tests and the tests still passed, so I think it was not exercised in the original test either. I put a breakpoint in the beginning of the test and it was never reached.
--
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] cadonna merged pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna merged PR #12492:
URL: https://github.com/apache/kafka/pull/12492
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946489287
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/TransformerSupplierAdapterTest.java:
##########
@@ -24,117 +24,113 @@
import org.apache.kafka.streams.kstream.TransformerSupplier;
import org.apache.kafka.streams.processor.ProcessorContext;
import org.apache.kafka.streams.state.StoreBuilder;
-import org.easymock.EasyMock;
-import org.easymock.EasyMockSupport;
-import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.IsSame.sameInstance;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
-public class TransformerSupplierAdapterTest extends EasyMockSupport {
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
+public class TransformerSupplierAdapterTest {
+ @Mock
private ProcessorContext context;
+ @Mock
private Transformer<String, String, KeyValue<Integer, Integer>> transformer;
+ @Mock
private TransformerSupplier<String, String, KeyValue<Integer, Integer>> transformerSupplier;
+ @Mock
private Set<StoreBuilder<?>> stores;
final String key = "Hello";
final String value = "World";
- @Before
- public void before() {
- context = mock(ProcessorContext.class);
- transformer = mock(Transformer.class);
- transformerSupplier = mock(TransformerSupplier.class);
- stores = mock(Set.class);
- }
-
@Test
public void shouldCallInitOfAdapteeTransformer() {
- EasyMock.expect(transformerSupplier.get()).andReturn(transformer);
- transformer.init(context);
- replayAll();
+ when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
- verifyAll();
+ verify(transformerSupplier).get();
+ verify(transformer).init(any(ProcessorContext.class));
Review Comment:
It is possible and I have change it in upcoming commits.
--
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] clolov commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1237211750
@cadonna a polite bump 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] clolov commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1208033623
Hello @cadonna and @ijuma! Tagging you for visibility.
--
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] dplavcic commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
dplavcic commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r944765384
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
[optional]
I see that sometimes we explicitly verify stubbed actions and I'm wondering if there is a rule/guideline on when to do that (`stub` + `verify`) 🤔
I'm asking because according to the [Mockito docs](https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#stubbing), if we stub something while using `Strict Stubbing mode`, we get verify part out-of-the-box.
<img width="597" alt="image" src="https://user-images.githubusercontent.com/9079844/184425984-5a23e722-83d0-4104-9428-dde40ba4fd67.png">
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946490115
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/TransformerSupplierAdapterTest.java:
##########
@@ -145,10 +141,12 @@ public void shouldAlwaysGetNewAdapterTransformer() {
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adapterTransformer3 = adapter.get();
adapterTransformer3.init(context);
- verifyAll();
assertThat(adapterTransformer1, not(sameInstance(adapterTransformer2)));
assertThat(adapterTransformer2, not(sameInstance(adapterTransformer3)));
assertThat(adapterTransformer3, not(sameInstance(adapterTransformer1)));
+ verify(transformer1).init(any(ProcessorContext.class));
+ verify(transformer2).init(any(ProcessorContext.class));
+ verify(transformer3).init(any(ProcessorContext.class));
Review Comment:
Changed in upcoming commits.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946948581
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##########
@@ -47,19 +56,14 @@ public class KStreamFlatTransformTest extends EasyMockSupport {
public void setUp() {
inputKey = 1;
inputValue = 10;
- transformer = mock(Transformer.class);
- context = strictMock(InternalProcessorContext.class);
Review Comment:
Addressed in subsequent commits.
--
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] cadonna commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946708908
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
@clolov I think we do not need to verify all stubs. In the case you pointed out above I think that verifying `verify(transformer).init(context)` is enough since the transformer is returned by the stub and if it were not called a would get a `NullpointerException`.
The following test would be fine with me:
```
@Test
public void shouldCallInitOfAdapteeTransformer() {
when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
verify(transformer).init(context);
}
```
However, there might be cases where it makes sense to verify the call of the stub.
I also think that we do not need to use `verifyNoMoreInteractions(transformer);` each and every time. We have a similar behavior with EasyMock and that leads to tests that are harder to maintain because even the slightest change in production code might lead to changes in the tests. We suffered a bit because of that in the past. Admittedly, without checking additional interactions you lose a bit of control in the test. I think it is a trade-off.
What I do not understand is why the following test does not fail when run with `@RunWith(MockitoJUnitRunner.StrictStubs.class)`:
```
@Test
public void shouldCallInitOfAdapteeTransformer() {
final Record record = mock(Record.class);
when(transformerSupplier.get()).thenReturn(transformer);
when(record.hasKey()).thenReturn(true);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
verify(transformer).init(context);
}
```
`record.hasKey()` is never called and so the corresponding stubbing is unnecessary. According to Mockito's doc (https://javadoc.io/static/org.mockito/mockito-core/4.6.1/org/mockito/quality/Strictness.html#STRICT_STUBS):
> Cleaner tests without unnecessary stubbings: the test fails when unused stubs are present (see UnnecessaryStubbingException).
Am I missing something?
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946763519
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java:
##########
@@ -148,34 +155,27 @@ private void createMockTaskManager(final Set<TaskId> activeTasks) {
}
private void createMockTaskManager(final Map<TaskId, Long> taskOffsetSums) {
- taskManager = EasyMock.createNiceMock(TaskManager.class);
- expect(taskManager.topologyMetadata()).andStubReturn(topologyMetadata);
- expect(taskManager.getTaskOffsetSums()).andReturn(taskOffsetSums).anyTimes();
Review Comment:
Mockito said that the stubbing was unnecessary. However, now that you have asked I will look further into the test.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946937305
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new KafkaException());
assertThrows(StreamsException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldRethrowTimeoutException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new TimeoutException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new TimeoutException());
assertThrows(TimeoutException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
assertTrue(fetchCommittedOffsets(emptySet(), consumer).isEmpty());
}
@Test
public void fetchEndOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
+ final Admin adminClient = mock(AdminClient.class);
assertTrue(fetchEndOffsets(emptySet(), adminClient).isEmpty());
}
@Test
public void fetchEndOffsetsShouldRethrowRuntimeExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new RuntimeException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new RuntimeException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
Review Comment:
Okay, I will leave the verification on the admin client out for all three tests (this one and the other two below with a similar comment). We are using strict mocks which would complain that we have unnecessary stubbings if we start going down a different codepath.
--
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] cadonna commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r964500209
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##########
@@ -68,71 +76,59 @@ public void shouldTransformInputRecordToMultipleOutputRecords() {
KeyValue.pair(2, 20),
KeyValue.pair(3, 30),
KeyValue.pair(4, 40));
+
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue)).andReturn(outputRecords);
- for (final KeyValue<Integer, Integer> outputRecord : outputRecords) {
- context.forward(new Record<>(outputRecord.key, outputRecord.value, 0L));
- }
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(outputRecords);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final KeyValue<Integer, Integer> outputRecord : outputRecords) {
+ inOrder.verify(context).forward(new Record<>(outputRecord.key, outputRecord.value, 0L));
+ }
}
@Test
public void shouldAllowEmptyListAsResultOfTransform() {
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue))
- .andReturn(Collections.emptyList());
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ inOrder.verify(context, never()).forward(ArgumentMatchers.<Record<Integer, Integer>>any());
}
@Test
public void shouldAllowNullAsResultOfTransform() {
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue))
- .andReturn(null);
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ inOrder.verify(context, never()).forward(ArgumentMatchers.<Record<Integer, Integer>>any());
}
@Test
public void shouldCloseFlatTransformProcessor() {
- transformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(transformer).close();
}
@Test
public void shouldGetFlatTransformProcessor() {
+ @SuppressWarnings("unchecked")
final TransformerSupplier<Number, Number, Iterable<KeyValue<Integer, Integer>>> transformerSupplier =
mock(TransformerSupplier.class);
Review Comment:
nit: You could define the mock as a instance field and annotate it with `@Mock` to avoid suppressing the warning. That is a bit unfortunate in Mockito that there is no other way to set type parameters for mocks.
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##########
@@ -68,71 +76,59 @@ public void shouldTransformInputRecordToMultipleOutputRecords() {
KeyValue.pair(2, 20),
KeyValue.pair(3, 30),
KeyValue.pair(4, 40));
+
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue)).andReturn(outputRecords);
- for (final KeyValue<Integer, Integer> outputRecord : outputRecords) {
- context.forward(new Record<>(outputRecord.key, outputRecord.value, 0L));
- }
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(outputRecords);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final KeyValue<Integer, Integer> outputRecord : outputRecords) {
+ inOrder.verify(context).forward(new Record<>(outputRecord.key, outputRecord.value, 0L));
+ }
}
@Test
public void shouldAllowEmptyListAsResultOfTransform() {
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue))
- .andReturn(Collections.emptyList());
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ inOrder.verify(context, never()).forward(ArgumentMatchers.<Record<Integer, Integer>>any());
Review Comment:
nit: Do you actually need in order here since it is just one verification?
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +76,59 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ inOrder.verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ inOrder.verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
Review Comment:
nit: see my comment above about in order with just one verification. This comment applies also to other parts of this PR.
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##########
@@ -68,71 +76,59 @@ public void shouldTransformInputRecordToMultipleOutputRecords() {
KeyValue.pair(2, 20),
KeyValue.pair(3, 30),
KeyValue.pair(4, 40));
+
Review Comment:
nit: I would remove this line. I usually try to have a block for the setup, a block for the call under test, and a block for the verifications. Does not always work, but often.
This comment applies also to other places in this PR.
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##########
@@ -68,71 +76,59 @@ public void shouldTransformInputRecordToMultipleOutputRecords() {
KeyValue.pair(2, 20),
KeyValue.pair(3, 30),
KeyValue.pair(4, 40));
+
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue)).andReturn(outputRecords);
- for (final KeyValue<Integer, Integer> outputRecord : outputRecords) {
- context.forward(new Record<>(outputRecord.key, outputRecord.value, 0L));
- }
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(outputRecords);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final KeyValue<Integer, Integer> outputRecord : outputRecords) {
+ inOrder.verify(context).forward(new Record<>(outputRecord.key, outputRecord.value, 0L));
+ }
}
@Test
public void shouldAllowEmptyListAsResultOfTransform() {
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue))
- .andReturn(Collections.emptyList());
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ inOrder.verify(context, never()).forward(ArgumentMatchers.<Record<Integer, Integer>>any());
}
@Test
public void shouldAllowNullAsResultOfTransform() {
processor.init(context);
- EasyMock.reset(transformer);
- EasyMock.expect(transformer.transform(inputKey, inputValue))
- .andReturn(null);
- replayAll();
+ when(transformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ inOrder.verify(context, never()).forward(ArgumentMatchers.<Record<Integer, Integer>>any());
Review Comment:
See my comment above.
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/TransformerSupplierAdapterTest.java:
##########
@@ -24,117 +24,104 @@
import org.apache.kafka.streams.kstream.TransformerSupplier;
import org.apache.kafka.streams.processor.ProcessorContext;
import org.apache.kafka.streams.state.StoreBuilder;
-import org.easymock.EasyMock;
-import org.easymock.EasyMockSupport;
-import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.IsSame.sameInstance;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
-public class TransformerSupplierAdapterTest extends EasyMockSupport {
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
+public class TransformerSupplierAdapterTest {
+ @Mock
private ProcessorContext context;
+ @Mock
private Transformer<String, String, KeyValue<Integer, Integer>> transformer;
+ @Mock
private TransformerSupplier<String, String, KeyValue<Integer, Integer>> transformerSupplier;
+ @Mock
private Set<StoreBuilder<?>> stores;
final String key = "Hello";
final String value = "World";
- @Before
- public void before() {
- context = mock(ProcessorContext.class);
- transformer = mock(Transformer.class);
- transformerSupplier = mock(TransformerSupplier.class);
- stores = mock(Set.class);
- }
-
@Test
public void shouldCallInitOfAdapteeTransformer() {
- EasyMock.expect(transformerSupplier.get()).andReturn(transformer);
- transformer.init(context);
- replayAll();
+ when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
- verifyAll();
+ verify(transformer).init(context);
}
@Test
public void shouldCallCloseOfAdapteeTransformer() {
- EasyMock.expect(transformerSupplier.get()).andReturn(transformer);
- transformer.close();
- replayAll();
+ when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.close();
- verifyAll();
+ verify(transformer).close();
}
@Test
public void shouldCallStoresOfAdapteeTransformerSupplier() {
- EasyMock.expect(transformerSupplier.stores()).andReturn(stores);
- replayAll();
+ when(transformerSupplier.stores()).thenReturn(stores);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
adapter.stores();
- verifyAll();
}
@Test
public void shouldCallTransformOfAdapteeTransformerAndReturnSingletonIterable() {
- EasyMock.expect(transformerSupplier.get()).andReturn(transformer);
- EasyMock.expect(transformer.transform(key, value)).andReturn(KeyValue.pair(0, 1));
- replayAll();
+ when(transformerSupplier.get()).thenReturn(transformer);
+ when(transformer.transform(key, value)).thenReturn(KeyValue.pair(0, 1));
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
final Iterator<KeyValue<Integer, Integer>> iterator = adaptedTransformer.transform(key, value).iterator();
- verifyAll();
assertThat(iterator.hasNext(), equalTo(true));
iterator.next();
assertThat(iterator.hasNext(), equalTo(false));
}
@Test
public void shouldCallTransformOfAdapteeTransformerAndReturnEmptyIterable() {
- EasyMock.expect(transformerSupplier.get()).andReturn(transformer);
- EasyMock.expect(transformer.transform(key, value)).andReturn(null);
- replayAll();
+ when(transformerSupplier.get()).thenReturn(transformer);
+ when(transformer.transform(key, value)).thenReturn(null);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
final Iterator<KeyValue<Integer, Integer>> iterator = adaptedTransformer.transform(key, value).iterator();
- verifyAll();
assertThat(iterator.hasNext(), equalTo(false));
}
@Test
public void shouldAlwaysGetNewAdapterTransformer() {
+ @SuppressWarnings("unchecked")
final Transformer<String, String, KeyValue<Integer, Integer>> transformer1 = mock(Transformer.class);
+ @SuppressWarnings("unchecked")
final Transformer<String, String, KeyValue<Integer, Integer>> transformer2 = mock(Transformer.class);
+ @SuppressWarnings("unchecked")
final Transformer<String, String, KeyValue<Integer, Integer>> transformer3 = mock(Transformer.class);
Review Comment:
nit: see my comment above about avoiding suppressing warnings. Maybe you can just rename the global `transformer` to `transformer1` and then move `transformer2` and `transformer3` to the object fields.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
Review Comment:
nit: I think here it makes sense to move the `Consumer` mock to the object fields to avoid the suppress.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946480290
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
I have a slight preference for not verifying something which has been explicitly stubbed. My rule of thumb is to only verify behaviour of mocks which would call important logic that we otherwise have no way of confirming purely by the return value(s). For example, verify that a `close()` method on some resource has been called. What is your opinion on this @cadonna? How would you like it approached?
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946533408
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
I did a bit more digging and I found https://github.com/mockito/mockito/issues/1086. As far as I understand the issue discussed there is that the documentation can be misinterpreted to say "if you use strict stubs verification happens without you using verifyNoMoreInteractions". This was fixed in https://github.com/mockito/mockito/pull/1280. However, it was un-fixed(?) again in https://github.com/mockito/mockito/commit/cf62c73422a14641ddcaad9c4a8f581daf899049. As far as I can tell the problem still persists in Mockito 4.7.0 (Kafka uses 4.4.0).
**This fails**:
```
@After
public void tearDown() {
verifyNoMoreInteractions(transformer);
}
@Test
public void shouldCallInitOfAdapteeTransformer() {
when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
// verify(transformerSupplier).get();
// verify(transformer).init(context);
}
```
**This succeeds**:
```
@Test
public void shouldCallInitOfAdapteeTransformer() {
when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
// verify(transformerSupplier).get();
// verify(transformer).init(context);
}
```
As such, I suspect that if we truly want to be as strict as possible we have to use both strict stubs and verifyNoMoreInteractions pretty much everywhere. Thoughts?
--
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] cadonna commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946788936
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
I think it does so, because you could have stubbings in the setup method for the test class. If only some test methods use those stubbings, you would get failures if you run a single test that does not use the those stubbings.
--
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] dplavcic commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
dplavcic commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r944750815
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamPrintTest.java:
##########
@@ -31,11 +33,15 @@
import static org.junit.Assert.assertEquals;
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
public class KStreamPrintTest {
private ByteArrayOutputStream byteOutStream;
private Processor<Integer, String, Void, Void> printProcessor;
+ @Mock
+ ProcessorContext<Void, Void> processorContext;
Review Comment:
[optional] Can you please double check if we can set `private` access modifier to the `ProcessorContext<Void, Void> processorContext;` ?
--
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] cadonna commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r944375842
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/MaterializedInternalTest.java:
##########
@@ -73,11 +71,11 @@ public void shouldUseProvidedStoreNameWhenSet() {
@Test
public void shouldUseStoreNameOfSupplierWhenProvided() {
final String storeName = "other-store-name";
- EasyMock.expect(supplier.name()).andReturn(storeName).anyTimes();
- EasyMock.replay(supplier);
+ when(supplier.name()).thenReturn(storeName);
final MaterializedInternal<Object, Object, KeyValueStore<Bytes, byte[]>> materialized =
new MaterializedInternal<>(Materialized.as(supplier), nameProvider, prefix);
assertThat(materialized.storeName(), equalTo(storeName));
+ verify(supplier, times(2)).name();
Review Comment:
Why `times(2)`? The original test does not says anything about the number of calls. I think if you swap line 77 with line 78, you can remove `times(2)`.
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/TransformerSupplierAdapterTest.java:
##########
@@ -145,10 +141,12 @@ public void shouldAlwaysGetNewAdapterTransformer() {
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adapterTransformer3 = adapter.get();
adapterTransformer3.init(context);
- verifyAll();
assertThat(adapterTransformer1, not(sameInstance(adapterTransformer2)));
assertThat(adapterTransformer2, not(sameInstance(adapterTransformer3)));
assertThat(adapterTransformer3, not(sameInstance(adapterTransformer1)));
+ verify(transformer1).init(any(ProcessorContext.class));
+ verify(transformer2).init(any(ProcessorContext.class));
+ verify(transformer3).init(any(ProcessorContext.class));
Review Comment:
Why not?
```
verify(transformer1).init(context);
verify(transformer2).init(context);
verify(transformer3).init(context);
```
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##########
@@ -47,19 +56,14 @@ public class KStreamFlatTransformTest extends EasyMockSupport {
public void setUp() {
inputKey = 1;
inputValue = 10;
- transformer = mock(Transformer.class);
- context = strictMock(InternalProcessorContext.class);
Review Comment:
In EasyMock a strict mock verifies also the order of the calls. In Mockito that is done by using `InOrder` (https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#in_order_verification). Currently, this PR does not check the order which softens up the test.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java:
##########
@@ -148,34 +155,27 @@ private void createMockTaskManager(final Set<TaskId> activeTasks) {
}
private void createMockTaskManager(final Map<TaskId, Long> taskOffsetSums) {
- taskManager = EasyMock.createNiceMock(TaskManager.class);
- expect(taskManager.topologyMetadata()).andStubReturn(topologyMetadata);
- expect(taskManager.getTaskOffsetSums()).andReturn(taskOffsetSums).anyTimes();
Review Comment:
Why was this stub not migrated?
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -47,19 +56,14 @@ public class KStreamFlatTransformValuesTest extends EasyMockSupport {
public void setUp() {
inputKey = 1;
inputValue = 10;
- valueTransformer = mock(ValueTransformerWithKey.class);
- context = strictMock(InternalProcessorContext.class);
Review Comment:
n EasyMock a strict mock verifies also the order of the calls. In Mockito that is done by using `InOrder` (https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#in_order_verification). Currently, this PR does not check the order which softens up the test.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new KafkaException());
assertThrows(StreamsException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldRethrowTimeoutException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new TimeoutException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new TimeoutException());
assertThrows(TimeoutException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
assertTrue(fetchCommittedOffsets(emptySet(), consumer).isEmpty());
}
@Test
public void fetchEndOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
+ final Admin adminClient = mock(AdminClient.class);
assertTrue(fetchEndOffsets(emptySet(), adminClient).isEmpty());
}
@Test
public void fetchEndOffsetsShouldRethrowRuntimeExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new RuntimeException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new RuntimeException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
Review Comment:
Verifications of the admin client are missing.
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/TransformerSupplierAdapterTest.java:
##########
@@ -24,117 +24,113 @@
import org.apache.kafka.streams.kstream.TransformerSupplier;
import org.apache.kafka.streams.processor.ProcessorContext;
import org.apache.kafka.streams.state.StoreBuilder;
-import org.easymock.EasyMock;
-import org.easymock.EasyMockSupport;
-import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.IsSame.sameInstance;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
-public class TransformerSupplierAdapterTest extends EasyMockSupport {
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
+public class TransformerSupplierAdapterTest {
+ @Mock
private ProcessorContext context;
+ @Mock
private Transformer<String, String, KeyValue<Integer, Integer>> transformer;
+ @Mock
private TransformerSupplier<String, String, KeyValue<Integer, Integer>> transformerSupplier;
+ @Mock
private Set<StoreBuilder<?>> stores;
final String key = "Hello";
final String value = "World";
- @Before
- public void before() {
- context = mock(ProcessorContext.class);
- transformer = mock(Transformer.class);
- transformerSupplier = mock(TransformerSupplier.class);
- stores = mock(Set.class);
- }
-
@Test
public void shouldCallInitOfAdapteeTransformer() {
- EasyMock.expect(transformerSupplier.get()).andReturn(transformer);
- transformer.init(context);
- replayAll();
+ when(transformerSupplier.get()).thenReturn(transformer);
final TransformerSupplierAdapter<String, String, Integer, Integer> adapter =
new TransformerSupplierAdapter<>(transformerSupplier);
final Transformer<String, String, Iterable<KeyValue<Integer, Integer>>> adaptedTransformer = adapter.get();
adaptedTransformer.init(context);
- verifyAll();
+ verify(transformerSupplier).get();
+ verify(transformer).init(any(ProcessorContext.class));
Review Comment:
Is it possible to use `verify(transformer).init(context)`?
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new KafkaException());
assertThrows(StreamsException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldRethrowTimeoutException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new TimeoutException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new TimeoutException());
assertThrows(TimeoutException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
assertTrue(fetchCommittedOffsets(emptySet(), consumer).isEmpty());
}
@Test
public void fetchEndOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
+ final Admin adminClient = mock(AdminClient.class);
assertTrue(fetchEndOffsets(emptySet(), adminClient).isEmpty());
}
@Test
public void fetchEndOffsetsShouldRethrowRuntimeExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new RuntimeException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new RuntimeException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
}
@Test
public void fetchEndOffsetsShouldRethrowInterruptedExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new InterruptedException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new InterruptedException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
Review Comment:
Verifications of the admin client are missing.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new KafkaException());
assertThrows(StreamsException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldRethrowTimeoutException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new TimeoutException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new TimeoutException());
assertThrows(TimeoutException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
assertTrue(fetchCommittedOffsets(emptySet(), consumer).isEmpty());
}
@Test
public void fetchEndOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
+ final Admin adminClient = mock(AdminClient.class);
assertTrue(fetchEndOffsets(emptySet(), adminClient).isEmpty());
}
@Test
public void fetchEndOffsetsShouldRethrowRuntimeExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new RuntimeException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new RuntimeException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
}
@Test
public void fetchEndOffsetsShouldRethrowInterruptedExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new InterruptedException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new InterruptedException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
}
@Test
public void fetchEndOffsetsShouldRethrowExecutionExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new ExecutionException(new RuntimeException()));
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new ExecutionException(new RuntimeException()));
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
Review Comment:
Verifications of the admin client are missing.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946814408
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/ClientUtilsTest.java:
##########
@@ -108,75 +110,73 @@ public class ClientUtilsTest {
@Test
public void fetchCommittedOffsetsShouldRethrowKafkaExceptionAsStreamsException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new KafkaException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new KafkaException());
assertThrows(StreamsException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldRethrowTimeoutException() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
- expect(consumer.committed(PARTITIONS)).andThrow(new TimeoutException());
- replay(consumer);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
+ when(consumer.committed(PARTITIONS)).thenThrow(new TimeoutException());
assertThrows(TimeoutException.class, () -> fetchCommittedOffsets(PARTITIONS, consumer));
}
@Test
public void fetchCommittedOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Consumer<byte[], byte[]> consumer = EasyMock.createMock(Consumer.class);
+ @SuppressWarnings("unchecked")
+ final Consumer<byte[], byte[]> consumer = mock(Consumer.class);
assertTrue(fetchCommittedOffsets(emptySet(), consumer).isEmpty());
}
@Test
public void fetchEndOffsetsShouldReturnEmptyMapIfPartitionsAreEmpty() {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
+ final Admin adminClient = mock(AdminClient.class);
assertTrue(fetchEndOffsets(emptySet(), adminClient).isEmpty());
}
@Test
public void fetchEndOffsetsShouldRethrowRuntimeExceptionAsStreamsException() throws Exception {
- final Admin adminClient = EasyMock.createMock(AdminClient.class);
- final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class);
- final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = EasyMock.createMock(KafkaFuture.class);
+ final Admin adminClient = mock(AdminClient.class);
+ final ListOffsetsResult result = mock(ListOffsetsResult.class);
+ @SuppressWarnings("unchecked")
+ final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> allFuture = mock(KafkaFuture.class);
- EasyMock.expect(adminClient.listOffsets(EasyMock.anyObject())).andStubReturn(result);
- EasyMock.expect(result.all()).andStubReturn(allFuture);
- EasyMock.expect(allFuture.get()).andThrow(new RuntimeException());
- replay(adminClient, result, allFuture);
+ when(adminClient.listOffsets(any())).thenReturn(result);
+ when(result.all()).thenReturn(allFuture);
+ when(allFuture.get()).thenThrow(new RuntimeException());
assertThrows(StreamsException.class, () -> fetchEndOffsets(PARTITIONS, adminClient));
- verify(adminClient);
Review Comment:
@cadonna Following the logic about not verifying stubs discussed in the comment above what else would you like to be verified here?
--
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] clolov commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1217679169
@cadonna and @dplavcic I hope that the new version has addressed all comments up to now. Thank you in advance for the 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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946760595
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
@cadonna I went into the source code of Mockito and came across the following
```
// only report when:
// 1. if all tests from given test have ran (filter requested is false)
// Otherwise we would report unnecessary stubs even if the user runs just single test
// from the class
// 2. tests are successful (we don't want to add an extra failure on top of any existing
// failure, to avoid confusion)
```
Indeed, if you run the test class rather than just the test you do get the report. Why it does this, however, I do not know.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946487973
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/MaterializedInternalTest.java:
##########
@@ -73,11 +71,11 @@ public void shouldUseProvidedStoreNameWhenSet() {
@Test
public void shouldUseStoreNameOfSupplierWhenProvided() {
final String storeName = "other-store-name";
- EasyMock.expect(supplier.name()).andReturn(storeName).anyTimes();
- EasyMock.replay(supplier);
+ when(supplier.name()).thenReturn(storeName);
final MaterializedInternal<Object, Object, KeyValueStore<Bytes, byte[]>> materialized =
new MaterializedInternal<>(Materialized.as(supplier), nameProvider, prefix);
assertThat(materialized.storeName(), equalTo(storeName));
+ verify(supplier, times(2)).name();
Review Comment:
That's a fair point, swapped lines 77 with 78 and removed `times(2)` in upcoming commits.
--
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] cadonna commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946788936
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -68,69 +72,60 @@ public void shouldTransformInputRecordToMultipleOutputValues() {
"Hello",
"Blue",
"Planet");
+
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(outputValues);
- for (final String outputValue : outputValues) {
- context.forward(new Record<>(inputKey, outputValue, 0L));
- }
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(outputValues);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ for (final String outputValue : outputValues) {
+ verify(context).forward(new Record<>(inputKey, outputValue, 0L));
+ }
}
@Test
public void shouldEmitNoRecordIfTransformReturnsEmptyList() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(Collections.emptyList());
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(Collections.emptyList());
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldEmitNoRecordIfTransformReturnsNull() {
processor.init(context);
- EasyMock.reset(valueTransformer);
- EasyMock.expect(valueTransformer.transform(inputKey, inputValue)).andReturn(null);
- replayAll();
+ when(valueTransformer.transform(inputKey, inputValue)).thenReturn(null);
processor.process(new Record<>(inputKey, inputValue, 0L));
- verifyAll();
+ verify(context, never()).forward(ArgumentMatchers.<Record<Integer, String>>any());
}
@Test
public void shouldCloseFlatTransformValuesProcessor() {
- valueTransformer.close();
- replayAll();
-
processor.close();
- verifyAll();
+ verify(valueTransformer).close();
}
@Test
public void shouldGetFlatTransformValuesProcessor() {
+ @SuppressWarnings("unchecked")
final ValueTransformerWithKeySupplier<Integer, Integer, Iterable<String>> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final KStreamFlatTransformValues<Integer, Integer, String> processorSupplier =
new KStreamFlatTransformValues<>(valueTransformerSupplier);
- EasyMock.expect(valueTransformerSupplier.get()).andReturn(valueTransformer);
- replayAll();
+ when(valueTransformerSupplier.get()).thenReturn(valueTransformer);
final Processor<Integer, Integer, Integer, String> processor = processorSupplier.get();
- verifyAll();
+ verify(valueTransformerSupplier).get();
Review Comment:
I think it does so, because you could have stubbings in the setup method for the test class. If only some test methods use those stubbings, you would get failures if you run a single test that does not use those stubbings.
--
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] cadonna commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
cadonna commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1239066227
Build failures are unrelated:
```
Build / JDK 11 and Scala 2.13 / org.apache.kafka.controller.QuorumControllerTest.testBalancePartitionLeaders()
Build / JDK 8 and Scala 2.12 / org.apache.kafka.controller.QuorumControllerTest.testDelayedConfigurationOperations()
```
--
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] dplavcic commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
dplavcic commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1219679741
> @cadonna and @dplavcic I hope that the new version has addressed all comments up to now. Thank you in advance for the review!
LGTM!
--
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] clolov commented on pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on PR #12492:
URL: https://github.com/apache/kafka/pull/12492#issuecomment-1208034281
Also @mdedetrich since you volunteered a pair of eyes for reviews :)
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946945343
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformValuesTest.java:
##########
@@ -47,19 +56,14 @@ public class KStreamFlatTransformValuesTest extends EasyMockSupport {
public void setUp() {
inputKey = 1;
inputValue = 10;
- valueTransformer = mock(ValueTransformerWithKey.class);
- context = strictMock(InternalProcessorContext.class);
Review Comment:
Addressed in subsequent commits.
--
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] clolov commented on a diff in pull request #12492: KAFKA-14133: Replace EasyMock with Mockito in streams tests
Posted by GitBox <gi...@apache.org>.
clolov commented on code in PR #12492:
URL: https://github.com/apache/kafka/pull/12492#discussion_r946487527
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamPrintTest.java:
##########
@@ -31,11 +33,15 @@
import static org.junit.Assert.assertEquals;
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
public class KStreamPrintTest {
private ByteArrayOutputStream byteOutStream;
private Processor<Integer, String, Void, Void> printProcessor;
+ @Mock
+ ProcessorContext<Void, Void> processorContext;
Review Comment:
Change the modifier to `private` in upcoming commits.
--
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