[
https://issues.apache.org/jira/browse/KAFKA-8977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16951855#comment-16951855
]
bibin sebastian commented on KAFKA-8977:
----------------------------------------
Got it. Glancing through the test cases/classes where MockStreamsMetrics is
used, what I see is that in lot of the test cases, class/object that is tested,
already have references to many other classes. streamsMetrics instance is
passed down to these reference classes and they in turn call the methods on
streamsMetrics.
Here is an example. ThreadCache accepts StreamsMetrics as a constructor
parameter (this.metrics) and this is passed down to NamesCache instance as
follows in getOrCreateCache() method.
{code:java}
public ThreadCache(final LogContext logContext, final long maxCacheSizeBytes,
final StreamsMetricsImpl metrics) {
this.maxCacheSizeBytes = maxCacheSizeBytes;
this.metrics = metrics;
this.log = logContext.logger(getClass());
}
private synchronized NamedCache getOrCreateCache(final String name) {
NamedCache cache = caches.get(name);
if (cache == null) {
cache = new NamedCache(name, this.metrics);
caches.put(name, cache);
}
return cache;
}
{code}
Now if you look at the constructor of NamedCache (shown below), streamsMetrics
is again passed down to NamedCacheMetrics.
{code:java}
NamedCache(final String name, final StreamsMetricsImpl streamsMetrics) {
this.name = name;
this.streamsMetrics = streamsMetrics;
storeName = ThreadCache.underlyingStoreNamefromCacheName(name);
taskName = ThreadCache.taskIDfromCacheName(name);
hitRatioSensor = NamedCacheMetrics.hitRatioSensor(
streamsMetrics,
Thread.currentThread().getName(),
taskName,
storeName
);
}
{code}
While replacing MockStreamsMetrics with an EasyMock/PowerMock instances, I am
also trying to mock the external dependencies (wherever appropriate) to reduce
the scope of the mocking StreamsMetricsImpl. So in this just above mentioned
case I will also mock NamedCacheMetrics.hitRatioSensor() as this is an external
logic to ThreadCache. IMO this will leave tests in a better shape as a general
practice it is good to mock all the external dependencies in a unit test.
There are many test files where we have this above mentioned situation. I will
share the PR once I am done with the changes.
> Remove MockStreamsMetrics Since it is not a Mock
> ------------------------------------------------
>
> Key: KAFKA-8977
> URL: https://issues.apache.org/jira/browse/KAFKA-8977
> Project: Kafka
> Issue Type: Improvement
> Components: streams
> Reporter: Bruno Cadonna
> Assignee: bibin sebastian
> Priority: Minor
> Labels: newbie
>
> The class {{MockStreamsMetrics}} is used throughout unit tests as a mock but
> it is not really a mock since it only hides two parameters of the
> {{StreamsMetricsImpl}} constructor. Either a real mock or the real
> {{StreamsMetricsImpl}} should be used in the tests.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
