[GitHub] [kafka] vvcephei commented on a change in pull request #8541: KAFKA-6145: KIP-441: Add TaskAssignor class config

[GitBox]

vvcephei commented on a change in pull request #8541:
URL: https://github.com/apache/kafka/pull/8541#discussion_r416916295



##########
File path: 
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java
##########
@@ -41,132 +54,107 @@
 import static org.easymock.EasyMock.replay;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.UUID;
-import org.apache.kafka.streams.processor.TaskId;
-import 
org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs;
-import org.easymock.EasyMock;
-import org.junit.Test;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 
 public class HighAvailabilityTaskAssignorTest {
-    private long acceptableRecoveryLag = 100L;
-    private int balanceFactor = 1;
-    private int maxWarmupReplicas = 2;
-    private int numStandbyReplicas = 0;
-    private long probingRebalanceInterval = 60 * 1000L;
-
-    private Map<UUID, ClientState> clientStates = new HashMap<>();
-    private Set<TaskId> allTasks = new HashSet<>();
-    private Set<TaskId> statefulTasks = new HashSet<>();
-
-    private ClientState client1;
-    private ClientState client2;
-    private ClientState client3;
-    
-    private HighAvailabilityTaskAssignor taskAssignor;
-
-    private void createTaskAssignor() {
-        final AssignmentConfigs configs = new AssignmentConfigs(
-            acceptableRecoveryLag,
-            balanceFactor,
-            maxWarmupReplicas,
-            numStandbyReplicas,
-            probingRebalanceInterval
-        );
-        taskAssignor = new HighAvailabilityTaskAssignor(
-            clientStates,
-            allTasks,
-            statefulTasks,
-            configs);
-    }
+    private final AssignmentConfigs configWithoutStandbys = new 
AssignmentConfigs(
+        /*acceptableRecoveryLag*/ 100L,
+        /*balanceFactor*/ 1,
+        /*maxWarmupReplicas*/ 2,
+        /*numStandbyReplicas*/ 0,
+        /*probingRebalanceIntervalMs*/ 60 * 1000L
+    );
+
+    private final AssignmentConfigs configWithStandbys = new AssignmentConfigs(
+        /*acceptableRecoveryLag*/ 100L,
+        /*balanceFactor*/ 1,
+        /*maxWarmupReplicas*/ 2,
+        /*numStandbyReplicas*/ 1,
+        /*probingRebalanceIntervalMs*/ 60 * 1000L
+    );
 
-    @Test
-    public void 
shouldDecidePreviousAssignmentIsInvalidIfThereAreUnassignedActiveTasks() {
-        client1 = EasyMock.createNiceMock(ClientState.class);
-        expect(client1.prevActiveTasks()).andReturn(singleton(TASK_0_0));
-        expect(client1.prevStandbyTasks()).andStubReturn(EMPTY_TASKS);
-        replay(client1);
-        allTasks =  mkSet(TASK_0_0, TASK_0_1);
-        clientStates = singletonMap(UUID_1, client1);
-        createTaskAssignor();
 
-        assertFalse(taskAssignor.previousAssignmentIsValid());

Review comment:
       Since you have a follow-on PR that touches this method, I'll leave it 
alone and just proceed to merge. We should consider both of these options in 
the follow-on.
   
   Thanks!




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org