lianetm commented on code in PR #16982:
URL: https://github.com/apache/kafka/pull/16982#discussion_r1731802528
##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -2523,16 +2537,32 @@ public void
testListOffsetShouldUpdateSubscriptions(GroupProtocol groupProtocol)
// poll once to update with the current metadata
consumer.poll(Duration.ofMillis(0));
+ TestUtils.waitForCondition(() -> requestGenerated(client,
ApiKeys.FIND_COORDINATOR),
+ "No metadata requests sent");
client.respond(FindCoordinatorResponse.prepareResponse(Errors.NONE,
groupId, metadata.fetch().nodes().get(0)));
consumer.seek(tp0, 50L);
+
+ if (groupProtocol == GroupProtocol.CONSUMER) {
Review Comment:
I see how you make the test pass, and that is fine and works, but I believe
we could greatly simplify this test if we get the FindCoord, OffsetFetch and
FetchRequests out of the picture, which seem to me are really not relevant to
this test?
This test is all about partition offsets (end offsets stored in the leader),
compared to a position set manually with seek, so not related at all with
committed offsets or affected by fetch.
So, this is the simplification I'm thinking about:
1. we create the consumer without groupId -> this will ensure we don't send
any OffsetFetch request. We don't really need them given that we're only
playing with the partition offsets
2. we pause the partition right after assign -> this ensures that we don't
issue fetch requests. We don't really need them for this test, and having them
makes the test different for both consumers
With those 2 small changes, I would expect we can keep the same test for
both consumer, without any specifics for the new consumer, and it would still
be true to its purpose, testing what it has always tested. What do you think?
--
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]
