lianetm commented on code in PR #16140:
URL: https://github.com/apache/kafka/pull/16140#discussion_r1625034602
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##########
@@ -150,19 +209,19 @@ public void testStartupAndTearDown() throws
InterruptedException {
}
@Test
- public void testApplicationEvent() {
- ApplicationEvent e = new PollEvent(100);
- applicationEventsQueue.add(e);
+ void testRequestManagersArePolledOnce() {
consumerNetworkThread.runOnce();
- verify(applicationEventProcessor, times(1)).process(e);
+ requestManagers.entries().forEach(rmo -> rmo.ifPresent(rm ->
verify(rm).poll(anyLong())));
+ requestManagers.entries().forEach(rmo -> rmo.ifPresent(rm ->
verify(rm).maximumTimeToWait(anyLong())));
+ verify(networkClientDelegate).poll(anyLong(), anyLong());
Review Comment:
This test seems to leave out one of the core actions of the `runOnce`:
1. poll managers (this generates the requests) -> covered by the test
2. add newly generated requests are added to the network client -> not
covered
3. poll network client to send the requests -> covered by the test
Step 3 wouldn't have the effect we want (send the request) if step 2 does
not happen, so what about adding a `verify(networkClientDelegate).addAll(..`
before verifying the network client poll, to make sure we cover the whole flow
of how requests move from the managers to the network layer on run once?
--
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
