You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "harmadasg (via GitHub)" <gi...@apache.org> on 2026/08/28 00:22:26 UTC

[PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

harmadasg opened a new pull request, #23291:
URL: https://github.com/apache/kafka/pull/23291

   Migrate usages of hamcrest to junit


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #23291:
URL: https://github.com/apache/kafka/pull/23291


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #23291:
URL: https://github.com/apache/kafka/pull/23291#issuecomment-5494989222

   ```
   Execution failed for task ':streams:spotlessJavaCheck' (registered in build file 'build.gradle').
   > The following files had format violations:
         src/test/java/org/apache/kafka/streams/processor/internals/assignment/GraphTest.java
             @@ -25,9 +25,9 @@
              import·java.util.Map;
              import·java.util.Set;
              
             -import·static·org.junit.jupiter.api.Assertions.assertIterableEquals;
              import·static·org.junit.jupiter.api.Assertions.assertEquals;
              import·static·org.junit.jupiter.api.Assertions.assertFalse;
             +import·static·org.junit.jupiter.api.Assertions.assertIterableEquals;
              import·static·org.junit.jupiter.api.Assertions.assertSame;
              import·static·org.junit.jupiter.api.Assertions.assertThrows;
              import·static·org.junit.jupiter.api.Assertions.assertTrue;
     Run './gradlew spotlessApply' to fix all violations.
   ```
   @harmadasg please fix build failure


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "harmadasg (via GitHub)" <gi...@apache.org>.
harmadasg commented on PR #23291:
URL: https://github.com/apache/kafka/pull/23291#issuecomment-5483799294

   @chia7712 thanks for checking the PR, I have addressed the review items


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "harmadasg (via GitHub)" <gi...@apache.org>.
harmadasg commented on PR #23291:
URL: https://github.com/apache/kafka/pull/23291#issuecomment-5495558827

   @chia7712 apologies, just pushed the fix. also added a local commit hook to catch such errors in the future before sending for review


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #23291:
URL: https://github.com/apache/kafka/pull/23291#issuecomment-5518965238

   @harmadasg could you check the failing tests in KafkaProducerTest?


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23291:
URL: https://github.com/apache/kafka/pull/23291#discussion_r3897354318


##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfigurationTest.java:
##########
@@ -46,12 +44,11 @@ public void setup() {
 
     @Test
     public void configsShouldRejectZeroWarmups() {
-        final ConfigException exception = assertThrows(
+        assertThrows(
             ConfigException.class,
-            () -> new AssignmentConfigs(1L, 0, 1, 1L, EMPTY_RACK_AWARE_ASSIGNMENT_TAGS)
+            () -> new AssignmentConfigs(1L, 0, 1, 1L, EMPTY_RACK_AWARE_ASSIGNMENT_TAGS),
+            "Invalid value 0 for configuration max.warmup.replicas"

Review Comment:
   ditto - we should verify the error message



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "harmadasg (via GitHub)" <gi...@apache.org>.
harmadasg commented on PR #23291:
URL: https://github.com/apache/kafka/pull/23291#issuecomment-5454181361

   @m1a2st Thanks for checking the PR. The review items have been addressed. In the same spirit I have changed other code parts in the `org.apache.kafka.streams.processor.internals.assignment` package to take advantage of the immutable collection factory methods. Lastly I have also resolved the conflict from https://github.com/apache/kafka/pull/23279


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #23291:
URL: https://github.com/apache/kafka/pull/23291#discussion_r3878588407


##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java:
##########
@@ -321,8 +315,8 @@ public void shouldAddTasksWithLatestOffsetToPrevActiveTasks() {
         final Map<TaskId, Long> taskOffsetSums = Collections.singletonMap(TASK_0_1, Task.LATEST_OFFSET);
         client.addPreviousTasksAndOffsetSums("c1", taskOffsetSums);
         client.initializePrevTasks(Collections.emptyMap(), false);
-        assertThat(client.prevActiveTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.previousAssignedTasks(), equalTo(Collections.singleton(TASK_0_1)));
+        assertEquals(Collections.singleton(TASK_0_1), client.prevActiveTasks());
+        assertEquals(Collections.singleton(TASK_0_1), client.previousAssignedTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java:
##########
@@ -163,75 +157,75 @@ public void shouldRefuseToUnassignStandbyTaskAsActive() {
     public void shouldUnassignActiveTask() {
         final ClientState clientState = new ClientState(1);
         clientState.assignActive(TASK_0_0);
-        assertThat(clientState, hasActiveTasks(1));
+        assertEquals(1, clientState.activeTaskCount());
         clientState.unassignActive(TASK_0_0);
-        assertThat(clientState, hasActiveTasks(0));
+        assertEquals(0, clientState.activeTaskCount());
     }
 
     @Test
     public void shouldUnassignStandbyTask() {
         final ClientState clientState = new ClientState(1);
         clientState.assignStandby(TASK_0_0);
-        assertThat(clientState, hasStandbyTasks(1));
+        assertEquals(1, clientState.standbyTaskCount());
         clientState.unassignStandby(TASK_0_0);
-        assertThat(clientState, hasStandbyTasks(0));
+        assertEquals(0, clientState.standbyTaskCount());
     }
 
     @Test
     public void shouldNotModifyActiveView() {
         final ClientState clientState = new ClientState(1);
         final Set<TaskId> taskIds = clientState.activeTasks();
         assertThrows(UnsupportedOperationException.class, () -> taskIds.add(TASK_0_0));
-        assertThat(clientState, hasActiveTasks(0));
+        assertEquals(0, clientState.activeTaskCount());
     }
 
     @Test
     public void shouldNotModifyStandbyView() {
         final ClientState clientState = new ClientState(1);
         final Set<TaskId> taskIds = clientState.standbyTasks();
         assertThrows(UnsupportedOperationException.class, () -> taskIds.add(TASK_0_0));
-        assertThat(clientState, hasStandbyTasks(0));
+        assertEquals(0, clientState.standbyTaskCount());
     }
 
     @Test
     public void shouldNotModifyAssignedView() {
         final ClientState clientState = new ClientState(1);
         final Set<TaskId> taskIds = clientState.assignedTasks();
         assertThrows(UnsupportedOperationException.class, () -> taskIds.add(TASK_0_0));
-        assertThat(clientState, hasActiveTasks(0));
-        assertThat(clientState, hasStandbyTasks(0));
+        assertEquals(0, clientState.activeTaskCount());
+        assertEquals(0, clientState.standbyTaskCount());
     }
 
     @Test
     public void shouldAddActiveTasksToBothAssignedAndActive() {
         client.assignActive(TASK_0_1);
-        assertThat(client.activeTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.assignedTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.assignedTaskCount(), equalTo(1));
-        assertThat(client.standbyTasks().size(), equalTo(0));
+        assertEquals(Collections.singleton(TASK_0_1), client.activeTasks());
+        assertEquals(Collections.singleton(TASK_0_1), client.assignedTasks());

Review Comment:
   ```suggestion
           assertEquals(Set.of(TASK_0_1), client.activeTasks());
           assertEquals(Set.of(TASK_0_1), client.assignedTasks());
   ```



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java:
##########
@@ -381,20 +375,16 @@ public void shouldReturnPreviousActiveStandbyTasksForConsumer() {
                 mkEntry(TASK_0_0, 10L)));
         client.addPreviousTasksAndOffsetSums("c2", Collections.singletonMap(TASK_0_2, 0L));
 
-        assertThat(client.prevOwnedStatefulTasksByConsumer("c1"), equalTo(Set.of(TASK_0_1, TASK_0_0)));
-        assertThat(client.prevOwnedStatefulTasksByConsumer("c2"), equalTo(Set.of(TASK_0_2)));
-        assertThat(client.prevOwnedActiveTasksByConsumer(), equalTo(
-                mkMap(
+        assertEquals(Set.of(TASK_0_1, TASK_0_0), client.prevOwnedStatefulTasksByConsumer("c1"));
+        assertEquals(Set.of(TASK_0_2), client.prevOwnedStatefulTasksByConsumer("c2"));
+        assertEquals(mkMap(
                         mkEntry("c1", Collections.singleton(TASK_0_1)),
                         mkEntry("c2", Collections.singleton(TASK_0_2))

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java:
##########
@@ -381,20 +375,16 @@ public void shouldReturnPreviousActiveStandbyTasksForConsumer() {
                 mkEntry(TASK_0_0, 10L)));
         client.addPreviousTasksAndOffsetSums("c2", Collections.singletonMap(TASK_0_2, 0L));
 
-        assertThat(client.prevOwnedStatefulTasksByConsumer("c1"), equalTo(Set.of(TASK_0_1, TASK_0_0)));
-        assertThat(client.prevOwnedStatefulTasksByConsumer("c2"), equalTo(Set.of(TASK_0_2)));
-        assertThat(client.prevOwnedActiveTasksByConsumer(), equalTo(
-                mkMap(
+        assertEquals(Set.of(TASK_0_1, TASK_0_0), client.prevOwnedStatefulTasksByConsumer("c1"));
+        assertEquals(Set.of(TASK_0_2), client.prevOwnedStatefulTasksByConsumer("c2"));
+        assertEquals(mkMap(
                         mkEntry("c1", Collections.singleton(TASK_0_1)),
                         mkEntry("c2", Collections.singleton(TASK_0_2))
-                ))
-        );
-        assertThat(client.prevOwnedStandbyByConsumer(), equalTo(
-                mkMap(
+                ), client.prevOwnedActiveTasksByConsumer());
+        assertEquals(mkMap(
                         mkEntry("c1", Collections.singleton(TASK_0_0)),
                         mkEntry("c2", Collections.emptySet())

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java:
##########
@@ -163,75 +157,75 @@ public void shouldRefuseToUnassignStandbyTaskAsActive() {
     public void shouldUnassignActiveTask() {
         final ClientState clientState = new ClientState(1);
         clientState.assignActive(TASK_0_0);
-        assertThat(clientState, hasActiveTasks(1));
+        assertEquals(1, clientState.activeTaskCount());
         clientState.unassignActive(TASK_0_0);
-        assertThat(clientState, hasActiveTasks(0));
+        assertEquals(0, clientState.activeTaskCount());
     }
 
     @Test
     public void shouldUnassignStandbyTask() {
         final ClientState clientState = new ClientState(1);
         clientState.assignStandby(TASK_0_0);
-        assertThat(clientState, hasStandbyTasks(1));
+        assertEquals(1, clientState.standbyTaskCount());
         clientState.unassignStandby(TASK_0_0);
-        assertThat(clientState, hasStandbyTasks(0));
+        assertEquals(0, clientState.standbyTaskCount());
     }
 
     @Test
     public void shouldNotModifyActiveView() {
         final ClientState clientState = new ClientState(1);
         final Set<TaskId> taskIds = clientState.activeTasks();
         assertThrows(UnsupportedOperationException.class, () -> taskIds.add(TASK_0_0));
-        assertThat(clientState, hasActiveTasks(0));
+        assertEquals(0, clientState.activeTaskCount());
     }
 
     @Test
     public void shouldNotModifyStandbyView() {
         final ClientState clientState = new ClientState(1);
         final Set<TaskId> taskIds = clientState.standbyTasks();
         assertThrows(UnsupportedOperationException.class, () -> taskIds.add(TASK_0_0));
-        assertThat(clientState, hasStandbyTasks(0));
+        assertEquals(0, clientState.standbyTaskCount());
     }
 
     @Test
     public void shouldNotModifyAssignedView() {
         final ClientState clientState = new ClientState(1);
         final Set<TaskId> taskIds = clientState.assignedTasks();
         assertThrows(UnsupportedOperationException.class, () -> taskIds.add(TASK_0_0));
-        assertThat(clientState, hasActiveTasks(0));
-        assertThat(clientState, hasStandbyTasks(0));
+        assertEquals(0, clientState.activeTaskCount());
+        assertEquals(0, clientState.standbyTaskCount());
     }
 
     @Test
     public void shouldAddActiveTasksToBothAssignedAndActive() {
         client.assignActive(TASK_0_1);
-        assertThat(client.activeTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.assignedTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.assignedTaskCount(), equalTo(1));
-        assertThat(client.standbyTasks().size(), equalTo(0));
+        assertEquals(Collections.singleton(TASK_0_1), client.activeTasks());
+        assertEquals(Collections.singleton(TASK_0_1), client.assignedTasks());
+        assertEquals(1, client.assignedTaskCount());
+        assertEquals(0, client.standbyTasks().size());
     }
 
     @Test
     public void shouldAddStandbyTasksToBothStandbyAndAssigned() {
         client.assignStandby(TASK_0_1);
-        assertThat(client.assignedTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.standbyTasks(), equalTo(Collections.singleton(TASK_0_1)));
-        assertThat(client.assignedTaskCount(), equalTo(1));
-        assertThat(client.activeTasks().size(), equalTo(0));
+        assertEquals(Collections.singleton(TASK_0_1), client.assignedTasks());
+        assertEquals(Collections.singleton(TASK_0_1), client.standbyTasks());

Review Comment:
   ditto
   



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -209,11 +203,11 @@ public void shouldMigrateActiveTasksToNewProcessWithoutChangingAllAssignments(fi
 
         final boolean probingRebalanceNeeded = assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2);
 
-        assertThat(probingRebalanceNeeded, is(false));
-        assertThat(clients.get(PID_2).activeTasks(), equalTo(singleton(TASK_0_1)));
-        assertThat(clients.get(PID_1).activeTasks().size(), equalTo(1));
-        assertThat(clients.get(PID_3).activeTasks().size(), equalTo(1));
-        assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2)));
+        assertFalse(probingRebalanceNeeded);
+        assertEquals(singleton(TASK_0_1), clients.get(PID_2).activeTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -209,11 +203,11 @@ public void shouldMigrateActiveTasksToNewProcessWithoutChangingAllAssignments(fi
 
         final boolean probingRebalanceNeeded = assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2);
 
-        assertThat(probingRebalanceNeeded, is(false));
-        assertThat(clients.get(PID_2).activeTasks(), equalTo(singleton(TASK_0_1)));
-        assertThat(clients.get(PID_1).activeTasks().size(), equalTo(1));
-        assertThat(clients.get(PID_3).activeTasks().size(), equalTo(1));
-        assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2)));
+        assertFalse(probingRebalanceNeeded);
+        assertEquals(singleton(TASK_0_1), clients.get(PID_2).activeTasks());
+        assertEquals(1, clients.get(PID_1).activeTasks().size());
+        assertEquals(1, clients.get(PID_3).activeTasks().size());
+        assertEquals(asList(TASK_0_0, TASK_0_1, TASK_0_2), allActiveTasks());

Review Comment:
   ```suggestion
           assertEquals(List.of(TASK_0_0, TASK_0_1, TASK_0_2), allActiveTasks());
   ```



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -355,25 +347,25 @@ public void shouldAssignStandbyTasksToDifferentClientThanCorrespondingActiveTask
         createClientWithPreviousActiveTasks(PID_4, 1, TASK_0_3);
 
         final boolean probingRebalanceNeeded = assign(1, rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3);
-        assertThat(probingRebalanceNeeded, is(false));
+        assertFalse(probingRebalanceNeeded);
 
 
-        assertThat(clients.get(PID_1).standbyTasks(), not(hasItems(TASK_0_0)));
-        assertThat(clients.get(PID_1).standbyTasks().size(), lessThanOrEqualTo(2));
-        assertThat(clients.get(PID_2).standbyTasks(), not(hasItems(TASK_0_1)));
-        assertThat(clients.get(PID_2).standbyTasks().size(), lessThanOrEqualTo(2));
-        assertThat(clients.get(PID_3).standbyTasks(), not(hasItems(TASK_0_2)));
-        assertThat(clients.get(PID_3).standbyTasks().size(), lessThanOrEqualTo(2));
-        assertThat(clients.get(PID_4).standbyTasks(), not(hasItems(TASK_0_3)));
-        assertThat(clients.get(PID_4).standbyTasks().size(), lessThanOrEqualTo(2));
+        assertFalse(clients.get(PID_1).standbyTasks().contains(TASK_0_0));
+        assertTrue(clients.get(PID_1).standbyTasks().size() <= 2);
+        assertFalse(clients.get(PID_2).standbyTasks().contains(TASK_0_1));
+        assertTrue(clients.get(PID_2).standbyTasks().size() <= 2);
+        assertFalse(clients.get(PID_3).standbyTasks().contains(TASK_0_2));
+        assertTrue(clients.get(PID_3).standbyTasks().size() <= 2);
+        assertFalse(clients.get(PID_4).standbyTasks().contains(TASK_0_3));
+        assertTrue(clients.get(PID_4).standbyTasks().size() <= 2);
 
         int nonEmptyStandbyTaskCount = 0;
         for (final ClientState clientState : clients.values()) {
             nonEmptyStandbyTaskCount += clientState.standbyTasks().isEmpty() ? 0 : 1;
         }
 
-        assertThat(nonEmptyStandbyTaskCount, greaterThanOrEqualTo(3));
-        assertThat(allStandbyTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3)));
+        assertTrue(nonEmptyStandbyTaskCount >= 3);
+        assertEquals(asList(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3), allStandbyTasks());

Review Comment:
   ```suggestion
           assertEquals(List.of(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3), allStandbyTasks());
   ```



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -335,10 +327,10 @@ public void shouldAssignBasedOnCapacityWhenMultipleClientHaveStandbyTasks(final
 
         final boolean probingRebalanceNeeded = assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2);
 
-        assertThat(probingRebalanceNeeded, is(false));
+        assertFalse(probingRebalanceNeeded);
 
-        assertThat(clients.get(PID_1).activeTasks(), equalTo(singleton(TASK_0_0)));
-        assertThat(clients.get(PID_2).activeTasks(), equalTo(Set.of(TASK_0_2, TASK_0_1)));
+        assertEquals(singleton(TASK_0_0), clients.get(PID_1).activeTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -464,9 +456,9 @@ public void shouldAssignEachActiveTaskToOneClientWhenMoreClientsThanTasks(final
         createClient(PID_6, 1);
 
         final boolean probingRebalanceNeeded = assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2);
-        assertThat(probingRebalanceNeeded, is(false));
+        assertFalse(probingRebalanceNeeded);
 
-        assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2)));
+        assertEquals(asList(TASK_0_0, TASK_0_1, TASK_0_2), allActiveTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -423,10 +415,10 @@ public void shouldAssignActiveAndStandbyTasks(final String rackAwareStrategy) {
         createClient(PID_3, 1);
 
         final boolean probingRebalanceNeeded = assign(1, rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2);
-        assertThat(probingRebalanceNeeded, is(false));
+        assertFalse(probingRebalanceNeeded);
 
-        assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2)));
-        assertThat(allStandbyTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2)));
+        assertEquals(asList(TASK_0_0, TASK_0_1, TASK_0_2), allActiveTasks());
+        assertEquals(asList(TASK_0_0, TASK_0_1, TASK_0_2), allStandbyTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java:
##########
@@ -417,15 +407,15 @@ public void shouldReturnAssignedTasksForConsumer() {
         // calling it multiple tasks should be idempotent
         client.revokeActiveFromConsumer(TASK_0_1, "c1");
 
-        assertThat(client.assignedActiveTasksByConsumer(), equalTo(mkMap(
+        assertEquals(mkMap(
                 mkEntry("c1", Set.of(TASK_0_0, TASK_0_1)),
                 mkEntry("c2", Set.of(TASK_0_2))
-        )));
-        assertThat(client.assignedStandbyTasksByConsumer(), equalTo(mkMap(
+        ), client.assignedActiveTasksByConsumer());
+        assertEquals(mkMap(
                 mkEntry("c1", Set.of(TASK_0_2)),
                 mkEntry("c2", Set.of(TASK_0_0))
-        )));
-        assertThat(client.revokingActiveTasksByConsumer(), equalTo(Collections.singletonMap("c1", Set.of(TASK_0_1))));
+        ), client.assignedStandbyTasksByConsumer());
+        assertEquals(Collections.singletonMap("c1", Set.of(TASK_0_1)), client.revokingActiveTasksByConsumer());

Review Comment:
   ```suggestion
           assertEquals(Map.of("c1", Set.of(TASK_0_1)), client.revokingActiveTasksByConsumer());
   ```



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -275,11 +267,11 @@ public void shouldKeepActiveTaskStickinessWhenMoreClientThanActiveTasks(final St
         createClient(PID_4, 1);
         createClient(PID_5, 1);
 
-        assertThat(assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2), is(false));
+        assertFalse(assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2));
 
-        assertThat(clients.get(PID_1).activeTasks(), equalTo(singleton(TASK_0_0)));
-        assertThat(clients.get(PID_2).activeTasks(), equalTo(singleton(TASK_0_2)));
-        assertThat(clients.get(PID_3).activeTasks(), equalTo(singleton(TASK_0_1)));
+        assertEquals(singleton(TASK_0_0), clients.get(PID_1).activeTasks());
+        assertEquals(singleton(TASK_0_2), clients.get(PID_2).activeTasks());
+        assertEquals(singleton(TASK_0_1), clients.get(PID_3).activeTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -313,11 +305,11 @@ public void shouldAssignTasksToClientWithPreviousStandbyTasks(final String rackA
 
         final boolean probingRebalanceNeeded = assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2);
 
-        assertThat(probingRebalanceNeeded, is(false));
+        assertFalse(probingRebalanceNeeded);
 
-        assertThat(clients.get(PID_1).activeTasks(), equalTo(singleton(TASK_0_2)));
-        assertThat(clients.get(PID_2).activeTasks(), equalTo(singleton(TASK_0_1)));
-        assertThat(clients.get(PID_3).activeTasks(), equalTo(singleton(TASK_0_0)));
+        assertEquals(singleton(TASK_0_2), clients.get(PID_1).activeTasks());
+        assertEquals(singleton(TASK_0_1), clients.get(PID_2).activeTasks());
+        assertEquals(singleton(TASK_0_0), clients.get(PID_3).activeTasks());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/KafkaStreamsStateTest.java:
##########
@@ -60,13 +59,14 @@ public void shouldCorrectlyReturnTasksByLag() {
         );
 
         assertThrows(IllegalStateException.class, () -> state.lagFor(NAMED_TASK_T0_1_0));
-        assertThat(state.lagFor(NAMED_TASK_T0_0_0), equalTo(2000L));
-        assertThat(state.lagFor(NAMED_TASK_T0_0_1), equalTo(1000L));
+        assertEquals(2000L, state.lagFor(NAMED_TASK_T0_0_0));
+        assertEquals(1000L, state.lagFor(NAMED_TASK_T0_0_1));
 
-        assertThat(state.prevTasksByLag("c0"), equalTo(new TreeSet<>()));
-        assertThat(state.prevTasksByLag("c1"), equalTo(new TreeSet<>(
-            Arrays.asList(NAMED_TASK_T0_0_1, NAMED_TASK_T0_0_0)
-        )));
+        assertEquals(new TreeSet<>(), state.prevTasksByLag("c0"));
+        assertEquals(
+            new TreeSet<>(Arrays.asList(NAMED_TASK_T0_0_1, NAMED_TASK_T0_0_0)),

Review Comment:
   ```suggestion
               new TreeSet<>(List.of(NAMED_TASK_T0_0_1, NAMED_TASK_T0_0_0)),
   ```



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/LegacyStickyTaskAssignorTest.java:
##########
@@ -289,11 +281,11 @@ public void shouldKeepActiveTaskStickinessWhenMoreClientThanActiveTasks(final St
         createClientWithPreviousActiveTasks(PID_4, 1, TASK_0_2);
         createClientWithPreviousActiveTasks(PID_5, 1, TASK_0_1);
 
-        assertThat(assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2), is(false));
+        assertFalse(assign(rackAwareStrategy, TASK_0_0, TASK_0_1, TASK_0_2));
 
-        assertThat(clients.get(PID_2).activeTasks(), equalTo(singleton(TASK_0_0)));
-        assertThat(clients.get(PID_4).activeTasks(), equalTo(singleton(TASK_0_2)));
-        assertThat(clients.get(PID_5).activeTasks(), equalTo(singleton(TASK_0_1)));
+        assertEquals(singleton(TASK_0_0), clients.get(PID_2).activeTasks());
+        assertEquals(singleton(TASK_0_2), clients.get(PID_4).activeTasks());
+        assertEquals(singleton(TASK_0_1), clients.get(PID_5).activeTasks());

Review Comment:
   ditto



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] KAFKA-20993: Remove hamcrest from org.apache.kafka.streams.processor.internals.assignment [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23291:
URL: https://github.com/apache/kafka/pull/23291#discussion_r3897363592


##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/GraphTest.java:
##########
@@ -60,7 +58,7 @@ public void setUp() {
     public void testBasic() {
         final Set<Integer> nodes = graph.nodes();
         assertEquals(6, nodes.size());
-        assertThat(nodes, contains(-1, 0, 1, 2, 3, 99));
+        assertEquals(Set.of(-1, 0, 1, 2, 3, 99), nodes);

Review Comment:
   `nodes` is a sorted set, so please use `assertIterableEquals` instead



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/GraphTest.java:
##########
@@ -99,7 +97,7 @@ public void testResidualGraph() {
 
         final Set<Integer> nodes = residualGraph.nodes();
         assertEquals(6, nodes.size());
-        assertThat(nodes, contains(-1, 0, 1, 2, 3, 99));
+        assertEquals(Set.of(-1, 0, 1, 2, 3, 99), nodes);

Review Comment:
   ditto



-- 
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-unsubscribe@kafka.apache.org

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