-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KAFKA-14587: Move AclCommand to tools #17880
base: trunk
Are you sure you want to change the base?
Conversation
@@ -303,7 +294,7 @@ public void testUseBootstrapServerOptWithBootstrapControllerOpt() { | |||
@Test | |||
public void testUseWithoutBootstrapServerOptAndBootstrapControllerOpt() { | |||
assertInitializeInvalidOptionsExitCodeAndMsg( | |||
Collections.emptyList(), | |||
Collections.singletonList(ADD), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually if you don't provide any arguments on the CLI, just the help message is printed.
The previous test was expecting a different behavior as maybePrintHelpOrVersion()
used to be called in main()
while I moved it to AclCommandOptions.checkArgs()
for consistency with the other tools, and consistency with the CLI behavior.
@@ -355,7 +346,7 @@ public void testInvalidArgs() { | |||
"Option \"[list]\" can't be used with option \"[producer]\"" | |||
); | |||
assertInitializeInvalidOptionsExitCodeAndMsg( | |||
Arrays.asList(BOOTSTRAP_SERVER, LOCALHOST, ADD, PRODUCER, OPERATION), | |||
Arrays.asList(BOOTSTRAP_SERVER, LOCALHOST, ADD, PRODUCER, OPERATION, "all"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running the same command on the CLI did not produce that error message, so this test was effectively testing internal logic that would not be reachable for users.
./bin/kafka-acls.sh --bootstrap-server localhost:9092 --add --producer --operation
Exception in thread "main" joptsimple.OptionMissingRequiredArgumentException: Option operation requires an argument
To produce that error message from the CLI you need to specify an operation.
6a3627d
to
50115da
Compare
50115da
to
49120e4
Compare
Committer Checklist (excluded from commit message)