tomtongue commented on code in PR #12853:
URL: https://github.com/apache/iceberg/pull/12853#discussion_r2051870082
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/TaskCheckHelper.java:
##########
@@ -45,61 +47,58 @@ public static void assertEquals(FileScanTask expected,
FileScanTask actual) {
assertEquals(expected.file(), actual.file());
// PartitionSpec implements its own equals method
- Assert.assertEquals("PartitionSpec doesn't match", expected.spec(),
actual.spec());
+ assertThat(actual.spec()).as("PartitionSpec doesn't
match").isEqualTo(expected.spec());
- Assert.assertEquals("starting position doesn't match", expected.start(),
actual.start());
+ assertThat(actual.start()).as("starting position doesn't
match").isEqualTo(expected.start());
- Assert.assertEquals(
- "the number of bytes to scan doesn't match", expected.start(),
actual.start());
+ assertThat(actual.start())
+ .as("the number of bytes to scan doesn't match")
+ .isEqualTo(expected.start());
// simplify comparison on residual expression via comparing toString
- Assert.assertEquals(
- "Residual expression doesn't match",
- expected.residual().toString(),
- actual.residual().toString());
+ assertThat(actual.residual())
+ .asString()
+ .as("Residual expression doesn't match")
+ .isEqualTo(expected.residual().toString());
}
public static void assertEquals(DataFile expected, DataFile actual) {
- Assert.assertEquals(
- "Should match the serialized record path", expected.location(),
actual.location());
- Assert.assertEquals(
- "Should match the serialized record format", expected.format(),
actual.format());
- Assert.assertEquals(
- "Should match the serialized record partition",
- expected.partition().get(0, Object.class),
- actual.partition().get(0, Object.class));
- Assert.assertEquals(
- "Should match the serialized record count", expected.recordCount(),
actual.recordCount());
- Assert.assertEquals(
- "Should match the serialized record size",
- expected.fileSizeInBytes(),
- actual.fileSizeInBytes());
- Assert.assertEquals(
- "Should match the serialized record value counts",
- expected.valueCounts(),
- actual.valueCounts());
- Assert.assertEquals(
- "Should match the serialized record null value counts",
- expected.nullValueCounts(),
- actual.nullValueCounts());
- Assert.assertEquals(
- "Should match the serialized record lower bounds",
- expected.lowerBounds(),
- actual.lowerBounds());
- Assert.assertEquals(
- "Should match the serialized record upper bounds",
- expected.upperBounds(),
- actual.upperBounds());
- Assert.assertEquals(
- "Should match the serialized record key metadata",
- expected.keyMetadata(),
- actual.keyMetadata());
- Assert.assertEquals(
- "Should match the serialized record offsets",
- expected.splitOffsets(),
- actual.splitOffsets());
- Assert.assertEquals(
- "Should match the serialized record offsets", expected.keyMetadata(),
actual.keyMetadata());
+ assertThat(actual.location())
+ .as("Should match the serialized record path")
+ .isEqualTo(expected.location());
+ assertThat(actual.format())
+ .as("Should match the serialized record format")
+ .isEqualTo(expected.format());
+ assertThat(actual.partition().get(0, Object.class))
+ .as("Should match the serialized record partition")
+ .isEqualTo(expected.partition().get(0, Object.class));
+ assertThat(actual.recordCount())
+ .as("Should match the serialized record count")
+ .isEqualTo(expected.recordCount());
+ assertThat(actual.fileSizeInBytes())
+ .as("Should match the serialized record size")
+ .isEqualTo(expected.fileSizeInBytes());
+ assertThat(actual.valueCounts())
+ .as("Should match the serialized record value counts")
+ .isEqualTo(expected.valueCounts());
+ assertThat(actual.nullValueCounts())
+ .as("Should match the serialized record null value counts")
+ .isEqualTo(expected.nullValueCounts());
+ assertThat(actual.lowerBounds())
+ .as("Should match the serialized record lower bounds")
+ .isEqualTo(expected.lowerBounds());
+ assertThat(actual.upperBounds())
+ .as("Should match the serialized record upper bounds")
+ .isEqualTo(expected.upperBounds());
+ assertThat(actual.keyMetadata())
+ .as("Should match the serialized record key metadata")
+ .isEqualTo(expected.keyMetadata());
+ assertThat(actual.splitOffsets())
+ .as("Should match the serialized record offsets")
+ .isEqualTo(expected.splitOffsets());
+ assertThat(actual.keyMetadata())
+ .as("Should match the serialized record offsets")
+ .isEqualTo(expected.keyMetadata());
Review Comment:
Thanks for checking this! The Spark 3.5 alos has this duplicated check, let
me remove them from both versions.
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/TaskCheckHelper.java:
##########
@@ -45,61 +47,58 @@ public static void assertEquals(FileScanTask expected,
FileScanTask actual) {
assertEquals(expected.file(), actual.file());
// PartitionSpec implements its own equals method
- Assert.assertEquals("PartitionSpec doesn't match", expected.spec(),
actual.spec());
+ assertThat(actual.spec()).as("PartitionSpec doesn't
match").isEqualTo(expected.spec());
- Assert.assertEquals("starting position doesn't match", expected.start(),
actual.start());
+ assertThat(actual.start()).as("starting position doesn't
match").isEqualTo(expected.start());
- Assert.assertEquals(
- "the number of bytes to scan doesn't match", expected.start(),
actual.start());
+ assertThat(actual.start())
+ .as("the number of bytes to scan doesn't match")
+ .isEqualTo(expected.start());
// simplify comparison on residual expression via comparing toString
- Assert.assertEquals(
- "Residual expression doesn't match",
- expected.residual().toString(),
- actual.residual().toString());
+ assertThat(actual.residual())
+ .asString()
+ .as("Residual expression doesn't match")
+ .isEqualTo(expected.residual().toString());
}
public static void assertEquals(DataFile expected, DataFile actual) {
- Assert.assertEquals(
- "Should match the serialized record path", expected.location(),
actual.location());
- Assert.assertEquals(
- "Should match the serialized record format", expected.format(),
actual.format());
- Assert.assertEquals(
- "Should match the serialized record partition",
- expected.partition().get(0, Object.class),
- actual.partition().get(0, Object.class));
- Assert.assertEquals(
- "Should match the serialized record count", expected.recordCount(),
actual.recordCount());
- Assert.assertEquals(
- "Should match the serialized record size",
- expected.fileSizeInBytes(),
- actual.fileSizeInBytes());
- Assert.assertEquals(
- "Should match the serialized record value counts",
- expected.valueCounts(),
- actual.valueCounts());
- Assert.assertEquals(
- "Should match the serialized record null value counts",
- expected.nullValueCounts(),
- actual.nullValueCounts());
- Assert.assertEquals(
- "Should match the serialized record lower bounds",
- expected.lowerBounds(),
- actual.lowerBounds());
- Assert.assertEquals(
- "Should match the serialized record upper bounds",
- expected.upperBounds(),
- actual.upperBounds());
- Assert.assertEquals(
- "Should match the serialized record key metadata",
- expected.keyMetadata(),
- actual.keyMetadata());
- Assert.assertEquals(
- "Should match the serialized record offsets",
- expected.splitOffsets(),
- actual.splitOffsets());
- Assert.assertEquals(
- "Should match the serialized record offsets", expected.keyMetadata(),
actual.keyMetadata());
+ assertThat(actual.location())
+ .as("Should match the serialized record path")
+ .isEqualTo(expected.location());
+ assertThat(actual.format())
+ .as("Should match the serialized record format")
+ .isEqualTo(expected.format());
+ assertThat(actual.partition().get(0, Object.class))
+ .as("Should match the serialized record partition")
+ .isEqualTo(expected.partition().get(0, Object.class));
+ assertThat(actual.recordCount())
+ .as("Should match the serialized record count")
+ .isEqualTo(expected.recordCount());
+ assertThat(actual.fileSizeInBytes())
+ .as("Should match the serialized record size")
+ .isEqualTo(expected.fileSizeInBytes());
+ assertThat(actual.valueCounts())
+ .as("Should match the serialized record value counts")
+ .isEqualTo(expected.valueCounts());
+ assertThat(actual.nullValueCounts())
+ .as("Should match the serialized record null value counts")
+ .isEqualTo(expected.nullValueCounts());
+ assertThat(actual.lowerBounds())
+ .as("Should match the serialized record lower bounds")
+ .isEqualTo(expected.lowerBounds());
+ assertThat(actual.upperBounds())
+ .as("Should match the serialized record upper bounds")
+ .isEqualTo(expected.upperBounds());
+ assertThat(actual.keyMetadata())
+ .as("Should match the serialized record key metadata")
+ .isEqualTo(expected.keyMetadata());
+ assertThat(actual.splitOffsets())
+ .as("Should match the serialized record offsets")
+ .isEqualTo(expected.splitOffsets());
+ assertThat(actual.keyMetadata())
+ .as("Should match the serialized record offsets")
+ .isEqualTo(expected.keyMetadata());
Review Comment:
Thanks for checking this! The Spark 3.5 also has this duplicated check, let
me remove them from both versions.
--
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]
