Core, Test: Parsing and Writing Tests for V3 Metadata#12025
Core, Test: Parsing and Writing Tests for V3 Metadata#12025HonahX wants to merge 14 commits intoapache:mainfrom
Conversation
Parameterize TestTableMetadata
# Conflicts: # core/src/test/java/org/apache/iceberg/TestTableMetadata.java
| @Test | ||
| public void testInvalidMainBranch() throws IOException { | ||
| @ParameterizedTest | ||
| @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") |
There was a problem hiding this comment.
Instead of this, we could also put it as:
| @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") | |
| @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.
There was a problem hiding this comment.
I'd suggest to do the same as mentioned in #11948 (comment) and parameterize this at the class level
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
This used to be part of #11947 that does not include new unsafe builders for Snapshot and TableMetadata.