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


##########
flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/TestHelpers.java:
##########
@@ -337,82 +334,72 @@ private static void assertAvroEquals(
     if (expected == null && actual == null) {
       return;
     }
-
-    Assert.assertTrue(
-        "expected and actual should be both null or not null", expected != 
null && actual != null);
+    assertThat(expected).isNotNull();
+    assertThat(actual).isNotNull();
 
     switch (type.typeId()) {
       case BOOLEAN:
       case INTEGER:
       case LONG:
       case FLOAT:
       case DOUBLE:
-        Assertions.assertThat(expected)
+        assertThat(expected)
             .as("Should expect a " + type.typeId().javaClass())
             .isInstanceOf(type.typeId().javaClass());
-        Assertions.assertThat(actual)
+        assertThat(actual)
             .as("Should expect a " + type.typeId().javaClass())
             .isInstanceOf(type.typeId().javaClass());
-        Assert.assertEquals(type.typeId() + " value should be equal", 
expected, actual);
+        assertThat(actual).as(type.typeId() + " value should be 
equal").isEqualTo(expected);
         break;
       case STRING:
-        Assertions.assertThat(expected)
-            .as("Should expect a CharSequence")
-            .isInstanceOf(CharSequence.class);
-        Assertions.assertThat(actual)
-            .as("Should expect a CharSequence")
-            .isInstanceOf(CharSequence.class);
-        Assert.assertEquals("string should be equal", expected.toString(), 
actual.toString());
+        assertThat(expected).as("Should expect a 
CharSequence").isInstanceOf(CharSequence.class);
+        assertThat(actual).as("Should expect a 
CharSequence").isInstanceOf(CharSequence.class);
+        assertThat(actual.toString()).as("string should be 
equal").isEqualTo(expected.toString());
         break;
       case DATE:
-        Assertions.assertThat(expected).as("Should expect a 
Date").isInstanceOf(LocalDate.class);
+        assertThat(expected).as("Should expect a 
Date").isInstanceOf(LocalDate.class);
         LocalDate date = DateTimeUtil.dateFromDays((int) actual);
-        Assert.assertEquals("date should be equal", expected, date);
+        assertThat(date).as("date should be equal").isEqualTo(expected);
         break;
       case TIME:
-        Assertions.assertThat(expected)
-            .as("Should expect a LocalTime")
-            .isInstanceOf(LocalTime.class);
+        assertThat(expected).as("Should expect a 
LocalTime").isInstanceOf(LocalTime.class);
         int milliseconds = (int) (((LocalTime) expected).toNanoOfDay() / 
1000_000);
-        Assert.assertEquals("time millis should be equal", milliseconds, 
actual);
+        assertThat(actual).as("time millis should be 
equal").isEqualTo(milliseconds);
         break;
       case TIMESTAMP:
         if (((Types.TimestampType) type).shouldAdjustToUTC()) {
-          Assertions.assertThat(expected)
+          assertThat(expected)
               .as("Should expect a OffsetDataTime")
               .isInstanceOf(OffsetDateTime.class);
           OffsetDateTime ts = (OffsetDateTime) expected;
-          Assert.assertEquals(
-              "OffsetDataTime should be equal",
-              ts.toLocalDateTime(),
-              ((TimestampData) actual).toLocalDateTime());
+          assertThat(((TimestampData) actual).toLocalDateTime())
+              .as("OffsetDataTime should be equal")
+              .isEqualTo(ts.toLocalDateTime());
         } else {
-          Assertions.assertThat(expected)
+          assertThat(expected)
               .as("Should expect a LocalDataTime")
               .isInstanceOf(LocalDateTime.class);
           LocalDateTime ts = (LocalDateTime) expected;
-          Assert.assertEquals(
-              "LocalDataTime should be equal", ts, ((TimestampData) 
actual).toLocalDateTime());
+          assertThat(((TimestampData) actual).toLocalDateTime())
+              .as("LocalDataTime should be equal")
+              .isEqualTo(ts);
         }
         break;
       case BINARY:
-        Assertions.assertThat(expected)
+        assertThat(expected)

Review Comment:
   looks like the actual/expected is the wrong way here as well. Can you please 
check all the other places in this file to make sure that we're using 
`assertThat(actual)...isEqualTo(expected)



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