mdedetrich commented on code in PR #12781:
URL: https://github.com/apache/kafka/pull/12781#discussion_r1003264688
##########
connect/runtime/src/test/java/org/apache/kafka/connect/util/KafkaBasedLogTest.java:
##########
@@ -135,8 +161,7 @@ public class KafkaBasedLogTest {
@SuppressWarnings("unchecked")
@Before
public void setUp() {
- store = PowerMock.createPartialMock(KafkaBasedLog.class, new
String[]{"createConsumer", "createProducer"},
- TOPIC, PRODUCER_PROPS, CONSUMER_PROPS, consumedCallback, time,
initializer);
+ store = new MockedKafkaBasedLog(TOPIC, PRODUCER_PROPS, CONSUMER_PROPS,
() -> null, consumedCallback, time, null);
Review Comment:
Minor tidbit, in the old test `initializer` was actually the wrong type (its
a `Runnable` when it should be a `Supplier`) so if it was ever accessed during
a test it would have thrown a `ClassCastException` (or something along those
lines). Furthermore the `createPartialMock` was actually missing an argument
(constructor has 7 arguments, there are only 6 here).
This is why the version in this PR as `() -> null` as a `topicAdminSupplier`
and `null` for the `initializer`. These fields are never actually called (or if
they are they are setup with another method, i.e. `setupWithAdmin` which
inserts the correct fields)
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
