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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
