You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "TaiJuWu (via GitHub)" <gi...@apache.org> on 2025/01/15 14:31:21 UTC
[PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
TaiJuWu opened a new pull request, #18554:
URL: https://github.com/apache/kafka/pull/18554
Jira: https://issues.apache.org/jira/browse/KAFKA-18529
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2599515271
Thanks @kirktrue for good point and suggestion , I will revisit all tests.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2599553545
Revert some change and do follow check:
```
1. Apply @kirktrue patch
2. check all failure use two protocol but there are three exceptions
3. Successful case only run one time
```
three exception is
```
1. testDeleteRecordsAfterCorruptRecords, this like behavior difference but I have not checked totally.
2. testDescribeClassicGroups, this is for classic protocol
3. testListGroups, already include both protocol
```
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1920838118
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -93,10 +93,11 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
ConfigResource.Type.BROKER_LOGGER, brokers.head.config.brokerId.toString)
}
- @ParameterizedTest
+
@Timeout(30)
- @ValueSource(strings = Array("kraft"))
- def testDescribeConfigWithOptionTimeoutMs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeConfigWithOptionTimeoutMs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -140,9 +141,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(configEntries, quotaEntities.get(entity).asScala)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeUserScramCredentials(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeUserScramCredentials(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -566,17 +567,17 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally producer.close()
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testClose(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testClose(quorum: String, groupProtocol: String): Unit = {
val client = createAdminClient
client.close()
client.close() // double close has no effect
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListNodes(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListNodes(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -587,9 +588,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(brokerStrs.mkString(","), nodeStrs.mkString(","))
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListNodesWithFencedBroker(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListNodesWithFencedBroker(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -662,9 +663,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, List(), topics)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDeleteTopicsWithOptionTimeoutMs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDeleteTopicsWithOptionTimeoutMs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -764,9 +765,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertFutureThrows(results.get(nonExistingTopicId), classOf[UnknownTopicIdException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeTopicsWithNames(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeTopicsWithNames(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -114,9 +115,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally brokenClient.close(time.Duration.ZERO)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testCreatePartitionWithOptionRetryOnQuotaViolation(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testCreatePartitionWithOptionRetryOnQuotaViolation(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -801,9 +802,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeLogDirs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeLogDirs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2091,9 +2092,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
/**
* Test the consumer group APIs.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testConsumerGroupWithMemberMigration(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testConsumerGroupWithMemberMigration(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This one does create a `Consumer`, so it's fine as is.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1705,9 +1706,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
futures.foreach(_.get)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testInvalidAlterConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testInvalidAlterConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1669,9 +1670,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(assertThrows(classOf[ExecutionException], () => describeResult2.values.get(invalidTopic).get).getCause.isInstanceOf[InvalidTopicException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncludeDocumentation(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncludeDocumentation(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1062,9 +1063,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
checkValidAlterConfigs(client, this, topicResource1, topicResource2, maxMessageBytes, retentionMs)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterAndDescribeGroupConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterAndDescribeGroupConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -313,9 +314,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally ongoingProducer.close()
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeTransactions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeTransactions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -398,10 +399,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally abortProducer.close()
}
- @ParameterizedTest
@Timeout(10)
- @ValueSource(strings = Array("kraft"))
- def testDescribeTransactionsTimeout(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeTransactionsTimeout(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -566,17 +567,17 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally producer.close()
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testClose(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testClose(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3106,9 +3107,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(electResult.partitions.get.get(unknownTopic).get.isInstanceOf[UnknownTopicOrPartitionException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersWhenNoLiveBrokers(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersWhenNoLiveBrokers(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -413,10 +414,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
- @ParameterizedTest
@Timeout(10)
- @ValueSource(strings = Array("kraft"))
- def testAbortTransactionTimeout(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testAbortTransactionTimeout(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3040,9 +3041,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.assertLeader(client, partition2, broker2)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersForAllPartitions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersForAllPartitions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -428,9 +429,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListTransactions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListTransactions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -688,18 +689,18 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListTopicsWithOptionListInternal(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListTopicsWithOptionListInternal(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3163,9 +3164,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(electResult.partitions.get.get(partition1).get.isInstanceOf[ElectionNotNeededException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersAndNoop(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersAndNoop(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -611,9 +612,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
})
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testAdminClientHandlingBadIPWithoutTimeout(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testAdminClientHandlingBadIPWithoutTimeout(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -626,9 +627,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
client.describeCluster().nodes().get()
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testCreateExistingTopicsThrowTopicExistsException(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testCreateExistingTopicsThrowTopicExistsException(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -675,9 +676,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListTopicsWithOptionTimeoutMs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListTopicsWithOptionTimeoutMs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -643,9 +644,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(e.getCause.isInstanceOf[TopicExistsException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDeleteTopicsWithIds(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDeleteTopicsWithIds(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -714,9 +715,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, List(), testTopics)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeTopicsWithOptionTimeoutMs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeTopicsWithOptionTimeoutMs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -730,9 +731,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
/**
* describe should not auto create topics
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeNonExistingTopic(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeNonExistingTopic(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -779,9 +780,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(existingTopicId, results.get(existingTopic).get.topicId())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeCluster(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeCluster(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -688,18 +689,18 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListTopicsWithOptionListInternal(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListTopicsWithOptionListInternal(quorum: String, groupProtocol: String): Unit = {
client = createAdminClient
val topicNames = client.listTopics(new ListTopicsOptions().listInternal(true)).names().get()
assertFalse(topicNames.isEmpty, "Expected to see internal topics")
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeTopicsWithOptionPartitionSizeLimitPerResponse(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeTopicsWithOptionPartitionSizeLimitPerResponse(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -745,9 +746,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertFutureThrows(results.get(nonExistingTopic), classOf[UnknownTopicOrPartitionException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeTopicsWithIds(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeTopicsWithIds(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2823,9 +2824,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectPreferredLeaders(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectPreferredLeaders(quorum: String, groupProtocol: String): Unit = {
Review Comment:
I don't see that this test creates any `Consumer`s, but it did fail locally.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -963,9 +964,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertInstanceOf(classOf[TimeoutException], brokerLoggerException.getCause)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeConfigsNonexistentForKraft(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeConfigsNonexistentForKraft(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -977,9 +978,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(0, metricResult.entries().size())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeAndAlterConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeAndAlterConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1124,9 +1125,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
"consumer.session.timeout.ms must be greater than or equal to group.consumer.min.session.timeout.ms")
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testCreatePartitions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testCreatePartitions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1479,9 +1480,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForFollowerLog(expectedStartOffset=117L, expectedEndOffset=200L)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testAlterLogDirsAfterDeleteRecords(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testAlterLogDirsAfterDeleteRecords(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -942,9 +943,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeConfigsNonexistent(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeConfigsNonexistent(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -833,9 +834,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeReplicaLogDirs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeReplicaLogDirs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1649,9 +1650,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(classOf[OffsetOutOfRangeException], cause.getClass)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeConfigsForTopic(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeConfigsForTopic(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1770,9 +1771,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
* Check that a call with a timeout does not complete before the minimum timeout has elapsed,
* even when the default request timeout is shorter.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testMinimumRequestTimeouts(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testMinimumRequestTimeouts(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1430,9 +1431,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(3, brokers(i).replicaManager.localLog(topicPartition).get.logStartOffset)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1788,9 +1789,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
/**
* Test injecting timeouts for calls that are in flight.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testCallInFlightTimeouts(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testCallInFlightTimeouts(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1717,9 +1718,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
* Also see [[kafka.api.SaslSslAdminIntegrationTest.testAclOperations()]] for tests of ACL operations
* when the authorizer is enabled.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testAclOperations(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testAclOperations(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3586,9 +3587,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertFutureThrows(invalidReplicaResult.get(tp3), classOf[InvalidReplicaAssignmentException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testLongTopicNames(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testLongTopicNames(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3606,9 +3607,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}
// Verify that createTopics and alterConfigs fail with null values
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testNullConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testNullConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2972,9 +2973,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.assertLeader(client, partition2, 2)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersForOnePartition(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersForOnePartition(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3425,9 +3426,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}, "Expected to see the broker properties we just modified", pause=25)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsDeleteBrokerConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsDeleteBrokerConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1734,9 +1735,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
* Test closing the AdminClient with a generous timeout. Calls in progress should be completed,
* since they can be done within the timeout. New calls should receive exceptions.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDelayedClose(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDelayedClose(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3391,9 +3392,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(appendValues, configs.get(topicResource).get(QuotaConfig.LEADER_REPLICATION_THROTTLED_REPLICAS_CONFIG).value)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsDeleteAndSetBrokerConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsDeleteAndSetBrokerConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3667,9 +3668,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(clusterReplicaLogLevel.synonyms().isEmpty)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsForLog4jLogLevels(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsForLog4jLogLevels(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3803,9 +3804,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(assertThrows(classOf[ExecutionException], () => alterBrokerLoggers(deleteRootLoggerEntry)).getCause.isInstanceOf[InvalidRequestException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsForLog4jLogLevelsDoesNotWorkWithInvalidConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsForLog4jLogLevelsDoesNotWorkWithInvalidConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3650,9 +3651,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
validateLogConfig(compressionType = "producer")
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testDescribeConfigsForLog4jLogLevels(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testDescribeConfigsForLog4jLogLevels(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3220,9 +3221,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(0, allReassignmentsMap.size())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListReassignmentsDoesNotShowDeletedPartitions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListReassignmentsDoesNotShowDeletedPartitions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3918,9 +3919,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
* Note: this test requires some custom static broker and controller configurations, which are set up in
* BaseAdminIntegrationTest.modifyConfigs.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testCreateTopicsReturnsConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testCreateTopicsReturnsConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3080,9 +3081,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.assertLeader(client, partition2, broker3)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersForUnknownPartitions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersForUnknownPartitions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -1752,9 +1753,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
* Test closing the AdminClient with a timeout of 0, when there are calls with extremely long
* timeouts in progress. The calls should be aborted after the hard shutdown timeout elapses.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testForceClose(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testForceClose(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3462,9 +3463,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
}, "Expected to see the broker properties we just removed to be deleted", pause=25)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testInvalidIncrementalAlterConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testInvalidIncrementalAlterConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3350,9 +3351,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
"Invalid value zip for configuration compression.type: String must be one of: uncompressed, zstd, lz4, snappy, gzip, producer")
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testAppendAlreadyExistsConfigsAndSubtractNotExistsConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testAppendAlreadyExistsConfigsAndSubtractNotExistsConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3203,9 +3204,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.assertLeader(client, partition2, broker3)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListReassignmentsDoesNotShowNonReassigningPartitions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListReassignmentsDoesNotShowNonReassigningPartitions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3546,9 +3547,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
"Invalid value 1.1 for configuration min.cleanable.dirty.ratio: Value must be no more than 1")
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testInvalidAlterPartitionReassignments(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testInvalidAlterPartitionReassignments(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3850,14 +3851,14 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
def describeBrokerLoggers(): Config =
client.describeConfigs(Collections.singletonList(brokerLoggerConfigResource)).values.get(brokerLoggerConfigResource).get()
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
def testAppendConfigToEmptyDefaultValue(ignored: String): Unit = {
testAppendConfig(new Properties(), "0:0", "0:0")
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3728,9 +3729,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
* 4. Change kafka logger to ERROR
* 5. Ensure the kafka.server.ControllerServer logger's level is ERROR (the current kafka logger level)
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsForLog4jLogLevelsCanResetLoggerToCurrentRoot(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsForLog4jLogLevelsCanResetLoggerToCurrentRoot(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3000,9 +3001,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.assertLeader(client, partition1, broker2)
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersForManyPartitions(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersForManyPartitions(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3850,14 +3851,14 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
def describeBrokerLoggers(): Config =
client.describeConfigs(Collections.singletonList(brokerLoggerConfigResource)).values.get(brokerLoggerConfigResource).get()
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3771,9 +3772,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(newAncestorLogLevel, newAncestorLoggerConfig.get("kafka.server.ControllerServer").value())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsForLog4jLogLevelsCanSetToRootLogger(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsForLog4jLogLevelsCanSetToRootLogger(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3135,9 +3136,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertTrue(electResult.partitions.get.get(partition1).get.isInstanceOf[EligibleLeadersNotAvailableException])
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testElectUncleanLeadersNoop(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testElectUncleanLeadersNoop(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3235,9 +3236,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(0, allReassignmentsMap.size())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testValidIncrementalAlterConfigs(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testValidIncrementalAlterConfigs(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3792,9 +3793,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(newRootLogLevel, changedRootLoggerConfig.get(Log4jController.ROOT_LOGGER).value())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testIncrementalAlterConfigsForLog4jLogLevelsCannotResetRootLogger(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testIncrementalAlterConfigsForLog4jLogLevelsCannotResetRootLogger(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3882,9 +3883,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
assertEquals(expected, config.value())
}
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testListClientMetricsResources(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testListClientMetricsResources(quorum: String, groupProtocol: String): Unit = {
Review Comment:
This test does not create any `Consumer`s, so this change should be reverted.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2599308345
They do by using the `createConsumer` and directly` new KafkaConsumer` (looking for those will show the usages).
That being said, good point that in the admin there seems to be some using only admin/producer, so we could review and clean up a bit if the consumer grp param if not needed. Thanks for pointing it out @kirktrue! Could you take a look @TaiJuWu ? 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1918826202
##########
core/src/test/scala/integration/kafka/api/ConsumerRebootstrapTest.scala:
##########
@@ -135,12 +135,13 @@ object ConsumerRebootstrapTest {
final val RebootstrapTestName = s"${TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames}.useRebootstrapTriggerMs={2}"
def rebootstrapTestParams: stream.Stream[Arguments] = {
- assertEquals(1, getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly.count())
- val args = getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly
- .findFirst().get.get
- stream.Stream.of(
- Arguments.of((args :+ true):_*),
- Arguments.of((args :+ false):_*)
- )
+ assertEquals(2, getTestQuorumAndGroupProtocolParametersAll.count())
Review Comment:
Thanks, remove it.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1918331389
##########
core/src/test/scala/integration/kafka/api/ConsumerRebootstrapTest.scala:
##########
@@ -135,12 +135,13 @@ object ConsumerRebootstrapTest {
final val RebootstrapTestName = s"${TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames}.useRebootstrapTriggerMs={2}"
def rebootstrapTestParams: stream.Stream[Arguments] = {
- assertEquals(1, getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly.count())
- val args = getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly
- .findFirst().get.get
- stream.Stream.of(
- Arguments.of((args :+ true):_*),
- Arguments.of((args :+ false):_*)
- )
+ assertEquals(2, getTestQuorumAndGroupProtocolParametersAll.count())
Review Comment:
Should we still need this assertion?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1924361968
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
To address your fourth question, yes, ideally we should update the Jira and PR to accurately reflect what's changing.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1924158432
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
agree with this change, but interesting, this shows that `PlainTestAdminIntegration` was already running for async, even in cases where it was not needed, right?
So checking all these admin tests that were running for both consumers, I notice that `testLogStartOffsetCheckpoint` is somehow similar to this one, should we apply the same fix to it?
It's running for both but it really only uses a consumer to `subscribeAndWaitForAssignment`, which is already covered in many consumer-specific tests, is it valuable in this testLogStartOffsetCheckpoint? (seems unrelated). What do you think @kirktrue ? I see you enabled it for both consumers as part of https://github.com/apache/kafka/pull/17670.
Also should we rename the jira/PR? It's only the ConsumerRebotstrap that was not running for async
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2593637360
Thanks for the patch @TaiJuWu ! Could you please merge trunk latest changes? 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1917209707
##########
core/src/test/scala/integration/kafka/api/ConsumerRebootstrapTest.scala:
##########
@@ -135,8 +135,8 @@ object ConsumerRebootstrapTest {
final val RebootstrapTestName = s"${TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames}.useRebootstrapTriggerMs={2}"
def rebootstrapTestParams: stream.Stream[Arguments] = {
- assertEquals(1, getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly.count())
- val args = getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly
+ assertEquals(2, getTestQuorumAndGroupProtocolParametersAll.count())
+ val args = getTestQuorumAndGroupProtocolParametersAll
.findFirst().get.get
Review Comment:
if we're only keeping the first entry from the full protocol list we still won't be testing with both consumers right?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2599348638
To test whether or not a test needed to be updated, I changed the code that creates a `Consumer`:
```diff
diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerDelegateCreator.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerDelegateCreator.java
index 74592972b9..fe70aebcee 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerDelegateCreator.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerDelegateCreator.java
@@ -56,6 +56,9 @@ public class ConsumerDelegateCreator {
public <K, V> ConsumerDelegate<K, V> create(ConsumerConfig config,
Deserializer<K> keyDeserializer,
Deserializer<V> valueDeserializer) {
+ if (true)
+ throw new RuntimeException("Intentionally blocking attempt to create a Consumer!");
+
try {
GroupProtocol groupProtocol = GroupProtocol.valueOf(config.getString(ConsumerConfig.GROUP_PROTOCOL_CONFIG).toUpperCase(Locale.ROOT));
@@ -79,6 +82,9 @@ public class ConsumerDelegateCreator {
SubscriptionState subscriptions,
ConsumerMetadata metadata,
List<ConsumerPartitionAssignor> assignors) {
+ if (true)
+ throw new RuntimeException("Intentionally blocking attempt to create a Consumer!");
+
try {
GroupProtocol groupProtocol = GroupProtocol.valueOf(config.getString(ConsumerConfig.GROUP_PROTOCOL_CONFIG).toUpperCase(Locale.ROOT));
```
Then I ran `PlaintextAdminIntegrationTest` locally.
Are there other changes in the queue that are going to start creating `Consumer`s in these tests?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1927342051
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
agree, that could be a safe start.
Also, it could make sense to start by reviewing Admin and Producer tests that are running for both consumers. It's probably on those where we can have some good candidates to rollback.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1918791042
##########
core/src/test/scala/integration/kafka/api/ConsumerRebootstrapTest.scala:
##########
@@ -135,12 +135,13 @@ object ConsumerRebootstrapTest {
final val RebootstrapTestName = s"${TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames}.useRebootstrapTriggerMs={2}"
def rebootstrapTestParams: stream.Stream[Arguments] = {
- assertEquals(1, getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly.count())
- val args = getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly
- .findFirst().get.get
- stream.Stream.of(
- Arguments.of((args :+ true):_*),
- Arguments.of((args :+ false):_*)
- )
+ assertEquals(2, getTestQuorumAndGroupProtocolParametersAll.count())
Review Comment:
makes sense to me to remove it, this test shouldn't care about the elements in the list, just make sure we use them all right? (this is the only comment left here imo @TaiJuWu, please take a look when you can and I'll take another look). Thanks!
##########
core/src/test/scala/integration/kafka/api/ConsumerRebootstrapTest.scala:
##########
@@ -135,12 +135,13 @@ object ConsumerRebootstrapTest {
final val RebootstrapTestName = s"${TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames}.useRebootstrapTriggerMs={2}"
def rebootstrapTestParams: stream.Stream[Arguments] = {
- assertEquals(1, getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly.count())
- val args = getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly
- .findFirst().get.get
- stream.Stream.of(
- Arguments.of((args :+ true):_*),
- Arguments.of((args :+ false):_*)
- )
+ assertEquals(2, getTestQuorumAndGroupProtocolParametersAll.count())
Review Comment:
makes sense to me to remove it, this test shouldn't care about the elements in the list, just make sure we use them all right? (this is the only comment left here imo @TaiJuWu, please take a look when you can and I'll take another look too). 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1924360209
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
Yes, at least one other committer has pushed back on enabling both consumer implementations anywhere that a consumer is created. I filed KAFKA-18087 to remedy that, but I haven't yet thought about the heuristic to apply to know when it's "safe" to just test the one consumer or the other.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm merged PR #18554:
URL: https://github.com/apache/kafka/pull/18554
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2599003570
> Hey @TaiJuWu, could you please merge trunk latest changes? (some changes just went in on these files). Thanks!
Done :grinning:
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2599471680
I agree with @kirktrue's suggestion that some tests do not utilize a consumer. Therefore, running these tests twice is redundant.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1920980944
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2091,9 +2092,9 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
/**
* Test the consumer group APIs.
*/
- @ParameterizedTest
- @ValueSource(strings = Array("kraft"))
- def testConsumerGroupWithMemberMigration(quorum: String): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testConsumerGroupWithMemberMigration(quorum: String, groupProtocol: String): Unit = {
Review Comment:
After check all test, I notice this comment :)
Will keep it.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1924158432
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
agree with this change, but interesting, this shows that `PlainTestAdminIntegration` was already running for async in cases where it was not needed, right?
So checking all these admin tests that were running for both consumers, I notice that `testLogStartOffsetCheckpoint` is somehow similar to this one, should we apply the same fix to it?
It's running for both but it really only uses a consumer to `subscribeAndWaitForAssignment`, which is already covered in many consumer-specific tests, is it valuable in this testLogStartOffsetCheckpoint? (seems unrelated). What do you think @kirktrue ? I see you enabled it for both consumers as part of https://github.com/apache/kafka/pull/17670.
Also should we rename the jira/PR? It's only the ConsumerRebotstrap that was not running for async
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1924699832
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
Rename jira and PR title.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2611613480
> @TaiJuWu could you merge trunk latest changes to get rid of the test failures we have here? (ex. fixed in #18612). Thanks!
Done :)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1926899717
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
> but I haven't yet thought about the heuristic to apply to know when it's "safe" to just test the one consumer or the other.
we are on the same page :)
It's difficult to definitively confirm that "this functionality of the consumer is already covered by another test." Therefore, a simpler approach is to test both consumers if the test requires the use of a consumer.
Maybe we can revisit only the "slow" tests?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1927342051
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -207,10 +207,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
Admin.create(config)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
- @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
@Timeout(10)
- def testDescribeUserScramCredentialsTimeout(quorum: String, groupProtocol: String): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
Review Comment:
agree, that could be a safe start.
Also, it could make sense to start by reviewing Admin and Producer tests that are running for both consumers. There it's probably where we can have some candidates to rollback.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2610413702
@TaiJuWu could you merge trunk latest changes to get rid of the test failures we have here? (ex. fixed in https://github.com/apache/kafka/pull/18612). 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2598683809
Hey @TaiJuWu, could you please merge trunk latest changes? (some changes just went in on these files). 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18554:
URL: https://github.com/apache/kafka/pull/18554#discussion_r1917883443
##########
core/src/test/scala/integration/kafka/api/ConsumerRebootstrapTest.scala:
##########
@@ -135,8 +135,8 @@ object ConsumerRebootstrapTest {
final val RebootstrapTestName = s"${TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames}.useRebootstrapTriggerMs={2}"
def rebootstrapTestParams: stream.Stream[Arguments] = {
- assertEquals(1, getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly.count())
- val args = getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly
+ assertEquals(2, getTestQuorumAndGroupProtocolParametersAll.count())
+ val args = getTestQuorumAndGroupProtocolParametersAll
.findFirst().get.get
Review Comment:
Thanks for catching it, update!
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18529: PlaintextAdminIntegrationTest and ConsumerRebootstrapTest should run for async consumer [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on PR #18554:
URL: https://github.com/apache/kafka/pull/18554#issuecomment-2593689093
> Thanks for the patch @TaiJuWu ! Could you please merge trunk latest changes? Thanks!
Done~~~~~~
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org