showuon commented on a change in pull request #11340:
URL: https://github.com/apache/kafka/pull/11340#discussion_r776176434
##########
File path:
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinatorTest.java
##########
@@ -3202,17 +3202,18 @@ private void gracefulCloseTest(ConsumerCoordinator
coordinator, boolean shouldLe
OffsetCommitRequest commitRequest = (OffsetCommitRequest) body;
return commitRequest.data().groupId().equals(groupId);
}, new OffsetCommitResponse(new OffsetCommitResponseData()));
+ if (shouldLeaveGroup)
Review comment:
Could you explain why we need to check `shouldLeaveGroup` after your
change?
##########
File path:
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java
##########
@@ -2230,7 +2230,7 @@ public void testInvalidGroupMetadata() throws
InterruptedException {
MockClient client = new MockClient(time, metadata);
initMetadata(client, Collections.singletonMap(topic, 1));
KafkaConsumer<String, String> consumer = newConsumer(time, client,
subscription, metadata,
- new RoundRobinAssignor(), true, groupInstanceId);
+ new RoundRobinAssignor(), false, groupInstanceId);
Review comment:
I'm afraid the change of `autoCommit` flag will change the test purpose.
I checked and it looks like we'll wait for consumer to close forever, because
the timer didn't tick. Could we do `consumer.close(Duration.ZERO);`, and keep
the `autoCommit` as true as before? What do you think?
--
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]
