Migrate other files in Core to JUnit5 #10027
Conversation
tomtongue
changed the title
|
@nastra Could you review this PR? Just let you know, the following two classes in
|
|
|
||
| @Rule public TemporaryFolder temp = new TemporaryFolder(); | ||
| private Table table = null; | ||
| // @Rule public TemporaryFolder temp = new TemporaryFolder(); |
There was a problem hiding this comment.
Thanks. Remove this comment.
| .allManifests(table.io()) | ||
| .get(0) | ||
| .path(), // manifest contained only deletes, was dropped | ||
| FILE_A.path().toString() // deleted |
There was a problem hiding this comment.
why is toString() needed here?
There was a problem hiding this comment.
This is not needed. Remove toString() here.
| .get(0) | ||
| .path(), // manifest was rewritten for delete | ||
| secondSnapshot.manifestListLocation(), // snapshot expired | ||
| FILE_A.path())); |
There was a problem hiding this comment.
original comment is missing here: // deleted
| .isEqualTo( | ||
| Sets.newHashSet( | ||
| secondSnapshot.manifestListLocation(), // snapshot expired | ||
| Iterables.getOnlyElement(secondSnapshotManifests) |
There was a problem hiding this comment.
can we avoid using Iterables here?
| assertThat(table.currentSnapshot()).isEqualTo(thirdSnapshot); | ||
| assertThat(table.snapshots()).hasSize(1); | ||
| assertThat(deletedFiles) | ||
| .isEqualTo( |
There was a problem hiding this comment.
The order of firstSnapshot.manifestLocation and secondSnapshot.manifestLocation depends on the hashset values, so the order is not always the same. So, I believe this part needs to use isEqualTo, or containsExactlyInAnyOrder.
There was a problem hiding this comment.
let's use containsExactlyInAnyOrder then
You mean migrated in a separate PR? Also I see a few more files in
|
I will submit a PR for the two classes There are also some files to be migrated in some sub-packages (like |
tomtongue
force-pushed
the
mig-junit5-core-others
branch
from
0cdeef9 to
2543b29
Compare
2 participants
Migrate the following test classes in iceberg-core to JUnit 5 and AssertJ style for #9085.
Current Progress
TestMetricsModes.javaTestMicroBatchBuilder.javaTestRemoveSnapshots.javaTestRewriteManifests.javaTestRowDelta.javaTestSequenceNumberForV2Table.javaTestSingleValueParser.javaTestSortOrder.javaTestSortOrderParser.javaTestSplitPlanning.javaTestV1ToV2RowDeltaDelete.javaV2TableTestBase.javaRefactoring (related to #10014 (comment)):
TestSchemaAndMappingUpdate.javaTestSchemaUpdate