Relocate commitTransaction metadata cleanup tests out of CommitTransactionEventTest#5041
Draft
sririshindra wants to merge 1 commit into
Draft
Conversation
…ctionEventTest CommitTransactionEventTest is dedicated to Polaris event emission, but two regression tests for metadata-file lifecycle landed there: - testCommitTransactionDoesNotDeleteOldMetadataFilesDuringTransaction (apache#4920) is rewritten as API-level tests in PolarisRestCatalogIntegrationBase (a failure variant plus the "positive" success variant suggested in review), using the RESTCatalog client and the client-side FileIO hook so the tests work across all storage backends: apache#4920 (comment) - testCommitTransactionCleansUpMetadataOnFailure (apache#4934) injects a Mockito spy into the metastore manager and cannot run as an integration test; it moves verbatim to a new CommitTransactionMetadataCleanupTest, restoring the recursive metadata-file scan its assertion relies on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #4920, as discussed in #4920 (comment).
CommitTransactionEventTestis dedicated to Polaris event emission, but two regression tests for metadata-file lifecycle landed there:testCommitTransactionDoesNotDeleteOldMetadataFilesDuringTransaction(from Fix data corruption via premature metadata deletion in commitTransaction #4920) is rewritten as API-level tests inPolarisRestCatalogIntegrationBasetestCommitTransactionDoesNotDeleteOldMetadataFilesOnFailure— a failed multi-table transaction must not delete metadata files evicted from the metadata log (write.metadata.delete-after-commit.enabled=true).testCommitTransactionDeletesOldMetadataFilesAfterSuccess— the "positive" counterpart also suggested in review: once the transaction commits, the deferred cleanup deletes the evicted metadata file.Both use the
RESTCatalogclient with the failure-injection pattern of the neighboring transaction tests, and check file existence through the class'sinitializeClientFileIO(...)hook (instead of walking localjava.nio.filepaths) so they work across the FILE/S3/MinIO/GCS/ADLS suites.testCommitTransactionCleansUpMetadataOnFailure(from Cleanup metadata files on transaction failure #4934) injects a Mockito spy viaTestServices.metaStoreManagerDecoratorto simulate a CAS failure in the final batch update, so it cannot run as an integration test. It moves verbatim to a newCommitTransactionMetadataCleanupTestin the same package. The move also restores the recursiveFiles.walkmetadata scan; the non-recursiveFiles.listvariant onmainmade this test's before/after comparison on the temp-dir root vacuously pass.CommitTransactionEventTestnow contains only the three event-emission tests.