dajac commented on code in PR #14781:
URL: https://github.com/apache/kafka/pull/14781#discussion_r1396123024
##########
core/src/test/scala/integration/kafka/api/BaseAsyncConsumerTest.scala:
##########
@@ -28,8 +30,9 @@ import scala.jdk.CollectionConverters._
class BaseAsyncConsumerTest extends AbstractConsumerTest {
val defaultBlockingAPITimeoutMs = 1000
- @Test
- def testCommitAsync(): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft", "kraft+kip848"))
Review Comment:
We don't need `zk` here.
##########
core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala:
##########
@@ -464,7 +477,7 @@ class ConsumerBounceTest extends AbstractConsumerTest with
Logging {
private def submitCloseAndValidate(consumer: Consumer[Array[Byte],
Array[Byte]],
closeTimeoutMs: Long, minCloseTimeMs: Option[Long], maxCloseTimeMs:
Option[Long]): Future[Any] = {
executor.submit(() => {
- val closeGraceTimeMs = 2000
+ val closeGraceTimeMs = 10000
Review Comment:
I suppose that we increase it. However, it is weird that the cluster type
has an impact here.
##########
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##########
@@ -1805,16 +1865,19 @@ class PlaintextConsumerTest extends BaseConsumerTest {
s"The current assignment is ${consumer.assignment()}")
}
- @Test
- def testConsumingWithNullGroupId(): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft", "kraft+kip848"))
+ def testConsumingWithNullGroupId(quorum: String): Unit = {
val topic = "test_topic"
val partition = 0;
val tp = new TopicPartition(topic, partition)
createTopic(topic, 1, 1)
- TestUtils.waitUntilTrue(() => {
- this.zkClient.topicExists(topic)
- }, "Failed to create topic")
+ if (!isKRaftTest()) {
+ TestUtils.waitUntilTrue(() => {
+ this.zkClientOrNull.topicExists(topic)
+ }, "Failed to create topic")
+ }
Review Comment:
We could perhaps use `TestUtils.createTopicWithAdmin`. I would take a look
at how we migrated other tests to KRaft.
##########
core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala:
##########
@@ -77,8 +83,9 @@ class ConsumerBounceTest extends AbstractConsumerTest with
Logging {
}
}
- @Test
- def testConsumptionWithBrokerFailures(): Unit = consumeWithBrokerFailures(10)
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft", "kraft+kip848"))
+ def testConsumptionWithBrokerFailures(quorum: String): Unit =
consumeWithBrokerFailures(10)
Review Comment:
Any idea why? Is it only this one or others are well?
##########
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##########
@@ -547,7 +565,7 @@ class PlaintextConsumerTest extends BaseConsumerTest {
assertEquals(2, parts.size)
}
- @Test
+ @Test // TODO: doesn't pass for kraft and kraft+kip848
Review Comment:
I suppose that we have to wait on metadata propagation here.
##########
core/src/test/scala/integration/kafka/api/SaslMultiMechanismConsumerTest.scala:
##########
@@ -41,8 +43,9 @@ class SaslMultiMechanismConsumerTest extends BaseConsumerTest
with SaslSetup {
closeSasl()
}
- @Test
- def testMultipleBrokerMechanisms(): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft", "kraft+kip848"))
+ def testMultipleBrokerMechanisms(quorum: String): Unit = {
Review Comment:
Do we need to migrate it now or could we do it separately?
##########
core/src/test/scala/integration/kafka/api/BaseAsyncConsumerTest.scala:
##########
@@ -51,8 +54,9 @@ class BaseAsyncConsumerTest extends AbstractConsumerTest {
assertTrue(consumer.assignment.contains(tp))
}
- @Test
- def testCommitSync(): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft", "kraft+kip848"))
Review Comment:
ditto.
--
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
