[GitHub] [kafka] dongjinleekr commented on pull request #8130: MINOR: trivial cleanups, javadoc errors, omitted StateStore tests, etc.

[GitBox <gi...@apache.org>]

dongjinleekr commented on pull request #8130:
URL: https://github.com/apache/kafka/pull/8130#issuecomment-646991777


   @mjsax Here is the fix. I investigated the problem and found the following.
   
   The root of the problem is: the consturctors of `XXXStoreBuilder` classes omit the MetricsScope nullity check (see commit `5b432d94f`.) As of present, the `NullPointerException`s are thrown from the constructor of `AbstractStoreBuilder` - it has nothing to do with MetricsScope. (Interestingly, `SessionStoreBuilderTest#shouldThrowNullPointerIfMetricsScopeIsNull` already takes the right way.)
   
   As a evidence, if we give the `XXXBytesStoreSupplier` mock with non-null `name` no exception is thrown. (see commit `01f2cf314`)
   
   To fix this problem, I added a validation method to each of the constructor of `XXXStoreBuilder`; To make the tests not broken, I also made the supplier mocks to return non-null MetricsScope by `expect(supplier.metricsScope()).andReturn("metricScope");` (see commit `2e0043b2b`)
   
   +1. I removed the 'two space' removal while rebasing onto the latest trunk; You are right, it is worthless. Instead, I added the other trivial fixes I found during reading the code.


----------------------------------------------------------------
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:
users@infra.apache.org