ijuma commented on code in PR #15488:
URL: https://github.com/apache/kafka/pull/15488#discussion_r1518681734
##########
core/src/test/scala/unit/kafka/log/LogLoaderTest.scala:
##########
@@ -352,19 +352,15 @@ class LogLoaderTest {
// Intercept all segment read calls
val interceptedLogSegments = new LogSegments(topicPartition) {
override def add(segment: LogSegment): LogSegment = {
- val wrapper = new LogSegment(segment) {
-
- override def read(startOffset: Long, maxSize: Int, maxPosition:
Long, minOneMessage: Boolean): FetchDataInfo = {
- segmentsWithReads += this
- super.read(startOffset, maxSize, maxPosition, minOneMessage)
- }
-
- override def recover(producerStateManager: ProducerStateManager,
- leaderEpochCache:
Optional[LeaderEpochFileCache]): Int = {
- recoveredSegments += this
- super.recover(producerStateManager, leaderEpochCache)
- }
- }
+ val wrapper = Mockito.spy(segment)
Review Comment:
There is a lot of magic to make mocks work and even more so for spy mocks.
Replacing readable/debuggable normal code for mocks is generally a bad idea. A
concrete example or the problems with mocks are the mock leaks that have caused
serious test instability.
As a general rule, mocks should only be used if the mock free alternative is
clearly worse in terms of verbosity, maintainability, etc. I don't think it's
the case 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
