lianetm commented on code in PR #19669:
URL: https://github.com/apache/kafka/pull/19669#discussion_r2093503258
##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -1842,12 +1845,12 @@ public void testOffsetOfPausedPartitions(GroupProtocol
groupProtocol) {
offsets.put(tp1, 0L);
client.prepareResponseFrom(offsetResponse(offsets, Errors.NONE),
coordinator);
- assertEquals(0,
consumer.committed(Collections.singleton(tp0)).get(tp0).offset());
+ assertEquals(0, consumer.committed(Set.of(tp0)).get(tp0).offset());
offsets.remove(tp0);
offsets.put(tp1, 0L);
client.prepareResponseFrom(offsetResponse(offsets, Errors.NONE),
coordinator);
- assertEquals(0,
consumer.committed(Collections.singleton(tp1)).get(tp1).offset());
+ assertEquals(0, consumer.committed(Set.of(tp1)).get(tp1).offset());
Review Comment:
since we're already changing to Set.of/Map.of in several place, should we do
it consistently for the whole file? (still using `singletonMap` in ln 1962
right below and others)
##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -3425,6 +3441,7 @@ public void testAssignorNameConflict(GroupProtocol
groupProtocol) {
() -> newConsumer(configs, new StringDeserializer(), new
StringDeserializer()));
}
+ @SuppressWarnings("resource")
Review Comment:
yeah, I expect that's because the try-with will attempt close with the def
timeout and we're not mocking what's needed for that to complete in time.
I'm ok with leaving the suppression, but maybe calling `consumer.close(0)`
after the assertion is a clean way to do it? (no complains + no suppressions).
Thoughts?
##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -1959,17 +1963,19 @@ public void testCloseShouldBeIdempotent(GroupProtocol
groupProtocol) {
consumer = newConsumer(groupProtocol, time, client, subscription,
metadata, assignor, false, groupInstanceId);
- consumer.close(Duration.ZERO);
- consumer.close(Duration.ZERO);
+ consumer.close(CloseOptions.timeout(Duration.ZERO));
+ consumer.close(CloseOptions.timeout(Duration.ZERO));
// verify that the call is idempotent by checking that the network
client is only closed once.
verify(client).close();
}
+ @SuppressWarnings("resource")
Review Comment:
we could also `try (Consumer<byte[], byte[]> ignored = newConsumer...`,
(then no need to suppress)
--
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
