majialoong commented on PR #22591: URL: https://github.com/apache/kafka/pull/22591#issuecomment-4780661623
@mimaison Thanks for raising this. I agree that the current name is too generic and does not describe the actual coverage well. Looking at the history, this test came from apache/kafka#3661, where both the `AdminClient` path and the `ConsumerGroupCommand` path were covered. At that time, the related tests all lived on the server side. However, when apache/kafka#15365 split this test into the tools module, it was mostly a mechanical split and kept the original generic name, which makes the narrow command level scenario less clear. I think keeping this test is still reasonable since it covers this scenario from the `ConsumerGroupCommand` path. Maybe `ConsumerGroupCommandAuthorizationTest` would be a clearer name for this test. The SASL test has the same issue, and `ConsumerGroupCommandSaslAuthenticationTest` may describe that one better. I have updated the PR with this rename. What do you think? -- 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]
