ableegoldman commented on a change in pull request #8902:
URL: https://github.com/apache/kafka/pull/8902#discussion_r446316588
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStoreTest.java
##########
@@ -124,18 +141,19 @@ public void before() {
Serdes.String()
);
metrics.config().recordLevel(Sensor.RecordingLevel.DEBUG);
- expect(context.metrics())
- .andReturn(new StreamsMetricsImpl(metrics, "test",
builtInMetricsVersion)).anyTimes();
- expect(context.taskId()).andReturn(taskId).anyTimes();
- expect(inner.name()).andReturn("metered").anyTimes();
+ expect(context.applicationId()).andStubReturn(APPLICATION_ID);
+ expect(context.metrics()).andStubReturn(new
StreamsMetricsImpl(metrics, "test", builtInMetricsVersion));
+ expect(context.taskId()).andStubReturn(taskId);
+
expect(context.changelogFor(STORE_NAME)).andStubReturn(CHANGELOG_TOPIC);
Review comment:
I think Bruno needs to give us all a short lesson on the correct usage
of EasyMock. He explained the `StubReturn` thing to me on another PR a while
back and I've been (slowly) trying to help migrate all the
`.andReturn.anyTimes` usages over to this where appropriate (which is most
places). It's definitely helped reduce the large number of EasyMock'ed tests
that have to be fixed after every minor implementation change.
That said, there does seem to be one critical difference between using
`.andStubReturn` and the actual `MockInternalProcessorContext`. If we add a new
method to the `InternalProcessorContext` interface, for example, we then have
to add this expectation to every test that calls it with an EasyMock context.
Having had to do this a number of times, it's definitely a huge timesuck.
But I also agree that it doesn't need to be done as part of this PR. Maybe
once the MockInternal and InternalMock processor contexts are finally merged we
can do some reasonable cleanup of the tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
