rondagostino commented on a change in pull request #11606: URL: https://github.com/apache/kafka/pull/11606#discussion_r787045338
########## File path: core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala ########## @@ -253,4 +248,32 @@ abstract class KafkaServerTestHarness extends QuorumTestHarness { getController().kafkaController.controllerContext.topicNames.toMap } + private def createBrokers(startup: Boolean): Unit = { + // Add each broker to `servers` buffer as soon as it is created to ensure that brokers + // are shutdown cleanly in tearDown even if a subsequent broker fails to start + for (config <- configs) { + val broker = createBrokerFromConfig(config) + _brokers += broker + if (startup) { + broker.startup() Review comment: It'd debatable if it is a problem. It is not a problem if indeed the assumption is that the list of alive brokers is only valid if all brokers get created and are (now optionally) started -- if it isn't expected to be valid then putting in the effort to keep it valid doesn't strictly add anything. That being said, I agree the effort is not onerous to do so, and keeping things valid wherever possible is best in the long-run, so I'll push a commit to fix this. Thanks for this feedback. -- 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