philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1612009583


##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -2952,8 +2952,8 @@ private static class FetchInfo {
     // TODO: this test requires rebalance logic which is not yet implemented 
in the CONSUMER group protocol.
     //       Once it is implemented, this should use both group protocols.
     @ParameterizedTest
-    @EnumSource(value = GroupProtocol.class, names = "CLASSIC")
-    public void testSubscriptionOnInvalidTopic(GroupProtocol groupProtocol) {
+    @EnumSource(value = GroupProtocol.class)
+    public void testSubscriptionOnInvalidTopic(GroupProtocol groupProtocol) 
throws InterruptedException {

Review Comment:
   i wonder if we should just move these tests to integration tests. In this 
case, KafkaConsumer test becomes a huge integration test.  
   
   ideally - we want to make sure the tests are deterministic and thread.sleep 
can potentially introduce flakiness, so i would conduct the test differently, 
i.e.:
   
   1. mock the background queue to ensure the right exception is being thrown 
during the poll
   2. add an integration test to test the exception (see the integration test 
module)



-- 
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