MINOR: migrate DescribeConsumerGroupTest to use ClusterTestExtensions #15908
Conversation
| @@ -92,6 +94,7 @@ static void generator(ClusterGenerator clusterGenerator) { | |||
| static <T> AutoCloseable buildConsumers(int numberOfConsumers, | |||
| boolean syncCommit, | |||
| String topic, | |||
| Collection<TopicPartition> topicPartitions, | |||
There was a problem hiding this comment.
nit: Change to set to be more specific.
| @@ -311,6 +311,7 @@ private AutoCloseable consumerGroupClosable(ClusterInstance cluster, GroupProtoc | |||
| 1, | |||
| false, | |||
| topicName, | |||
| null, | |||
There was a problem hiding this comment.
nit: Could we use empty set instead of null here?
| GroupProtocol groupProtocol = clusterInstance.config().serverProperties().get(GroupCoordinatorConfig.NEW_GROUP_COORDINATOR_ENABLE_CONFIG).trim().equals("true") ? GroupProtocol.CONSUMER : GroupProtocol.CLASSIC; | ||
| try (AutoCloseable protocolConsumerGroupExecutor = consumerGroupClosable(groupProtocol, PROTOCOL_GROUP, TOPIC, Collections.emptyMap()); |
There was a problem hiding this comment.
It looks to me that we could put the logic of determining groupProtocol in consumerGroupClosable method, so that we don't have to do the same thing in each test. Ex: Add a consumerGroupClosable method, and replace the groupProtocol with clusterInstance.config().serverProperties(). So we can do the groupProtocol determination inside the consumerGroupClosable method:
private AutoCloseable consumerGroupClosable(Map<String, String> serverProperties, GroupProtocol protocol, String groupId, String topicName) {
GroupProtocol groupProtocol = serverProperties.get(GroupCoordinatorConfig.NEW_GROUP_COORDINATOR_ENABLE_CONFIG).trim().equals("true") ? GroupProtocol.CONSUMER : GroupProtocol.CLASSIC;
return consumerGroupClosable(groupProtocol, ...);
|
One high-level question:
Anyway, to me, one test class takes more than 1 hour is really unacceptable. Trunk: |
|
Forgot to say, thanks for closing all the |
Signed-off-by: PoAn Yang <payang@apache.org>
FrankYang0529
force-pushed
the
minor-migrate-DescribeConsumerGroup
branch
from
bf608ec
to
79dfa65
Compare
By using ClusterTestExtensions, DescribeConsumerGroupTest get get away from KafkaServerTestHarness dependency.
Committer Checklist (excluded from commit message)