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