danielcweeks commented on code in PR #11495: URL: https://github.com/apache/iceberg/pull/11495#discussion_r1837076611
########## spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteManifestsAction.java: ########## @@ -956,6 +956,62 @@ public void testRewriteLargeDeleteManifestsPartitionedTable() throws IOException assertThat(deleteManifests).hasSizeGreaterThanOrEqualTo(2); } + @TestTemplate + public void testUpgradeToV3() throws IOException { Review Comment: I feel a little awkward about the scope of this test. It's testing upgrade and manifest rewrite, but there are specific aspects that are included. I feel like there are more scenarios to cover here (e.g. writing a DV to an existing file that has positional delete after the upgrade.). I would say either refine the name to be more explicit (e.g. `testUpgradeToV3WithExistingV2Deletes`) so that we can add more test cases for the upgrade path. -- 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