cadonna commented on a change in pull request #10529: URL: https://github.com/apache/kafka/pull/10529#discussion_r612266039
########## File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/InternalTopicManagerTest.java ########## @@ -153,7 +156,8 @@ public void shouldNotCreateTopicsWithEmptyInput() throws Exception { @Test public void shouldOnlyRetryNotSuccessfulFuturesDuringSetup() { - final AdminClient admin = EasyMock.createNiceMock(AdminClient.class); + final AdminClient admin = EasyMock.createStrictMock(AdminClient.class); + config.put(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG, 10_000L); Review comment: I think a default mock would be enough to get a better error message. Strict mocks also check the order of the calls which is not needed here. I would also pass `new MockTime(1)` instead of increasing the timeout, because the test retrieves the time twice. `new MockTime(1)` would progress time 1 ms each time. Hence, we would be within the 100 ms of the global `max.poll.interval.ms` and since we control the time progression the test would be more robust than by increasing `max.poll.interval.ms` to a higher value. With `new MockTime(1)`, we also avoid running the test indefinitely in case of a mistake in the production code, since after 50 time retrievals, the test would run into a timeout and the admin client mock would throw the unexpected method call error. Probably, it would be even better to use something like `new MockTime((Integer) config.get(StreamsConfig.consumerPrefix(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG)) / 3)`. -- 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