chia7712 commented on code in PR #20002:
URL: https://github.com/apache/kafka/pull/20002#discussion_r2161155100
##########
core/src/test/scala/unit/kafka/server/BrokerRegistrationRequestTest.scala:
##########
@@ -42,12 +43,15 @@ import java.util.concurrent.{CompletableFuture, TimeUnit,
TimeoutException}
class BrokerRegistrationRequestTest {
def brokerToControllerChannelManager(clusterInstance: ClusterInstance):
NodeToControllerChannelManager = {
+ def anyControllerSocketServer(clusterInstance: ClusterInstance):
SocketServer = {
+
clusterInstance.controllers().values().stream().map(_.socketServer).findFirst().get()
+ }
new NodeToControllerChannelManagerImpl(
new ControllerNodeProvider() {
def node: Option[Node] = Some(new Node(
- clusterInstance.anyControllerSocketServer().config.nodeId,
+ anyControllerSocketServer(clusterInstance).config.nodeId,
"127.0.0.1",
-
clusterInstance.anyControllerSocketServer().boundPort(clusterInstance.controllerListenerName()),
+
anyControllerSocketServer(clusterInstance).boundPort(clusterInstance.controllerListenerName()),
Review Comment:
this looks like a bug. It could create a nonexistent node, right? the port
and broker id could be unmatched
##########
core/src/test/scala/unit/kafka/server/ClientQuotasRequestTest.scala:
##########
@@ -217,7 +217,7 @@ class ClientQuotasRequestTest(cluster: ClusterInstance) {
else
InetAddress.getByName(entityName)
var currentServerQuota = 0
- currentServerQuota =
cluster.brokerSocketServers().asScala.head.connectionQuotas.connectionRateForIp(entityIp)
+ currentServerQuota =
cluster.brokers().values().asScala.head.socketServer.connectionQuotas.connectionRateForIp(entityIp)
Review Comment:
Perhaps we could use `Admin#describeClientQuotas` instead of accessing the
internal object.
##########
test-common/test-common-runtime/src/test/java/org/apache/kafka/common/test/junit/ClusterTestExtensionsTest.java:
##########
@@ -189,9 +191,9 @@ public void testClusterTestWithDisksPerBroker() throws
ExecutionException, Inter
@ClusterTest(autoStart = AutoStart.NO)
public void testNoAutoStart() {
- Assertions.assertThrows(RuntimeException.class,
clusterInstance::anyBrokerSocketServer);
+ Assertions.assertThrows(RuntimeException.class, () ->
clusterInstance.brokers().values().stream().map(KafkaBroker::socketServer).findFirst());
clusterInstance.start();
- assertNotNull(clusterInstance.anyBrokerSocketServer());
+
assertTrue(clusterInstance.brokers().values().stream().map(KafkaBroker::socketServer).findFirst().isPresent());
Review Comment:
this is used to verify the initialization, so checking
`clusterInstance.brokers()` is good enough for this test case
--
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
