AndrewJSchofield commented on code in PR #14758:
URL: https://github.com/apache/kafka/pull/14758#discussion_r1393365674


##########
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##########
@@ -1657,35 +1759,37 @@ class PlaintextConsumerTest extends BaseConsumerTest {
     assertEquals(numMessages - records.count, 
lag.metricValue.asInstanceOf[Double], epsilon, s"The lag should be 
${numMessages - records.count}")
   }
 
-  @Test
-  def testQuotaMetricsNotCreatedIfNoQuotasConfigured(): Unit = {
+  @ParameterizedTest
+  @ValueSource(strings = Array("generic", "consumer"))
+  def testQuotaMetricsNotCreatedIfNoQuotasConfigured(groupProtocol: String): 
Unit = {
     val numRecords = 1000
     val producer = createProducer()
     val startingTimestamp = System.currentTimeMillis()
     sendRecords(producer, numRecords, tp, startingTimestamp = 
startingTimestamp)
 
+    this.consumerConfig.setProperty(ConsumerConfig.GROUP_PROTOCOL_CONFIG, 
groupProtocol)
     val consumer = createConsumer()
     consumer.assign(List(tp).asJava)
     consumer.seek(tp, 0)
     consumeAndVerifyRecords(consumer = consumer, numRecords = numRecords, 
startingOffset = 0, startingTimestamp = startingTimestamp)
 
-    def assertNoMetric(broker: KafkaServer, name: String, quotaType: 
QuotaType, clientId: String): Unit = {
-        val metricName = broker.metrics.metricName("throttle-time",
-                                  quotaType.toString,
-                                  "",
-                                  "user", "",
-                                  "client-id", clientId)
-        assertNull(broker.metrics.metric(metricName), "Metric should not have 
been created " + metricName)
+    def assertNoMetric(broker: KafkaBroker, name: String, quotaType: 
QuotaType, clientId: String): Unit = {
+      val metricName = broker.metrics.metricName("throttle-time",
+        quotaType.toString,
+        "",
+        "user", "",
+        "client-id", clientId)
+      assertNull(broker.metrics.metric(metricName), "Metric should not have 
been created " + metricName)
     }
-    servers.foreach(assertNoMetric(_, "byte-rate", QuotaType.Produce, 
producerClientId))
-    servers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Produce, 
producerClientId))
-    servers.foreach(assertNoMetric(_, "byte-rate", QuotaType.Fetch, 
consumerClientId))
-    servers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Fetch, 
consumerClientId))
+    brokers.foreach(assertNoMetric(_, "byte-rate", QuotaType.Produce, 
producerClientId))
+    brokers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Produce, 
producerClientId))
+    brokers.foreach(assertNoMetric(_, "byte-rate", QuotaType.Fetch, 
consumerClientId))
+    brokers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Fetch, 
consumerClientId))
 
-    servers.foreach(assertNoMetric(_, "request-time", QuotaType.Request, 
producerClientId))
-    servers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Request, 
producerClientId))
-    servers.foreach(assertNoMetric(_, "request-time", QuotaType.Request, 
consumerClientId))
-    servers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Request, 
consumerClientId))
+    brokers.foreach(assertNoMetric(_, "request-time", QuotaType.Request, 
producerClientId))
+    brokers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Request, 
producerClientId))
+    brokers.foreach(assertNoMetric(_, "request-time", QuotaType.Request, 
consumerClientId))
+    brokers.foreach(assertNoMetric(_, "throttle-time", QuotaType.Request, 
consumerClientId))

Review Comment:
   In this context, `servers` and `brokers` are interchangeable. This change 
makes the test work for ZK or KRaft. Previously, it was ZK-only.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to