nastra commented on code in PR #9367:
URL: https://github.com/apache/iceberg/pull/9367#discussion_r1435510145
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestDeleteReachableFilesAction.java:
##########
@@ -345,11 +345,10 @@ public void testIgnoreMetadataFilesNotFound() {
DeleteOrphanFiles.Result result =
sparkActions().deleteOrphanFiles(table).olderThan(System.currentTimeMillis()).execute();
- Assert.assertEquals("Should delete 1 file", 1,
Iterables.size(result.orphanFileLocations()));
- Assert.assertTrue(
- "Should remove v1 file",
- StreamSupport.stream(result.orphanFileLocations().spliterator(), false)
- .anyMatch(file -> file.contains("v1.metadata.json")));
+ assertThat(result.orphanFileLocations()).as("Should delete 1
file").size().isOne();
Review Comment:
it's probably better to use `hasSize(..)` here and everywhere else
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestExpireSnapshotsAction.java:
##########
@@ -188,8 +187,10 @@ public void testFilesCleaned() throws Exception {
ExpireSnapshots.Result results =
SparkActions.get().expireSnapshots(table).expireOlderThan(end).execute();
- Assert.assertEquals(
- "Table does not have 1 snapshot after expiration", 1,
Iterables.size(table.snapshots()));
+ assertThat(table.snapshots())
+ .as("Table does not have 1 snapshot after expiration")
+ .size()
Review Comment:
same as previously mentioned. please also update all the other places
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestDeleteReachableFilesAction.java:
##########
@@ -133,30 +132,30 @@ private void checkRemoveFilesResults(
long expectedManifestListsDeleted,
long expectedOtherFilesDeleted,
DeleteReachableFiles.Result results) {
- Assert.assertEquals(
- "Incorrect number of manifest files deleted",
- expectedManifestsDeleted,
- results.deletedManifestsCount());
- Assert.assertEquals(
- "Incorrect number of datafiles deleted",
- expectedDatafiles,
- results.deletedDataFilesCount());
- Assert.assertEquals(
- "Incorrect number of position delete files deleted",
- expectedPosDeleteFiles,
- results.deletedPositionDeleteFilesCount());
- Assert.assertEquals(
- "Incorrect number of equality delete files deleted",
- expectedEqDeleteFiles,
- results.deletedEqualityDeleteFilesCount());
- Assert.assertEquals(
- "Incorrect number of manifest lists deleted",
- expectedManifestListsDeleted,
- results.deletedManifestListsCount());
- Assert.assertEquals(
- "Incorrect number of other lists deleted",
- expectedOtherFilesDeleted,
- results.deletedOtherFilesCount());
+
Review Comment:
unnecessary newline
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestExpireSnapshotsAction.java:
##########
@@ -482,10 +477,10 @@ public void testExpireOlderThanMultipleCalls() {
.expireOlderThan(thirdSnapshot.timestampMillis())
.execute();
- Assert.assertEquals(
- "Should have one snapshots.", 1,
Lists.newArrayList(table.snapshots()).size());
- Assert.assertNull(
- "Second snapshot should not present.",
table.snapshot(secondSnapshot.snapshotId()));
+ assertThat(table.snapshots()).as("Should have one
snapshots.").size().isOne();
Review Comment:
```suggestion
assertThat(table.snapshots()).as("Should have one snapshot").hasSize(1);
```
--
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
