cameronlee314 commented on a change in pull request #1382:
URL: https://github.com/apache/samza/pull/1382#discussion_r439657646
##########
File path:
samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
##########
@@ -82,7 +83,6 @@ class TestKafkaCheckpointManager extends
KafkaServerTestHarness {
val spec = new KafkaStreamSpec("id", checkpointTopic,
checkpointSystemName, 1, 1, props)
val checkPointManager = Mockito.spy(new KafkaCheckpointManager(spec, new
MockSystemFactory, false, config, new NoOpMetricsRegistry))
Review comment:
It's not necessary to use the spy, but this test seems to be using some
"real" method implementations for some underlying objects, so I guess it is a
little awkward to mix in mocking too. I'm not a fan of how this unit test class
was written overall (e.g. I think it uses an embedded Kafka/Zk just to run a
unit test), but I also didn't have time to refactor everything, so I went with
the easiest fix.
----------------------------------------------------------------
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:
[email protected]
