jsancio commented on code in PR #16563: URL: https://github.com/apache/kafka/pull/16563#discussion_r1674382183
########## raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientTest.java: ########## @@ -281,15 +281,20 @@ public void testGrantVotesWhenShuttingDown(boolean withKip853Rpc) throws Excepti context.client.poll(); // We will first transition to unattached and then grant vote and then transition to voted - assertTrue(context.client.quorum().isVoted()); + assertTrue( + context.client.quorum().isVoted(), + "Local Id: " + localId + + " Remote Id: " + remoteId + + " Quorum local Id: " + context.client.quorum().localIdOrSentinel() + + " Quorum leader Id: " + context.client.quorum().leaderIdOrSentinel()); Review Comment: How about: ```java "Local Id: " + localId + " Remote Id: " + remoteId + " Quorum local Id: " + context.client.quorum().localIdOrSentinel() + " Quorum leader Id: " + context.client.quorum().leaderIdOrSentinel() ); ``` ########## raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientSnapshotTest.java: ########## @@ -1977,12 +1978,20 @@ private static ReplicaKey replicaKey(int id, boolean withDirectoryId) { return ReplicaKey.of(id, directoryId); } + private static Integer randomReplicaId() { + int mockAddressPrefix = 9990; + // Number of nodes we can set up if we got a random number that is maximum in the range + int reservedNumberOfPorts = 50; + int maxPort = 65535 - mockAddressPrefix - reservedNumberOfPorts; + return ThreadLocalRandom.current().nextInt(maxPort); + } Review Comment: Let's return an `int` instead of the object `Integer`: `private static int randomReplicaId()`. The variable names `mockAddressPrefix` and `reserverdNumberOfPorts` are misleading this id has nothing to do with those concepts. If you want to limit the range of the integer, I am okay with generating a random id from 0 to 1024. ########## raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientTest.java: ########## @@ -3860,12 +3865,11 @@ private static ReplicaKey replicaKey(int id, boolean withDirectoryId) { return ReplicaKey.of(id, directoryId); } - private static Integer getRandomPort() { - int minPort = 1024; + private static Integer randomReplicaId() { int mockAddressPrefix = 9990; // Number of nodes we can set up if we got a random number that is maximum in the range int reservedNumberOfPorts = 50; int maxPort = 65535 - mockAddressPrefix - reservedNumberOfPorts; Review Comment: Let's return an `int` instead of the object `Integer`: `private static int randomReplicaId()`. The variable names `mockAddressPrefix` and `reserverdNumberOfPorts` are misleading this id has nothing to do with those concepts. If you want to limit the range of the integer, I am okay with generating a random id from 0 to 1024. -- 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