Re: [PR] MINOR: use classic consumer with ZK mode for DeleteOffsetsConsumerGroupCommandIntegrationTest [kafka]

2024-05-09 Thread via GitHub


chia7712 merged PR #15872:
URL: https://github.com/apache/kafka/pull/15872


-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: use classic consumer with ZK mode for DeleteOffsetsConsumerGroupCommandIntegrationTest [kafka]

2024-05-08 Thread via GitHub


FrankYang0529 commented on code in PR #15872:
URL: https://github.com/apache/kafka/pull/15872#discussion_r1594865250


##
tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java:
##
@@ -57,28 +56,47 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 
 @Tag("integration")
-@ClusterTestDefaults(clusterType = Type.ALL, serverProperties = {
-@ClusterConfigProperty(key = 
GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
-@ClusterConfigProperty(key = 
GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1"),
-@ClusterConfigProperty(key = "group.coordinator.new.enable", value = 
"true")
-})
 @ExtendWith(ClusterTestExtensions.class)
 public class DeleteOffsetsConsumerGroupCommandIntegrationTest {
 public static final String TOPIC_PREFIX = "foo.";
 public static final String GROUP_PREFIX = "test.group.";
 private final ClusterInstance clusterInstance;
 
-private final Iterable> consumerConfigs;
-
 DeleteOffsetsConsumerGroupCommandIntegrationTest(ClusterInstance 
clusterInstance) {
 this.clusterInstance = clusterInstance;
-this.consumerConfigs = clusterInstance.isKRaftTest()
-? 
Arrays.asList(Collections.singletonMap(ConsumerConfig.GROUP_PROTOCOL_CONFIG, 
GroupProtocol.CLASSIC.name()),
-Collections.singletonMap(ConsumerConfig.GROUP_PROTOCOL_CONFIG, 
GroupProtocol.CONSUMER.name()))
-: Collections.singletonList(Collections.emptyMap());
 }
 
-@ClusterTest
+private static void generator(ClusterGenerator clusterGenerator) {
+Map serverProperties = new HashMap<>();

Review Comment:
   Updated it. Thanks.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: use classic consumer with ZK mode for DeleteOffsetsConsumerGroupCommandIntegrationTest [kafka]

2024-05-07 Thread via GitHub


chia7712 commented on code in PR #15872:
URL: https://github.com/apache/kafka/pull/15872#discussion_r1593354055


##
tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java:
##
@@ -57,28 +56,47 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 
 @Tag("integration")
-@ClusterTestDefaults(clusterType = Type.ALL, serverProperties = {
-@ClusterConfigProperty(key = 
GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
-@ClusterConfigProperty(key = 
GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1"),
-@ClusterConfigProperty(key = "group.coordinator.new.enable", value = 
"true")
-})
 @ExtendWith(ClusterTestExtensions.class)
 public class DeleteOffsetsConsumerGroupCommandIntegrationTest {
 public static final String TOPIC_PREFIX = "foo.";
 public static final String GROUP_PREFIX = "test.group.";
 private final ClusterInstance clusterInstance;
 
-private final Iterable> consumerConfigs;
-
 DeleteOffsetsConsumerGroupCommandIntegrationTest(ClusterInstance 
clusterInstance) {
 this.clusterInstance = clusterInstance;
-this.consumerConfigs = clusterInstance.isKRaftTest()
-? 
Arrays.asList(Collections.singletonMap(ConsumerConfig.GROUP_PROTOCOL_CONFIG, 
GroupProtocol.CLASSIC.name()),
-Collections.singletonMap(ConsumerConfig.GROUP_PROTOCOL_CONFIG, 
GroupProtocol.CONSUMER.name()))
-: Collections.singletonList(Collections.emptyMap());
 }
 
-@ClusterTest
+private static void generator(ClusterGenerator clusterGenerator) {
+Map serverProperties = new HashMap<>();

Review Comment:
   Please use `ConsumerGroupCommandTestUtils#generator`



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: use classic consumer with ZK mode for DeleteOffsetsConsumerGroupCommandIntegrationTest [kafka]

2024-05-07 Thread via GitHub


FrankYang0529 commented on code in PR #15872:
URL: https://github.com/apache/kafka/pull/15872#discussion_r1592664550


##
tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java:
##
@@ -88,7 +113,7 @@ public void testDeleteOffsetsNonExistingGroup() {
 }
 }
 
-@ClusterTest
+@ClusterTemplate("generator")
 public void testDeleteOffsetsOfStableConsumerGroupWithTopicPartition() {
 for (Map consumerConfig: consumerConfigs) {

Review Comment:
   Updated it. Thanks.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: use classic consumer with ZK mode for DeleteOffsetsConsumerGroupCommandIntegrationTest [kafka]

2024-05-06 Thread via GitHub


chia7712 commented on code in PR #15872:
URL: https://github.com/apache/kafka/pull/15872#discussion_r1590841207


##
tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java:
##
@@ -88,7 +113,7 @@ public void testDeleteOffsetsNonExistingGroup() {
 }
 }
 
-@ClusterTest
+@ClusterTemplate("generator")
 public void testDeleteOffsetsOfStableConsumerGroupWithTopicPartition() {
 for (Map consumerConfig: consumerConfigs) {

Review Comment:
   `consumerConfigs` need to be rewritten by 
`ClusterInstance#supportedGroupProtocols`
   
   see https://github.com/apache/kafka/pull/15766/files#r1590222192



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: use classic consumer with ZK mode for DeleteOffsetsConsumerGroupCommandIntegrationTest [kafka]

2024-05-06 Thread via GitHub


chia7712 commented on PR #15872:
URL: https://github.com/apache/kafka/pull/15872#issuecomment-2095735857

   Maybe we should merge `DeleteOffsetsConsumerGroupCommandIntegrationTest` and 
`DeleteConsumerGroupsTest`. They are used to test delete-related commands.


-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org