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


##########
core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java:
##########
@@ -1158,123 +1137,92 @@ private static void assertEqualsSetStatistics(
               BlobMetadata expectedBlob = pair.first();
               BlobMetadata actualBlob = pair.second();
 
-              Assert.assertEquals(
-                  "Expected blob type should be equal", expectedBlob.type(), 
actualBlob.type());
-              Assert.assertEquals(
-                  "Expected blob fields should be equal",
-                  expectedBlob.fields(),
-                  actualBlob.fields());
-              Assert.assertEquals(
-                  "Expected blob source snapshot ID should be equal",
-                  expectedBlob.sourceSnapshotId(),
-                  actualBlob.sourceSnapshotId());
-              Assert.assertEquals(
-                  "Expected blob source snapshot sequence number should be 
equal",
-                  expectedBlob.sourceSnapshotSequenceNumber(),
-                  actualBlob.sourceSnapshotSequenceNumber());
-              Assert.assertEquals(
-                  "Expected blob properties should be equal",
-                  expectedBlob.properties(),
-                  actualBlob.properties());
+              assertThat(actualBlob.type()).isEqualTo(expectedBlob.type());
+              assertThat(actualBlob.fields()).isEqualTo(expectedBlob.fields());
+              
assertThat(actualBlob.sourceSnapshotId()).isEqualTo(expectedBlob.sourceSnapshotId());
+              assertThat(actualBlob.sourceSnapshotSequenceNumber())
+                  .isEqualTo(expectedBlob.sourceSnapshotSequenceNumber());
+              
assertThat(actualBlob.properties()).isEqualTo(expectedBlob.properties());
             });
   }
 
   private static void assertEqualsRemoveStatistics(
       MetadataUpdate.RemoveStatistics expected, 
MetadataUpdate.RemoveStatistics actual) {
-    Assert.assertEquals(
-        "Snapshots to remove should be the same", expected.snapshotId(), 
actual.snapshotId());
+    assertThat(actual.snapshotId())
+        .as("Snapshots to remove should be the same")
+        .isEqualTo(expected.snapshotId());
   }
 
   private static void assertEqualsSetPartitionStatistics(
       MetadataUpdate.SetPartitionStatistics expected,
       MetadataUpdate.SetPartitionStatistics actual) {
-    Assert.assertEquals("Snapshot IDs should be equal", expected.snapshotId(), 
actual.snapshotId());
-    Assert.assertEquals(
-        "Partition Statistics files snapshot IDs should be equal",
-        expected.partitionStatisticsFile().snapshotId(),
-        actual.partitionStatisticsFile().snapshotId());
-    Assert.assertEquals(
-        "Partition statistics files paths should be equal",
-        expected.partitionStatisticsFile().path(),
-        actual.partitionStatisticsFile().path());
-    Assert.assertEquals(
-        "Partition statistics file size should be equal",
-        expected.partitionStatisticsFile().fileSizeInBytes(),
-        actual.partitionStatisticsFile().fileSizeInBytes());
+    assertThat(actual.snapshotId()).isEqualTo(expected.snapshotId());
+    assertThat(actual.partitionStatisticsFile().snapshotId())
+        .isEqualTo(expected.partitionStatisticsFile().snapshotId());
+    assertThat(actual.partitionStatisticsFile().path())
+        .isEqualTo(expected.partitionStatisticsFile().path());
+    assertThat(actual.partitionStatisticsFile().fileSizeInBytes())
+        .isEqualTo(expected.partitionStatisticsFile().fileSizeInBytes());
   }
 
   private static void assertEqualsRemovePartitionStatistics(
       MetadataUpdate.RemovePartitionStatistics expected,
       MetadataUpdate.RemovePartitionStatistics actual) {
-    Assert.assertEquals(
-        "Snapshots to remove should be the same", expected.snapshotId(), 
actual.snapshotId());
+    assertThat(actual.snapshotId()).isEqualTo(expected.snapshotId());
   }
 
   private static void assertEqualsAddSnapshot(
       MetadataUpdate.AddSnapshot expected, MetadataUpdate.AddSnapshot actual) {
-    Assert.assertEquals(
-        "Snapshot ID should be equal",
-        expected.snapshot().snapshotId(),
-        actual.snapshot().snapshotId());
-    Assert.assertEquals(
-        "Manifest list location should be equal",
-        expected.snapshot().manifestListLocation(),
-        actual.snapshot().manifestListLocation());
-    Assertions.assertThat(actual.snapshot().summary())
+    
assertThat(actual.snapshot().snapshotId()).isEqualTo(expected.snapshot().snapshotId());
+    assertThat(actual.snapshot().manifestListLocation())
+        .isEqualTo(expected.snapshot().manifestListLocation());
+    assertThat(actual.snapshot().summary())
         .as("Snapshot summary should be equivalent")
         .containsExactlyEntriesOf(expected.snapshot().summary());
-    Assert.assertEquals(
-        "Snapshot Parent ID should be equal",
-        expected.snapshot().parentId(),
-        actual.snapshot().parentId());
-    Assert.assertEquals(
-        "Snapshot timestamp should be equal",
-        expected.snapshot().timestampMillis(),
-        actual.snapshot().timestampMillis());
-    Assert.assertEquals(
-        "Snapshot schema id should be equal",
-        expected.snapshot().schemaId(),
-        actual.snapshot().schemaId());
+    
assertThat(actual.snapshot().parentId()).isEqualTo(expected.snapshot().parentId());
+    assertThat(actual.snapshot().timestampMillis())
+        .isEqualTo(expected.snapshot().timestampMillis());
+    
assertThat(actual.snapshot().schemaId()).isEqualTo(expected.snapshot().schemaId());
   }
 
   private static void assertEqualsRemoveSnapshots(
       MetadataUpdate.RemoveSnapshot expected, MetadataUpdate.RemoveSnapshot 
actual) {
-    Assert.assertEquals(
-        "Snapshots to remove should be the same", expected.snapshotId(), 
actual.snapshotId());
+    assertThat(actual.snapshotId())
+        .as("Snapshots to remove should be the same")
+        .isEqualTo(expected.snapshotId());
   }
 
   private static void assertEqualsSetSnapshotRef(
       MetadataUpdate.SetSnapshotRef expected, MetadataUpdate.SetSnapshotRef 
actual) {
     // Non-null fields
-    Assert.assertNotNull("Snapshot ref name should not be null", 
actual.name());
-    Assert.assertEquals("Snapshot ref name should be equal", expected.name(), 
actual.name());
-    Assert.assertEquals("Snapshot ID should be equal", expected.snapshotId(), 
actual.snapshotId());
-    Assert.assertNotNull("Snapshot reference type should not be null", 
actual.type());
-    Assert.assertEquals("Snapshot reference type should be equal", 
expected.type(), actual.type());
+    assertThat(actual.name()).isNotNull();

Review Comment:
   isNotNull can be combined with the next assertion



##########
core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java:
##########
@@ -1158,123 +1137,92 @@ private static void assertEqualsSetStatistics(
               BlobMetadata expectedBlob = pair.first();
               BlobMetadata actualBlob = pair.second();
 
-              Assert.assertEquals(
-                  "Expected blob type should be equal", expectedBlob.type(), 
actualBlob.type());
-              Assert.assertEquals(
-                  "Expected blob fields should be equal",
-                  expectedBlob.fields(),
-                  actualBlob.fields());
-              Assert.assertEquals(
-                  "Expected blob source snapshot ID should be equal",
-                  expectedBlob.sourceSnapshotId(),
-                  actualBlob.sourceSnapshotId());
-              Assert.assertEquals(
-                  "Expected blob source snapshot sequence number should be 
equal",
-                  expectedBlob.sourceSnapshotSequenceNumber(),
-                  actualBlob.sourceSnapshotSequenceNumber());
-              Assert.assertEquals(
-                  "Expected blob properties should be equal",
-                  expectedBlob.properties(),
-                  actualBlob.properties());
+              assertThat(actualBlob.type()).isEqualTo(expectedBlob.type());
+              assertThat(actualBlob.fields()).isEqualTo(expectedBlob.fields());
+              
assertThat(actualBlob.sourceSnapshotId()).isEqualTo(expectedBlob.sourceSnapshotId());
+              assertThat(actualBlob.sourceSnapshotSequenceNumber())
+                  .isEqualTo(expectedBlob.sourceSnapshotSequenceNumber());
+              
assertThat(actualBlob.properties()).isEqualTo(expectedBlob.properties());
             });
   }
 
   private static void assertEqualsRemoveStatistics(
       MetadataUpdate.RemoveStatistics expected, 
MetadataUpdate.RemoveStatistics actual) {
-    Assert.assertEquals(
-        "Snapshots to remove should be the same", expected.snapshotId(), 
actual.snapshotId());
+    assertThat(actual.snapshotId())
+        .as("Snapshots to remove should be the same")
+        .isEqualTo(expected.snapshotId());
   }
 
   private static void assertEqualsSetPartitionStatistics(
       MetadataUpdate.SetPartitionStatistics expected,
       MetadataUpdate.SetPartitionStatistics actual) {
-    Assert.assertEquals("Snapshot IDs should be equal", expected.snapshotId(), 
actual.snapshotId());
-    Assert.assertEquals(
-        "Partition Statistics files snapshot IDs should be equal",
-        expected.partitionStatisticsFile().snapshotId(),
-        actual.partitionStatisticsFile().snapshotId());
-    Assert.assertEquals(
-        "Partition statistics files paths should be equal",
-        expected.partitionStatisticsFile().path(),
-        actual.partitionStatisticsFile().path());
-    Assert.assertEquals(
-        "Partition statistics file size should be equal",
-        expected.partitionStatisticsFile().fileSizeInBytes(),
-        actual.partitionStatisticsFile().fileSizeInBytes());
+    assertThat(actual.snapshotId()).isEqualTo(expected.snapshotId());
+    assertThat(actual.partitionStatisticsFile().snapshotId())
+        .isEqualTo(expected.partitionStatisticsFile().snapshotId());
+    assertThat(actual.partitionStatisticsFile().path())
+        .isEqualTo(expected.partitionStatisticsFile().path());
+    assertThat(actual.partitionStatisticsFile().fileSizeInBytes())
+        .isEqualTo(expected.partitionStatisticsFile().fileSizeInBytes());
   }
 
   private static void assertEqualsRemovePartitionStatistics(
       MetadataUpdate.RemovePartitionStatistics expected,
       MetadataUpdate.RemovePartitionStatistics actual) {
-    Assert.assertEquals(
-        "Snapshots to remove should be the same", expected.snapshotId(), 
actual.snapshotId());
+    assertThat(actual.snapshotId()).isEqualTo(expected.snapshotId());
   }
 
   private static void assertEqualsAddSnapshot(
       MetadataUpdate.AddSnapshot expected, MetadataUpdate.AddSnapshot actual) {
-    Assert.assertEquals(
-        "Snapshot ID should be equal",
-        expected.snapshot().snapshotId(),
-        actual.snapshot().snapshotId());
-    Assert.assertEquals(
-        "Manifest list location should be equal",
-        expected.snapshot().manifestListLocation(),
-        actual.snapshot().manifestListLocation());
-    Assertions.assertThat(actual.snapshot().summary())
+    
assertThat(actual.snapshot().snapshotId()).isEqualTo(expected.snapshot().snapshotId());
+    assertThat(actual.snapshot().manifestListLocation())
+        .isEqualTo(expected.snapshot().manifestListLocation());
+    assertThat(actual.snapshot().summary())
         .as("Snapshot summary should be equivalent")
         .containsExactlyEntriesOf(expected.snapshot().summary());
-    Assert.assertEquals(
-        "Snapshot Parent ID should be equal",
-        expected.snapshot().parentId(),
-        actual.snapshot().parentId());
-    Assert.assertEquals(
-        "Snapshot timestamp should be equal",
-        expected.snapshot().timestampMillis(),
-        actual.snapshot().timestampMillis());
-    Assert.assertEquals(
-        "Snapshot schema id should be equal",
-        expected.snapshot().schemaId(),
-        actual.snapshot().schemaId());
+    
assertThat(actual.snapshot().parentId()).isEqualTo(expected.snapshot().parentId());
+    assertThat(actual.snapshot().timestampMillis())
+        .isEqualTo(expected.snapshot().timestampMillis());
+    
assertThat(actual.snapshot().schemaId()).isEqualTo(expected.snapshot().schemaId());
   }
 
   private static void assertEqualsRemoveSnapshots(
       MetadataUpdate.RemoveSnapshot expected, MetadataUpdate.RemoveSnapshot 
actual) {
-    Assert.assertEquals(
-        "Snapshots to remove should be the same", expected.snapshotId(), 
actual.snapshotId());
+    assertThat(actual.snapshotId())
+        .as("Snapshots to remove should be the same")
+        .isEqualTo(expected.snapshotId());
   }
 
   private static void assertEqualsSetSnapshotRef(
       MetadataUpdate.SetSnapshotRef expected, MetadataUpdate.SetSnapshotRef 
actual) {
     // Non-null fields
-    Assert.assertNotNull("Snapshot ref name should not be null", 
actual.name());
-    Assert.assertEquals("Snapshot ref name should be equal", expected.name(), 
actual.name());
-    Assert.assertEquals("Snapshot ID should be equal", expected.snapshotId(), 
actual.snapshotId());
-    Assert.assertNotNull("Snapshot reference type should not be null", 
actual.type());
-    Assert.assertEquals("Snapshot reference type should be equal", 
expected.type(), actual.type());
+    assertThat(actual.name()).isNotNull();
+    assertThat(actual.name()).isEqualTo(expected.name());
+    assertThat(actual.snapshotId()).isEqualTo(expected.snapshotId());
+    assertThat(actual.type()).isNotNull();

Review Comment:
   same as above



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