kirktrue commented on code in PR #17035:
URL: https://github.com/apache/kafka/pull/17035#discussion_r1817411319


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetchRequestManagerTest.java:
##########
@@ -3386,6 +3394,71 @@ public void 
testWhenFetchResponseReturnsALeaderShipChangeErrorAndNewLeaderInform
         assertTrue(subscriptions.isFetchable(tp1));
     }
 
+    @Test
+    public void testPollWithoutCreateFetchRequests() {
+        buildFetcher();
+
+        assignFromUser(singleton(tp0));
+        subscriptions.seek(tp0, 0);

Review Comment:
   Prior to this change, all that was needed to start fetching was a valid set 
of assigned partitions. Post-change, that is insufficient; no fetch requests 
will be issued if not explicitly requested.
   
   If I removed the assignment, the test looks very anemic:
   
   ```java
       @Test
       public void testPollWithoutCreateFetchRequests() {
           buildFetcher();
           assertEquals(0, sendFetches(false));
       }
   ```
   
   I'm OK to remove that test since the other tests cover that basic test 
anyway.
   
   Thoughts?



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

Reply via email to