chia7712 commented on a change in pull request #9660: URL: https://github.com/apache/kafka/pull/9660#discussion_r532055796
########## File path: streams/test-utils/src/test/java/org/apache/kafka/streams/TopologyTestDriverTest.java ########## @@ -471,7 +471,7 @@ public void shouldCloseProcessor() { @Test public void shouldThrowForUnknownTopic() { - testDriver = new TopologyTestDriver(new Topology(), config); + testDriver = new TopologyTestDriver(new Topology()); Review comment: the ```config``` is not from empty properties so it may be incorrect to remove ```config```. ########## File path: streams/test-utils/src/main/java/org/apache/kafka/streams/TopologyTestDriver.java ########## @@ -298,8 +322,14 @@ private TopologyTestDriver(final InternalTopologyBuilder builder, final long initialWallClockTimeMs) { final Properties configCopy = new Properties(); configCopy.putAll(config); - configCopy.putIfAbsent(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, "dummy-bootstrap-host:0"); - configCopy.putIfAbsent(StreamsConfig.APPLICATION_ID_CONFIG, "dummy-topology-test-driver-app-id"); + if (!configCopy.containsKey(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG)) { Review comment: It seems to me ```putIfAbsent``` is more readable than ```containsKey + setProperty```. ########## File path: streams/test-utils/src/test/java/org/apache/kafka/streams/TopologyTestDriverTest.java ########## @@ -471,7 +471,7 @@ public void shouldCloseProcessor() { @Test public void shouldThrowForUnknownTopic() { - testDriver = new TopologyTestDriver(new Topology(), config); + testDriver = new TopologyTestDriver(new Topology()); Review comment: there are a lot of similar code change. ########## File path: streams/src/test/java/org/apache/kafka/streams/TopologyTest.java ########## @@ -353,8 +349,7 @@ public void shouldThrowOnUnassignedStateStoreAccess() { .addProcessor(badNodeName, new LocalMockProcessorSupplier(), sourceNodeName); try { - new TopologyTestDriver(topology, mkProperties( - mkMap(mkEntry(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory().getAbsolutePath())))); + new TopologyTestDriver(topology); Review comment: Is it ok to remove ```StreamsConfig.STATE_DIR_CONFIG``` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org