abstractdog commented on PR #12427: URL: https://github.com/apache/iceberg/pull/12427#issuecomment-2763554614
@pvary : added a testcase in TestRemoveSnapshots to check if the supplied executor service is used the same wasn't needed in case of SnapshotProducer as it already checked in another unit test, here: https://github.com/apache/iceberg/blob/d5971429ea903be873b5884c64a3dd41076179ea/core/src/test/java/org/apache/iceberg/TestRewriteManifests.java#L91 recap: the main point of the patch was to prevent the early initialization of this.workerPool, and unit testing it is only possible with reflection, given the fact that it's private, not sure if I can contribute code like that pseudo-code ``` SnapshotProducer snapshotProducer = new SnapshotProducer(): assert(snapshotProducer.workerPool == null) // <-- reflection ``` -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
