lianetm commented on code in PR #21280:
URL: https://github.com/apache/kafka/pull/21280#discussion_r2714139432
##########
tools/src/test/java/org/apache/kafka/tools/ConfigCommandIntegrationTest.java:
##########
@@ -461,27 +461,37 @@ 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);
+
+ boolean hasInvalidNull =
describeResult.contains("invalid=null");
+ boolean hasSensitiveTrue =
describeResult.contains("sensitive=true");
+
+ if (hasInvalidNull && !hasSensitiveTrue) {
+ throw new AssertionError("Expected sensitive=true when
invalid=null is visible, but it was not present.\n" +
Review Comment:
will this truly make it interrupt the `waitForCondition`? or is it maybe
going to continue retrying? (from a quick look at the code path seems it ends
up using `retryOnExceptionWithTimeout`?)
Maybe we can just `waitForCondition` checking
`describeResult.contains("invalid=null") `, and then, outside of the wait,
assert that `describeResult.contains("sensitive=true");` wdyt?
nit: the message seems a bit confusing to me, maybe something along the
lines of "Description of the invalid config did not include sensive=true as
expected"
--
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]
