nastra commented on code in PR #9341:
URL: https://github.com/apache/iceberg/pull/9341#discussion_r1432758094


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/GenericsHelpers.java:
##########
@@ -116,90 +116,85 @@ private static void assertEqualsSafe(Type type, Object 
expected, Object actual)
       case LONG:
       case FLOAT:
       case DOUBLE:
-        Assert.assertEquals("Primitive value should be equal to expected", 
expected, actual);
+        assertThat(actual).as("Primitive value should be equal to 
expected").isEqualTo(expected);
         break;
       case DATE:
-        Assertions.assertThat(expected)
-            .as("Should expect a LocalDate")
-            .isInstanceOf(LocalDate.class);
-        Assertions.assertThat(actual).as("Should be a 
Date").isInstanceOf(Date.class);
-        Assert.assertEquals(
-            "ISO-8601 date should be equal", expected.toString(), 
actual.toString());
+        assertThat(expected).as("Should expect a 
LocalDate").isInstanceOf(LocalDate.class);
+        assertThat(actual).as("Should be a Date").isInstanceOf(Date.class);
+        assertThat(actual.toString())
+            .as("ISO-8601 date should be equal")
+            .isEqualTo(String.valueOf(expected));
         break;
       case TIMESTAMP:
         Types.TimestampType timestampType = (Types.TimestampType) type;
         if (timestampType.shouldAdjustToUTC()) {
           // Timestamptz
-          Assertions.assertThat(actual).as("Should be a 
Timestamp").isInstanceOf(Timestamp.class);
+          assertThat(actual).as("Should be a 
Timestamp").isInstanceOf(Timestamp.class);
           Timestamp ts = (Timestamp) actual;
           // milliseconds from nanos has already been added by getTime
           OffsetDateTime actualTs =
               EPOCH.plusNanos((ts.getTime() * 1_000_000) + (ts.getNanos() % 
1_000_000));
 
-          Assertions.assertThat(expected)
+          assertThat(expected)
               .as("Should expect an OffsetDateTime")
               .isInstanceOf(OffsetDateTime.class);
-          Assert.assertEquals("Timestamp should be equal", expected, actualTs);
+
+          assertThat(actualTs).as("Timestamp should be 
equal").isEqualTo(expected);
         } else {
           // Timestamp
-          Assertions.assertThat(actual)
-              .as("Should be a LocalDateTime")
-              .isInstanceOf(LocalDateTime.class);
-          LocalDateTime ts = (LocalDateTime) actual;
+          assertThat(actual).as("Should be a 
LocalDateTime").isInstanceOf(LocalDateTime.class);
 
-          Assertions.assertThat(expected)
+          assertThat(expected)
               .as("Should expect an LocalDateTime")
               .isInstanceOf(LocalDateTime.class);
-          Assert.assertEquals("Timestamp should be equal", expected, ts);
+
+          assertThat(actual).as("Timestamp should be 
equal").isEqualTo(expected);
         }
         break;
       case STRING:
-        Assertions.assertThat(actual).as("Should be a 
String").isInstanceOf(String.class);
-        Assert.assertEquals("Strings should be equal", 
String.valueOf(expected), actual);
+        assertThat(actual).as("Should be a String").isInstanceOf(String.class);
+        assertThat(actual.toString())
+            .as("Strings should be equal")
+            .isEqualTo(String.valueOf(expected));
         break;
       case UUID:
-        Assertions.assertThat(expected).as("Should expect a 
UUID").isInstanceOf(UUID.class);
-        Assertions.assertThat(actual).as("Should be a 
String").isInstanceOf(String.class);
-        Assert.assertEquals("UUID string representation should match", 
expected.toString(), actual);
+        assertThat(expected).as("Should expect a 
UUID").isInstanceOf(UUID.class);
+        assertThat(actual).as("Should be a String").isInstanceOf(String.class);
+        assertThat(actual.toString())
+            .as("UUID string representation should match")
+            .isEqualTo(String.valueOf(expected));
         break;
       case FIXED:
-        Assertions.assertThat(expected).as("Should expect a 
byte[]").isInstanceOf(byte[].class);
-        Assertions.assertThat(actual).as("Should be a 
byte[]").isInstanceOf(byte[].class);
-        Assert.assertArrayEquals("Bytes should match", (byte[]) expected, 
(byte[]) actual);
+        assertThat(expected).as("Should expect a 
byte[]").isInstanceOf(byte[].class);
+        assertThat(actual).as("Should be a byte[]").isInstanceOf(byte[].class);
+        assertThat(actual).as("Bytes should match").isEqualTo(expected);
         break;
       case BINARY:
-        Assertions.assertThat(expected)
-            .as("Should expect a ByteBuffer")
-            .isInstanceOf(ByteBuffer.class);
-        Assertions.assertThat(actual).as("Should be a 
byte[]").isInstanceOf(byte[].class);
-        Assert.assertArrayEquals(
-            "Bytes should match", ((ByteBuffer) expected).array(), (byte[]) 
actual);
+        assertThat(expected).as("Should expect a 
ByteBuffer").isInstanceOf(ByteBuffer.class);
+        assertThat(actual).as("Should be a byte[]").isInstanceOf(byte[].class);
+        assertThat((byte[]) actual)

Review Comment:
   cast is probably not needed



-- 
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

Reply via email to