amogh-jahagirdar commented on code in PR #10265: URL: https://github.com/apache/iceberg/pull/10265#discussion_r1589798517
########## core/src/test/java/org/apache/iceberg/TestRewriteManifests.java: ########## @@ -266,31 +280,38 @@ public void testReplaceManifestsWithFilter() throws IOException { files.iterator(), statuses(ManifestEntry.Status.EXISTING, ManifestEntry.Status.EXISTING)); validateManifestEntries( - manifests.get(1), ids(appendIdA), files(FILE_A), statuses(ManifestEntry.Status.ADDED)); + manifests.get(1), + ids(appendA.snapshotId()), + files(FILE_A), + statuses(ManifestEntry.Status.ADDED)); } @TestTemplate public void testReplaceManifestsMaxSize() { Table table = load(); - table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); - long appendId = table.currentSnapshot().snapshotId(); - - assertThat(table.currentSnapshot().allManifests(table.io())).hasSize(1); + Snapshot append = + commit(table, table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B), branch); + assertThat(append.allManifests(table.io())).hasSize(1); // cluster by constant will combine manifests into one but small target size will create one per // entry BaseRewriteManifests rewriteManifests = spy((BaseRewriteManifests) table.rewriteManifests()); when(rewriteManifests.getManifestTargetSizeBytes()).thenReturn(1L); - rewriteManifests.clusterBy(file -> "file").commit(); - - List<ManifestFile> manifests = table.currentSnapshot().allManifests(table.io()); + Snapshot rewritten = commit(table, rewriteManifests.clusterBy(file -> "file"), branch); + List<ManifestFile> manifests = rewritten.allManifests(table.io()); assertThat(manifests).hasSize(2); manifests.sort(Comparator.comparing(ManifestFile::path)); validateManifestEntries( - manifests.get(0), ids(appendId), files(FILE_A), statuses(ManifestEntry.Status.EXISTING)); + manifests.get(0), + ids(append.snapshotId()), + files(FILE_A), + statuses(ManifestEntry.Status.EXISTING)); validateManifestEntries( - manifests.get(1), ids(appendId), files(FILE_B), statuses(ManifestEntry.Status.EXISTING)); + manifests.get(1), + ids(append.snapshotId()), + files(FILE_B), + statuses(ManifestEntry.Status.EXISTING)); } @TestTemplate Review Comment: Still working on updating all the tests to validate the branch operations -- 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