lianetm commented on code in PR #21280:
URL: https://github.com/apache/kafka/pull/21280#discussion_r2709022323
##########
tools/src/test/java/org/apache/kafka/tools/ConfigCommandIntegrationTest.java:
##########
@@ -461,27 +461,34 @@ public void
testUpdatePerBrokerConfigInKRaftThenShouldFail() {
}
@ClusterTest
- public void testUpdateInvalidBrokerConfigs() {
+ public void testUpdateInvalidBrokerConfigs() throws InterruptedException {
updateAndCheckInvalidBrokerConfig(Optional.empty());
updateAndCheckInvalidBrokerConfig(Optional.of(String.valueOf((cluster.brokers().entrySet().iterator().next().getKey()))));
}
- private void updateAndCheckInvalidBrokerConfig(Optional<String>
brokerIdOrDefault) {
+ private void updateAndCheckInvalidBrokerConfig(Optional<String>
brokerIdOrDefault) throws InterruptedException {
List<String> alterOpts =
generateDefaultAlterOpts(cluster.bootstrapServers());
try (Admin client = cluster.admin()) {
alterConfigWithAdmin(client, brokerIdOrDefault, Map.of("invalid",
"2"), alterOpts);
+ AtomicReference<String> last = new AtomicReference<>("");
- Stream<String> describeCommand = Stream.concat(
- Stream.concat(
- Stream.of("--bootstrap-server",
cluster.bootstrapServers()),
- Stream.of(entityOp(brokerIdOrDefault).toArray(new
String[0]))),
- Stream.of("--entity-type", "brokers", "--describe"));
- String describeResult = captureStandardOut(run(describeCommand));
-
- // We will treat unknown config as sensitive
- assertTrue(describeResult.contains("sensitive=true"),
describeResult);
- // Sensitive config will not return
- assertTrue(describeResult.contains("invalid=null"),
describeResult);
+ TestUtils.waitForCondition(() -> {
+ Stream<String> describeCommand = Stream.concat(
+ Stream.concat(
+ Stream.of("--bootstrap-server",
cluster.bootstrapServers()),
+
Stream.of(entityOp(brokerIdOrDefault).toArray(new String[0]))),
+ Stream.of("--entity-type", "brokers", "--describe")
+ );
+ String describeResult =
captureStandardOut(run(describeCommand));
+ last.set(describeResult);
+
+ // We will treat unknown config as sensitive
+ // For 'describeResult.contains("sensitive=true")'
+
+ // Sensitive config will not return
+ // For 'describeResult.contains("invalid=null")'
Review Comment:
also not clear on the value of these, the return line seems pretty explicit
to me, wdyt?
##########
tools/src/test/java/org/apache/kafka/tools/ConfigCommandIntegrationTest.java:
##########
@@ -461,27 +461,34 @@ public void
testUpdatePerBrokerConfigInKRaftThenShouldFail() {
}
@ClusterTest
- public void testUpdateInvalidBrokerConfigs() {
+ public void testUpdateInvalidBrokerConfigs() throws InterruptedException {
updateAndCheckInvalidBrokerConfig(Optional.empty());
updateAndCheckInvalidBrokerConfig(Optional.of(String.valueOf((cluster.brokers().entrySet().iterator().next().getKey()))));
}
- private void updateAndCheckInvalidBrokerConfig(Optional<String>
brokerIdOrDefault) {
+ private void updateAndCheckInvalidBrokerConfig(Optional<String>
brokerIdOrDefault) throws InterruptedException {
List<String> alterOpts =
generateDefaultAlterOpts(cluster.bootstrapServers());
try (Admin client = cluster.admin()) {
alterConfigWithAdmin(client, brokerIdOrDefault, Map.of("invalid",
"2"), alterOpts);
+ AtomicReference<String> last = new AtomicReference<>("");
- Stream<String> describeCommand = Stream.concat(
- Stream.concat(
- Stream.of("--bootstrap-server",
cluster.bootstrapServers()),
- Stream.of(entityOp(brokerIdOrDefault).toArray(new
String[0]))),
- Stream.of("--entity-type", "brokers", "--describe"));
- String describeResult = captureStandardOut(run(describeCommand));
-
- // We will treat unknown config as sensitive
- assertTrue(describeResult.contains("sensitive=true"),
describeResult);
- // Sensitive config will not return
- assertTrue(describeResult.contains("invalid=null"),
describeResult);
+ TestUtils.waitForCondition(() -> {
+ Stream<String> describeCommand = Stream.concat(
+ Stream.concat(
+ Stream.of("--bootstrap-server",
cluster.bootstrapServers()),
+
Stream.of(entityOp(brokerIdOrDefault).toArray(new String[0]))),
+ Stream.of("--entity-type", "brokers", "--describe")
+ );
+ String describeResult =
captureStandardOut(run(describeCommand));
+ last.set(describeResult);
+
+ // We will treat unknown config as sensitive
+ // For 'describeResult.contains("sensitive=true")'
+
+ // Sensitive config will not return
+ // For 'describeResult.contains("invalid=null")'
+ return describeResult.contains("sensitive=true") &&
describeResult.contains("invalid=null");
Review Comment:
I guess that it makes sense to wait until we get `invalid=null` in the
output, but once that happens we do expect to have `sensitive=true` right away,
right?
If my expectation is right, then we should fix this to ensure we fail fast
if sensitive is false for our "invalid" config (without waiting for sensitive
to become true)
##########
tools/src/test/java/org/apache/kafka/tools/ConfigCommandIntegrationTest.java:
##########
@@ -461,27 +461,34 @@ public void
testUpdatePerBrokerConfigInKRaftThenShouldFail() {
}
@ClusterTest
- public void testUpdateInvalidBrokerConfigs() {
+ public void testUpdateInvalidBrokerConfigs() throws InterruptedException {
updateAndCheckInvalidBrokerConfig(Optional.empty());
updateAndCheckInvalidBrokerConfig(Optional.of(String.valueOf((cluster.brokers().entrySet().iterator().next().getKey()))));
}
- private void updateAndCheckInvalidBrokerConfig(Optional<String>
brokerIdOrDefault) {
+ private void updateAndCheckInvalidBrokerConfig(Optional<String>
brokerIdOrDefault) throws InterruptedException {
List<String> alterOpts =
generateDefaultAlterOpts(cluster.bootstrapServers());
try (Admin client = cluster.admin()) {
alterConfigWithAdmin(client, brokerIdOrDefault, Map.of("invalid",
"2"), alterOpts);
+ AtomicReference<String> last = new AtomicReference<>("");
- Stream<String> describeCommand = Stream.concat(
- Stream.concat(
- Stream.of("--bootstrap-server",
cluster.bootstrapServers()),
- Stream.of(entityOp(brokerIdOrDefault).toArray(new
String[0]))),
- Stream.of("--entity-type", "brokers", "--describe"));
- String describeResult = captureStandardOut(run(describeCommand));
-
- // We will treat unknown config as sensitive
- assertTrue(describeResult.contains("sensitive=true"),
describeResult);
- // Sensitive config will not return
- assertTrue(describeResult.contains("invalid=null"),
describeResult);
+ TestUtils.waitForCondition(() -> {
+ Stream<String> describeCommand = Stream.concat(
+ Stream.concat(
+ Stream.of("--bootstrap-server",
cluster.bootstrapServers()),
+
Stream.of(entityOp(brokerIdOrDefault).toArray(new String[0]))),
+ Stream.of("--entity-type", "brokers", "--describe")
+ );
+ String describeResult =
captureStandardOut(run(describeCommand));
+ last.set(describeResult);
+
+ // We will treat unknown config as sensitive
+ // For 'describeResult.contains("sensitive=true")'
Review Comment:
don't quite get this comment. Needed?
--
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]
