lianetm commented on code in PR #16310:
URL: https://github.com/apache/kafka/pull/16310#discussion_r1638194106


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java:
##########
@@ -574,6 +575,93 @@ public void testPollLongThrowsException() {
             "This method is deprecated and will be removed in the next major 
release.", e.getMessage());
     }
 
+    @Test
+    public void testOffsetFetchStoresPendingEvent() {
+        consumer = newConsumer();
+        long timeoutMs = 0;
+        
doReturn(Fetch.empty()).when(fetchCollector).collectFetch(any(FetchBuffer.class));
+        consumer.assign(Collections.singleton(new TopicPartition("topic1", 
0)));
+
+        // The first attempt at poll() creates an event, enqueues it, but its 
Future does not complete within the
+        // timeout, leaving a pending fetch.
+        consumer.poll(Duration.ofMillis(timeoutMs));
+        verify(applicationEventHandler, 
times(1)).add(any(FetchCommittedOffsetsEvent.class));
+        CompletableApplicationEvent<Map<TopicPartition, OffsetAndMetadata>> 
event = getLastEnqueuedEvent();
+        assertThrows(TimeoutException.class, () -> 
ConsumerUtils.getResult(event.future(), time.timer(timeoutMs)));
+        assertTrue(consumer.hasPendingOffsetFetchEvent());
+
+        // For the second attempt, the event is reused, and this time the 
Future returns successfully, clearing
+        // the pending fetch. Verify that the number of 
FetchCommittedOffsetsEvent enqueued remains at 1.
+        event.future().complete(Collections.emptyMap());
+        consumer.poll(Duration.ofMillis(timeoutMs));
+        verify(applicationEventHandler, 
times(1)).add(any(FetchCommittedOffsetsEvent.class));

Review Comment:
   uhm actually here we expect that a Fetch event was never added (because it 
should reuse the previous one). We could have `verify(applicationEventHandler, 
never()).add(any(FetchCommittedOffsetsEvent.class));` here to make the 
intention clear, and `clearInvocations(applicationEventHandler);` before the 
poll (I know the logic it's right but this line gives the wrong impression and 
makes me doubt our logic is right, and re-read the whole test :) ) 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to