ShivsundarR commented on code in PR #19024:
URL: https://github.com/apache/kafka/pull/19024#discussion_r1970009792


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerHeartbeatRequestManagerTest.java:
##########
@@ -1029,7 +1029,8 @@ private static Collection<Arguments> errorProvider() {
             Arguments.of(Errors.UNSUPPORTED_VERSION, true),
             Arguments.of(Errors.UNRELEASED_INSTANCE_ID, true),
             Arguments.of(Errors.FENCED_INSTANCE_ID, true),
-            Arguments.of(Errors.GROUP_MAX_SIZE_REACHED, true));
+            Arguments.of(Errors.GROUP_MAX_SIZE_REACHED, true),
+            Arguments.of(Errors.TOPIC_AUTHORIZATION_FAILED, true));

Review Comment:
   Thanks, okay yeah I will change it to false, makes sense. 
   Also for the `ShareHeartbeatRequestManagerTest`, yes they are identical, 
except there is one check missing in that test which is present in 
`ConsumerHeartbeatRequestManagerTest`.
   ```
   case FENCED_MEMBER_EPOCH:
   verify(backgroundEventHandler, never()).add(any());
   assertNextHeartbeatTiming(0);
   break;
   ```
   Also in future if we decide to handle heartbeat responses differently for 
regular consumers and share consumers, it will be better to have 2 different 
tests right?



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