nastra commented on code in PR #8070:
URL: https://github.com/apache/iceberg/pull/8070#discussion_r1264369043
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewritePositionDeleteFiles.java:
##########
@@ -221,7 +221,7 @@ private <T> void testDanglingDelete(int numDataFiles)
throws Exception {
.execute();
List<DeleteFile> newDeleteFiles = deleteFiles(table);
- Assert.assertEquals("Should have removed all dangling delete files", 0,
newDeleteFiles.size());
+ assertThat(newDeleteFiles.size()).as("Remaining dangling
deletes").isEqualTo(0);
Review Comment:
```suggestion
assertThat(newDeleteFiles).as("Remaining dangling deletes").isEmpty();
```
the advantage of having the assertion like that is that it will show the
content of `newDeleteFiles` when the assertion fails, making it easier to debug
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewritePositionDeleteFiles.java:
##########
@@ -361,48 +358,43 @@ private void checkResult(
List<DeleteFile> rewrittenDeletes,
List<DeleteFile> newDeletes,
int expectedGroups) {
- Assert.assertEquals(
- "Expected rewritten delete file count does not match",
- rewrittenDeletes.size(),
- result.rewrittenDeleteFilesCount());
- Assert.assertEquals(
- "Expected new delete file count does not match",
- newDeletes.size(),
- result.addedDeleteFilesCount());
- Assert.assertEquals(
- "Expected rewritten delete byte count does not match",
- size(rewrittenDeletes),
- result.rewrittenBytesCount());
- Assert.assertEquals(
- "Expected new delete byte count does not match",
- size(newDeletes),
- result.addedBytesCount());
-
- Assert.assertEquals(
- "Expected rewrite group count does not match",
- expectedGroups,
- result.rewriteResults().size());
- Assert.assertEquals(
- "Expected rewritten delete file count in all groups to match",
- rewrittenDeletes.size(),
- result.rewriteResults().stream()
- .mapToInt(FileGroupRewriteResult::rewrittenDeleteFilesCount)
- .sum());
- Assert.assertEquals(
- "Expected added delete file count in all groups to match",
- newDeletes.size(),
- result.rewriteResults().stream()
- .mapToInt(FileGroupRewriteResult::addedDeleteFilesCount)
- .sum());
- Assert.assertEquals(
- "Expected rewritten delete bytes in all groups to match",
- size(rewrittenDeletes),
- result.rewriteResults().stream()
- .mapToLong(FileGroupRewriteResult::rewrittenBytesCount)
- .sum());
- Assert.assertEquals(
- "Expected added delete bytes in all groups to match",
- size(newDeletes),
-
result.rewriteResults().stream().mapToLong(FileGroupRewriteResult::addedBytesCount).sum());
+ assertThat(result.rewrittenDeleteFilesCount())
+ .as("Rewritten delete files")
+ .isEqualTo(rewrittenDeletes.size());
+ assertThat(result.addedDeleteFilesCount())
+ .as("Added delete files")
+ .isEqualTo(newDeletes.size());
+ assertThat(result.rewrittenBytesCount())
+ .as("Rewritten delete bytes")
+ .isEqualTo(size(rewrittenDeletes));
+ assertThat(result.addedBytesCount()).as("New Delete byte
count").isEqualTo(size(newDeletes));
+
+ assertThat(result.rewriteResults().size())
Review Comment:
```suggestion
assertThat(result.rewriteResults()).as("Rewritten group
count").hasSize(expectedGroups);
```
same advantage as I mentioned above as it allows easier debugging if the
assertion ever fails
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
