(kafka) branch trunk updated: MINOR: Fix unstable sorting in AssignmentsManagerTest (#14794)

[dajac]

This is an automated email from the ASF dual-hosted git repository.

dajac pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 9e884bcff5a MINOR: Fix unstable sorting in AssignmentsManagerTest 
(#14794)
9e884bcff5a is described below

commit 9e884bcff5a9cac8b49c4d3b783c7eedf60feff1
Author: Igor Soarez <soa...@apple.com>
AuthorDate: Mon Nov 20 15:31:45 2023 +0000

    MINOR: Fix unstable sorting in AssignmentsManagerTest (#14794)
    
    Building AssignReplicasToDirsRequestData relies on iteration over Map 
entries, which can result in different sorting order. The order does not matter 
to the semantics of the request, but it can cause issues with test assertions. 
This issue was introduced in #14369.
    
    Reviewers: Divij Vaidya <di...@amazon.com>, Ismael Juma 
<ism...@juma.me.uk>, David Jacot <dja...@confluent.io>
---
 .../kafka/server/AssignmentsManagerTest.java       | 30 ++++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git 
a/server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java 
b/server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java
index 01f9b40878e..1ddbdc4d729 100644
--- a/server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java
+++ b/server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java
@@ -31,12 +31,11 @@ import org.apache.kafka.server.common.TopicIdPartition;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.condition.DisabledOnJre;
-import org.junit.jupiter.api.condition.JRE;
 import org.mockito.ArgumentCaptor;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
@@ -74,6 +73,22 @@ public class AssignmentsManagerTest {
         manager.close();
     }
 
+    AssignReplicasToDirsRequestData normalize(AssignReplicasToDirsRequestData 
request) {
+        request = request.duplicate();
+        
request.directories().sort(Comparator.comparing(AssignReplicasToDirsRequestData.DirectoryData::id));
+        for (AssignReplicasToDirsRequestData.DirectoryData directory : 
request.directories()) {
+            
directory.topics().sort(Comparator.comparing(AssignReplicasToDirsRequestData.TopicData::topicId));
+            for (AssignReplicasToDirsRequestData.TopicData topic : 
directory.topics()) {
+                
topic.partitions().sort(Comparator.comparing(AssignReplicasToDirsRequestData.PartitionData::partitionIndex));
+            }
+        }
+        return request;
+    }
+
+    void assertRequestEquals(AssignReplicasToDirsRequestData expected, 
AssignReplicasToDirsRequestData actual) {
+        assertEquals(normalize(expected), normalize(actual));
+    }
+
     @Test
     void testBuildRequestData() {
         Map<TopicIdPartition, Uuid> assignment = new HashMap<TopicIdPartition, 
Uuid>() {{
@@ -127,11 +142,10 @@ public class AssignmentsManagerTest {
                                                 ))
                                 ))
                 ));
-        assertEquals(expected, built);
+        assertRequestEquals(expected, built);
     }
 
     @Test
-    @DisabledOnJre(JRE.JAVA_8)
     public void testAssignmentAggregation() throws InterruptedException {
         CountDownLatch readyToAssert = new CountDownLatch(1);
         doAnswer(invocation -> {
@@ -165,7 +179,7 @@ public class AssignmentsManagerTest {
                         put(new TopicIdPartition(TOPIC_2, 5), DIR_2);
                     }}
         );
-        assertEquals(expected, actual);
+        assertRequestEquals(expected, actual);
     }
 
     @Test
@@ -214,18 +228,18 @@ public class AssignmentsManagerTest {
         verify(channelManager, times(5)).sendRequest(captor.capture(), 
any(ControllerRequestCompletionHandler.class));
         verifyNoMoreInteractions(channelManager);
         assertEquals(5, captor.getAllValues().size());
-        assertEquals(AssignmentsManager.buildRequestData(
+        assertRequestEquals(AssignmentsManager.buildRequestData(
                 8, 100L, new HashMap<TopicIdPartition, Uuid>() {{
                         put(new TopicIdPartition(TOPIC_1, 1), DIR_1);
                     }}
         ), captor.getAllValues().get(0).build().data());
-        assertEquals(AssignmentsManager.buildRequestData(
+        assertRequestEquals(AssignmentsManager.buildRequestData(
                 8, 100L, new HashMap<TopicIdPartition, Uuid>() {{
                         put(new TopicIdPartition(TOPIC_1, 1), DIR_1);
                         put(new TopicIdPartition(TOPIC_1, 2), DIR_3);
                     }}
         ), captor.getAllValues().get(1).build().data());
-        assertEquals(AssignmentsManager.buildRequestData(
+        assertRequestEquals(AssignmentsManager.buildRequestData(
                 8, 100L, new HashMap<TopicIdPartition, Uuid>() {{
                         put(new TopicIdPartition(TOPIC_1, 1), DIR_1);
                         put(new TopicIdPartition(TOPIC_1, 2), DIR_3);