chia7712 commented on code in PR #15840:
URL: https://github.com/apache/kafka/pull/15840#discussion_r1598454210
##########
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##########
@@ -63,13 +73,26 @@
@SuppressWarnings("deprecation") // Added for Scala 2.12 compatibility for
usages of JavaConverters
@ExtendWith(value = ClusterTestExtensions.class)
+@ClusterTestDefaults
@Tag("integration")
public class ConfigCommandIntegrationTest {
- AdminZkClient adminZkClient;
- List<String> alterOpts;
+ private List<String> alterOpts;
+ private final String defaultBrokerId = "0";
private final ClusterInstance cluster;
+ private static Runnable run(Stream<String> command) {
+ return () -> {
+ try {
+ ConfigCommand.main(command.toArray(String[]::new));
+ } catch (RuntimeException e) {
Review Comment:
This is redundant if you do nothing in catching.
##########
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##########
@@ -63,13 +73,26 @@
@SuppressWarnings("deprecation") // Added for Scala 2.12 compatibility for
usages of JavaConverters
@ExtendWith(value = ClusterTestExtensions.class)
+@ClusterTestDefaults
Review Comment:
If you don't want to change any default value, this can get removed
##########
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##########
@@ -82,202 +105,448 @@ public void
testExitWithNonZeroStatusOnUpdatingUnallowedConfig() {
"--alter",
"--add-config", "security.inter.broker.protocol=PLAINTEXT")),
errOut ->
- assertTrue(errOut.contains("Cannot update these configs
dynamically: Set(security.inter.broker.protocol)"), errOut));
Review Comment:
Please revert this unrelated change. BTW, this test case should run all
cluster types, 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
