lianetm commented on code in PR #15640:
URL: https://github.com/apache/kafka/pull/15640#discussion_r1608897877
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##########
@@ -281,64 +276,15 @@ void testEnsureMetadataUpdateOnPoll() {
}
@Test
- void testEnsureEventsAreCompleted() {
Review Comment:
Actually seems to me that we shouldn't have this test here (and maybe this
is why @kirktrue removed it before?). As I see it, this unit test is testing
something that is not the `ConsumerNetworkThread`'s responsibility (and that's
why it ends up being complicated, having to mimic the reaper behaviour and
spying). It is testing that events are completed, and that's the reaper.reap
responsibility, so seems to me we need to:
1. test that the `ConsumerNetworkThread` calls the reaper with the full list
of events -> done already in the
[testCleanupInvokesReaper](https://github.com/apache/kafka/blob/91af164415b8b950c70d3a61bb0837c34ae4ed69/clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java#L331)
2. test that the `CompletableEventReaper.reap(Collection<?> events)`
completes the events -> done in CompletableEventReaperTest
([testIncompleteQueue](https://github.com/apache/kafka/blob/91af164415b8b950c70d3a61bb0837c34ae4ed69/clients/src/test/java/org/apache/kafka/clients/consumer/internals/events/CompletableEventReaperTest.java#L135)
and
[testIncompleteTracked](https://github.com/apache/kafka/blob/91af164415b8b950c70d3a61bb0837c34ae4ed69/clients/src/test/java/org/apache/kafka/clients/consumer/internals/events/CompletableEventReaperTest.java#L170))
In the end, as it is, we end up asserting a behaviour we're mocking
ourselves in the `doAnswer`, so not much value I would say? Agree with @cadonna
that we need coverage, but I would say that we have it, on my points 1 and 2,
and this should be removed. Makes sense?
--
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
