kirktrue commented on code in PR #14670:
URL: https://github.com/apache/kafka/pull/14670#discussion_r1386910396


##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -222,42 +214,74 @@ public void cleanup() {
         }
     }
 
-    @Test
-    public void testMetricsReporterAutoGeneratedClientId() {
+    private static Collection<Arguments> bothGroupProtocols() {
+        return 
Arrays.stream(GroupProtocol.values()).map(Arguments::of).collect(Collectors.toList());
+    }
+
+    /**
+     * A given test may choose to use the {@link GroupProtocol#GENERIC generic 
group protocol} for a number of reasons.
+     * Among the reasons for a test to do so is because it...
+     *
+     * <ul>
+     *     <li>
+     *         ...exercises rebalancing logic that is not yet implemented in 
the
+     *         {@link GroupProtocol#CONSUMER consumer group protocol}.
+     *     </li>
+     *     <li>...includes topic metadata that is not yet implemented in the 
consumer group protocol.</li>
+     *     <li>...fails, possibly due to the omission of functionality in the 
consumer group protocol.</li>
+     *     <li>...uses logic, timing, etc. that are not applicable to the 
consumer group protocol.</li>
+     * </ul>
+     *
+     * Less than half of the tests for the consumer group protocol pass as of 
now, but it's very tedious to
+     * investigate at this point due to known bugs and missing functionality.
+     */

Review Comment:
   @dajac—I've made the change to use `EnumSource` everywhere. Thanks for that 
suggestion 😄
   
   As far as the logistics for tracking these ~29 bugs via Jiras—do you suggest 
we file separate Jiras for each test failure, an "uber" Jira for all of the 
failures, or something else?



##########
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##########
@@ -222,42 +214,74 @@ public void cleanup() {
         }
     }
 
-    @Test
-    public void testMetricsReporterAutoGeneratedClientId() {
+    private static Collection<Arguments> bothGroupProtocols() {
+        return 
Arrays.stream(GroupProtocol.values()).map(Arguments::of).collect(Collectors.toList());
+    }
+
+    /**
+     * A given test may choose to use the {@link GroupProtocol#GENERIC generic 
group protocol} for a number of reasons.
+     * Among the reasons for a test to do so is because it...
+     *
+     * <ul>
+     *     <li>
+     *         ...exercises rebalancing logic that is not yet implemented in 
the
+     *         {@link GroupProtocol#CONSUMER consumer group protocol}.
+     *     </li>
+     *     <li>...includes topic metadata that is not yet implemented in the 
consumer group protocol.</li>
+     *     <li>...fails, possibly due to the omission of functionality in the 
consumer group protocol.</li>
+     *     <li>...uses logic, timing, etc. that are not applicable to the 
consumer group protocol.</li>
+     * </ul>
+     *
+     * Less than half of the tests for the consumer group protocol pass as of 
now, but it's very tedious to
+     * investigate at this point due to known bugs and missing functionality.
+     */

Review Comment:
   @dajac—I've made the change to use `EnumSource` everywhere. Thanks for that 
suggestion 😄
   
   As far as the logistics for tracking these ~29 bugs via Jiras—do you suggest 
we file separate Jiras for each test failure, an "uber" Jira for all of the 
failures, or something else?



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

Reply via email to