k-apol commented on code in PR #20326:
URL: https://github.com/apache/kafka/pull/20326#discussion_r2506964116
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/InternalTopicManagerTest.java:
##########
@@ -782,16 +782,18 @@ public void shouldCompleteTopicValidationOnRetry() {
// let the first describe succeed on topic, and fail on topic2, and
then let creation throws topics-existed;
// it should retry with just topic2 and then let it succeed
when(admin.describeTopics(Set.of(topic1, topic2)))
- .thenAnswer(answer -> new MockDescribeTopicsResult(mkMap(
- mkEntry(topic1, topicDescriptionSuccessFuture),
- mkEntry(topic2, topicDescriptionFailFuture)
- )));
+ .thenAnswer(answer -> new MockDescribeTopicsResult(mkMap(
+ mkEntry(topic1, topicDescriptionSuccessFuture),
+ mkEntry(topic2, topicDescriptionFailFuture) // first
call: missing
+ )))
+ .thenAnswer(answer -> new MockDescribeTopicsResult(mkMap(
+ mkEntry(topic1, topicDescriptionSuccessFuture),
Review Comment:
So when tracing this test to see why it was failing, it was retrying topic 2
repeatedly (which is always throwing the `TopicExistsException`, as the stub
indicates), but it was never exiting that loop. When mocking the time so that
it will deadline, I ultimately got a timeout exception (which I think is fine
here?).
I felt it was better to mock it succeeding the second call than changing the
expected test output by expecting the timeout exception. It still models a
transient error, and is still forced to exercise the retry logic that I feel
this test is looking to validate.
--
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]
