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

Reply via email to