rodmeneses commented on code in PR #9185:
URL: https://github.com/apache/iceberg/pull/9185#discussion_r1420810926
##########
flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/TestHelpers.java:
##########
@@ -523,89 +510,103 @@ public static void assertEquals(ManifestFile expected,
ManifestFile actual) {
if (expected == actual) {
return;
}
- Assert.assertTrue("Should not be null.", expected != null && actual !=
null);
- Assert.assertEquals("Path must match", expected.path(), actual.path());
- Assert.assertEquals("Length must match", expected.length(),
actual.length());
- Assert.assertEquals("Spec id must match", expected.partitionSpecId(),
actual.partitionSpecId());
- Assert.assertEquals("ManifestContent must match", expected.content(),
actual.content());
- Assert.assertEquals(
- "SequenceNumber must match", expected.sequenceNumber(),
actual.sequenceNumber());
- Assert.assertEquals(
- "MinSequenceNumber must match", expected.minSequenceNumber(),
actual.minSequenceNumber());
- Assert.assertEquals("Snapshot id must match", expected.snapshotId(),
actual.snapshotId());
- Assert.assertEquals(
- "Added files flag must match", expected.hasAddedFiles(),
actual.hasAddedFiles());
- Assert.assertEquals(
- "Added files count must match", expected.addedFilesCount(),
actual.addedFilesCount());
- Assert.assertEquals(
- "Added rows count must match", expected.addedRowsCount(),
actual.addedRowsCount());
- Assert.assertEquals(
- "Existing files flag must match", expected.hasExistingFiles(),
actual.hasExistingFiles());
- Assert.assertEquals(
- "Existing files count must match",
- expected.existingFilesCount(),
- actual.existingFilesCount());
- Assert.assertEquals(
- "Existing rows count must match", expected.existingRowsCount(),
actual.existingRowsCount());
- Assert.assertEquals(
- "Deleted files flag must match", expected.hasDeletedFiles(),
actual.hasDeletedFiles());
- Assert.assertEquals(
- "Deleted files count must match", expected.deletedFilesCount(),
actual.deletedFilesCount());
- Assert.assertEquals(
- "Deleted rows count must match", expected.deletedRowsCount(),
actual.deletedRowsCount());
+ assertThat(expected).isNotNull();
+ assertThat(actual).isNotNull();
+ assertThat(actual.path()).as("Path must match").isEqualTo(expected.path());
+ assertThat(actual.length()).as("Length must
match").isEqualTo(expected.length());
+ assertThat(actual.partitionSpecId())
+ .as("Spec id must match")
+ .isEqualTo(expected.partitionSpecId());
+ assertThat(actual.content()).as("ManifestContent must
match").isEqualTo(expected.content());
+ assertThat(actual.sequenceNumber())
+ .as("SequenceNumber must match")
+ .isEqualTo(expected.sequenceNumber());
+ assertThat(actual.minSequenceNumber())
+ .as("MinSequenceNumber must match")
+ .isEqualTo(expected.minSequenceNumber());
+ assertThat(actual.snapshotId()).as("Snapshot id must
match").isEqualTo(expected.snapshotId());
+ assertThat(actual.hasAddedFiles())
+ .as("Added files flag must match")
+ .isEqualTo(expected.hasAddedFiles());
+ assertThat(actual.addedFilesCount())
+ .as("Added files count must match")
+ .isEqualTo(expected.addedFilesCount());
+ assertThat(actual.addedRowsCount())
+ .as("Added rows count must match")
+ .isEqualTo(expected.addedRowsCount());
+ assertThat(actual.hasExistingFiles())
+ .as("Existing files flag must match")
+ .isEqualTo(expected.hasExistingFiles());
+ assertThat(actual.existingFilesCount())
+ .as("Existing files count must match")
+ .isEqualTo(expected.existingFilesCount());
+ assertThat(actual.existingRowsCount())
+ .as("Existing rows count must match")
+ .isEqualTo(expected.existingRowsCount());
+ assertThat(actual.hasDeletedFiles())
+ .as("Deleted files flag must match")
+ .isEqualTo(expected.hasDeletedFiles());
+ assertThat(actual.deletedFilesCount())
+ .as("Deleted files count must match")
+ .isEqualTo(expected.deletedFilesCount());
+ assertThat(actual.deletedRowsCount())
+ .as("Deleted rows count must match")
+ .isEqualTo(expected.deletedRowsCount());
List<ManifestFile.PartitionFieldSummary> expectedSummaries =
expected.partitions();
List<ManifestFile.PartitionFieldSummary> actualSummaries =
actual.partitions();
- Assert.assertEquals(
- "PartitionFieldSummary size does not match",
- expectedSummaries.size(),
- actualSummaries.size());
+ assertThat(actualSummaries)
+ .as("PartitionFieldSummary size does not match")
+ .hasSameSizeAs(expectedSummaries.size());
Review Comment:
I dont think this is the right way to use `hasSameSizeAs` assertions.
the parameter of
```java
.hasSameSizeAs
```
shouldn't be a number, but instead another collection.
Please make changes accordingly to the rest of the codebase
--
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
