lianetm commented on code in PR #19024: URL: https://github.com/apache/kafka/pull/19024#discussion_r1970153512
########## 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: > the ShareHeartbeatRequestManagerTest, yes they are identical, except there is one check missing Exactly, that shows how the Share test has already fallen behind, it is redundant and we're not maintaining it. We should review and remove it imo (but can be done separately, I just noticed it here) >if we decide to handle heartbeat responses differently for regular consumers and share consumers, it will be better to have 2 different yes, but better to add it when needed (and not have a duplicate now that we either maintain or leave behind) I will file a jira and we can consider the improvement separately. -- update https://issues.apache.org/jira/browse/KAFKA-18862 -- 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