Re: [PR] KAFKA-15756: [2/3] Migrate existing integration tests to run old protocol in new coordinator [kafka]
dongnuo123 closed pull request #14675: KAFKA-15756: [2/3] Migrate existing integration tests to run old protocol in new coordinator URL: https://github.com/apache/kafka/pull/14675 -- 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] KAFKA-15756: [2/3] Migrate existing integration tests to run old protocol in new coordinator [kafka]
dongnuo123 commented on code in PR #14675: URL: https://github.com/apache/kafka/pull/14675#discussion_r1531051374 ## core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala: ## @@ -89,6 +96,8 @@ class ConsumerBounceTest extends AbstractConsumerTest with Logging { val producer = createProducer() producerSend(producer, numRecords) + this.consumerConfig.setProperty(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG, "3") + Review Comment: Ah thanks for the catch. We shouldn't need it. Let me change it back -- 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] KAFKA-15756: [2/3] Migrate existing integration tests to run old protocol in new coordinator [kafka]
kirktrue commented on code in PR #14675: URL: https://github.com/apache/kafka/pull/14675#discussion_r1529229638 ## core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala: ## @@ -89,6 +96,8 @@ class ConsumerBounceTest extends AbstractConsumerTest with Logging { val producer = createProducer() producerSend(producer, numRecords) + this.consumerConfig.setProperty(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG, "3") + Review Comment: Is this override still needed for the tests to pass? -- 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] KAFKA-15756: [2/3] Migrate existing integration tests to run old protocol in new coordinator [kafka]
dongnuo123 commented on code in PR #14675: URL: https://github.com/apache/kafka/pull/14675#discussion_r1526926450 ## 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: Moved the test to [a new pr](https://github.com/apache/kafka/pull/15547) to unblock this one. Need to fix it later. -- 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