divijvaidya commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437034332
########## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ########## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time.scheduler, logManager = logManager, - quotaManagers = QuotaFactory.instantiate(config, metrics, time, ""), + quotaManagers = quotaManager, metadataCache = MetadataCache.zkMetadataCache(config.brokerId, config.interBrokerProtocolVersion), logDirFailureChannel = new LogDirFailureChannel(config.logDirs.size), alterPartitionManager = alterPartitionManager, threadNamePrefix = Option(this.getClass.getName)) - logManager.startup(Set.empty[String]) - - // Create a hosted topic, a hosted topic that will become stray, and a stray topic - val validLogs = createHostedLogs("hosted-topic", numLogs = 2, replicaManager).toSet - createHostedLogs("hosted-stray", numLogs = 10, replicaManager).toSet - createStrayLogs(10, logManager) + try { + logManager.startup(Set.empty[String]) - val allReplicasFromLISR = Set(new TopicPartition("hosted-topic", 0), new TopicPartition("hosted-topic", 1)) + // Create a hosted topic, a hosted topic that will become stray, and a stray topic + val validLogs = createHostedLogs("hosted-topic", numLogs = 2, replicaManager).toSet + createHostedLogs("hosted-stray", numLogs = 10, replicaManager).toSet + createStrayLogs(10, logManager) - replicaManager.updateStrayLogs(replicaManager.findStrayPartitionsFromLeaderAndIsr(allReplicasFromLISR)) + val allReplicasFromLISR = Set(new TopicPartition("hosted-topic", 0), new TopicPartition("hosted-topic", 1)) - assertEquals(validLogs, logManager.allLogs.toSet) - assertEquals(validLogs.size, replicaManager.partitionCount.value) + replicaManager.updateStrayLogs(replicaManager.findStrayPartitionsFromLeaderAndIsr(allReplicasFromLISR)) - replicaManager.shutdown() - logManager.shutdown() + assertEquals(validLogs, logManager.allLogs.toSet) + assertEquals(validLogs.size, replicaManager.partitionCount.value) + } finally { + replicaManager.shutdown() + logManager.shutdown() Review Comment: That is fair. In that case, we should perhaps use `public static void closeQuietly(AutoCloseable closeable, String name, AtomicReference<Throwable> firstException)` and end the finally with something similar to ``` if (firstException != null) { throw new KafkaException("Failed to close", exception); } ``` -- 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