[PR] release testkit resources when cleanup or startup fails [pekko-connectors-kafka]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning opened a new pull request, #651:
URL: https://github.com/apache/pekko-connectors-kafka/pull/651

   ### Motivation
   The testkit cleanup paths stop at the first failure, so one failing release leaks the resources that would have been released after it.
   
   - `KafkaSpec.cleanUp` closes the test producer, then the admin client, then shuts down the actor system. `Producer.close` blocks for up to 60 seconds and can fail; when it does, the admin client is never closed and the actor system is never shut down. In a suite that reuses a broker across test classes that leaks an `Admin` client plus a full `ActorSystem` (dispatcher threads) for every failing class. This is `src/main` of the published testkit, so it affects downstream users' test runs as well.
   - `KafkaTestKit.cleanUpAdminClient` closes the client and only then drops the reference, so a failing close leaves a closed `Admin` client in the field for the next `setUpAdminClient` to reuse.
   - `KafkaContainerCluster.start` catches everything and rethrows it as a `RuntimeException` without stopping the containers that already started. `TestcontainersKafka.stopCluster` is guarded by `kafkaPortInternal != -1`, which was never set because `start()` threw, so the brokers stay up until Ryuk reaps them.
   
   ### Modification
   - `KafkaSpec.cleanUp` releases every resource and then rethrows the first failure.
   - `KafkaTestKit.cleanUpAdminClient` drops the reference before closing.
   - `KafkaContainerCluster.start` stops the cluster when startup fails, attaching a failing stop as a suppressed exception before rethrowing.
   
   ### Result
   A failure while releasing one testkit resource no longer leaks the others, and a failed cluster start no longer leaves broker containers running.
   
   ### Tests
   - `sbt "tests/Test/testOnly org.apache.pekko.kafka.testkit.scaladsl.KafkaSpecCleanUpSpec"` — 1 succeeded, 0 failed. The new test fails without the fix: `cleanUp` rethrows immediately, so the admin client is still open and the actor system still running.
   - `sbt testkit/mimaReportBinaryIssues` — success
   - `sbt javafmtCheckAll scalafmtCheckAll headerCheckAll` — success, run with JDK 17
   - The `KafkaContainerCluster.start` change is not covered by a test: it would need a Docker cluster that fails to start partway through.
   
   ### References
   None - found while auditing the main sources for resource leaks


-- 
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: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org