lianetm commented on code in PR #17516:
URL: https://github.com/apache/kafka/pull/17516#discussion_r1840374672
##########
core/src/test/scala/integration/kafka/api/GroupAuthorizerIntegrationTest.scala:
##########
@@ -132,6 +133,136 @@ class GroupAuthorizerIntegrationTest extends
BaseRequestTest {
assertEquals(Set(topic), consumeException.unauthorizedTopics.asScala)
}
+ @ParameterizedTest(name =
TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+ @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+ def testConsumeUnsubscribeWithoutTopicPermission(quorum: String,
groupProtocol: String): Unit = {
Review Comment:
I wonder if this is better covered at the unit test level only. Here I don't
see how we can trust the test is actually testing the unsubscribe changes. The
trick is that the topic error comes in a metadata response, but the unsubscribe
completes successfully as soon as it gets a response to the HB, so it will
always complete ok unless we get a metadata response before the HB response
right? It's a really small window btw, because we only
processingBackgroundEvents (discover errors) from the moment we send the
Unsubscribe (leave HB), to the moment we get a response. Then we stop
processing background events, so nothing will be thrown even if it arrives in a
response.
The other `testConsumeUnsubscribeWithoutGroupPermission` makes sense to me,
because the group error comes in a HB response, as well as the unsusbcribe
response, so we can trust that if the unsubscribe does not throw is because
we're indeed swallowing the exception.
--
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]
