Fokko commented on code in PR #12025:
URL: https://github.com/apache/iceberg/pull/12025#discussion_r1925009567


##########
core/src/test/java/org/apache/iceberg/TestTableMetadata.java:
##########
@@ -314,8 +319,11 @@ public void testBackwardCompat() throws Exception {
     assertThat(metadata.snapshot(previousSnapshotId).schemaId()).isNull();
   }
 
-  @Test
-  public void testInvalidMainBranch() throws IOException {
+  @ParameterizedTest
+  @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS")

Review Comment:
   Instead of this, we could also put it as: 
   ```suggestion
     @ValueSource(ints = MIN_FORMAT_VERSION_V2)
   ```
   
   This avoids having to rely on the `assumeThat`, which I feel makes it harder 
to interpret the test. I'm not super strong on this, though. Maybe our 
test-connaisseur @nastra can share his opinion here.



-- 
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