pasharik commented on code in PR #15830:
URL: https://github.com/apache/kafka/pull/15830#discussion_r1612152214
##########
core/src/test/scala/unit/kafka/admin/AclCommandTest.scala:
##########
@@ -122,19 +128,27 @@ class AclCommandTest extends QuorumTestHarness with
Logging {
super.tearDown()
}
+ override protected def kraftControllerConfigs(): Seq[Properties] = {
+ val controllerConfig = new Properties
+ controllerConfig.put(KafkaConfig.AuthorizerClassNameProp,
classOf[StandardAuthorizer].getName)
+ controllerConfig.put(StandardAuthorizer.SUPER_USERS_CONFIG,
"User:ANONYMOUS")
+ Seq(controllerConfig)
+ }
+
@Test
def testAclCliWithAuthorizer(): Unit = {
testAclCli(zkArgs)
}
- @Test
- def testAclCliWithAdminAPI(): Unit = {
+ @ParameterizedTest
+ @ValueSource(strings = Array("zk"))
Review Comment:
Commited [alternative
fix](https://github.com/apache/kafka/pull/15830/commits/d39be8a7c349e868b7c5066e6be89f14a9e6adf3)
for failing tests
For now, I've added a condition, so we don't verify the output from the
`main` for the Kraft tests. For adding ACLs:
```
if (!isKRaftTest()) {
assertOutputContains(...)
```
and for deleting ACLs:
```
if (!isKRaftTest()) {
assertEquals("", out)
```
Just wondering, do we need to check this `main` output at all?
> Maybe we can create `AclCommand.AdminClientService` to test its method
`listAcls` with retry instead of grabbing the output from `main` ?
Yeah, there is already similar logic in the test:
```
for (resource <- resources) {
withAuthorizer() { authorizer =>
TestUtils.waitAndVerifyAcls(...)
```
so eventualy ACLs are propageted in Kraft mode, and tests pass.
The only problem is with capturing the immediate output from the `main`,
which may not be correct sometimes, because of the race condigion
--
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]
