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