rdblue commented on code in PR #17041:
URL: https://github.com/apache/iceberg/pull/17041#discussion_r3539517512


##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -514,84 +308,44 @@ void testProjectedStructLike() {
     assertThat(tracking.get(1, Long.class)).isEqualTo(1000L);
   }
 
-  @Test
-  void testJavaSerializationRoundTripForDataFile() throws IOException, 
ClassNotFoundException {
-    Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
-
-    Tracking deserialized = TestHelpers.roundTripSerialize(tracking);
-
-    assertThat(deserialized.status()).isEqualTo(EntryStatus.ADDED);
-    assertThat(deserialized.snapshotId()).isEqualTo(42L);
-    assertThat(deserialized.dvSnapshotId()).isEqualTo(42L);
-    assertThat(deserialized.deletedPositions()).isNull();
-    assertThat(deserialized.replacedPositions()).isNull();
-  }
-
-  @Test
-  void testJavaSerializationRoundTripForManifest() throws IOException, 
ClassNotFoundException {
+  @ParameterizedTest
+  @MethodSource("org.apache.iceberg.TestHelpers#serializers")
+  void testSerializationRoundTrip(TestHelpers.RoundTripSerializer<Tracking> 
roundTripSerializer)
+      throws IOException, ClassNotFoundException {
     Tracking tracking =
-        TrackingBuilder.from(manifestSourceTracking(), 1L)
-            .deletedPositions(ByteBuffer.wrap(new byte[] {1, 2}))
-            .replacedPositions(ByteBuffer.wrap(new byte[] {3, 4}))
-            .build();
-
-    Tracking deserialized = TestHelpers.roundTripSerialize(tracking);
+        new TrackingStruct(
+            EntryStatus.MODIFIED,
+            42L,
+            10L,
+            11L,
+            43L,
+            1000L,
+            DELETED_POSITIONS,
+            REPLACED_POSITIONS,
+            "manifest-location",
+            7L);
+
+    Tracking deserialized = roundTripSerializer.apply(tracking);
 
     assertThat(deserialized.status()).isEqualTo(EntryStatus.MODIFIED);
-    assertThat(deserialized.dvSnapshotId()).isEqualTo(1L);
-    assertThat(deserialized.deletedPositions()).isEqualTo(ByteBuffer.wrap(new 
byte[] {1, 2}));
-    assertThat(deserialized.replacedPositions()).isEqualTo(ByteBuffer.wrap(new 
byte[] {3, 4}));
-  }
-
-  @Test
-  void testKryoSerializationRoundTripForDataFile() throws IOException {
-    Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
-
-    Tracking deserialized = 
TestHelpers.KryoHelpers.roundTripSerialize(tracking);
-
-    assertThat(deserialized.status()).isEqualTo(EntryStatus.ADDED);
     assertThat(deserialized.snapshotId()).isEqualTo(42L);
-    assertThat(deserialized.dvSnapshotId()).isEqualTo(42L);
-    assertThat(deserialized.deletedPositions()).isNull();
-    assertThat(deserialized.replacedPositions()).isNull();
-  }
-
-  @Test
-  void testKryoSerializationRoundTripForManifest() throws IOException {
-    Tracking tracking =
-        TrackingBuilder.from(manifestSourceTracking(), 1L)
-            .deletedPositions(ByteBuffer.wrap(new byte[] {1, 2}))
-            .replacedPositions(ByteBuffer.wrap(new byte[] {3, 4}))
-            .build();
-
-    Tracking deserialized = 
TestHelpers.KryoHelpers.roundTripSerialize(tracking);
-
-    assertThat(deserialized.status()).isEqualTo(EntryStatus.MODIFIED);
-    assertThat(deserialized.dvSnapshotId()).isEqualTo(1L);
-    assertThat(deserialized.deletedPositions()).isEqualTo(ByteBuffer.wrap(new 
byte[] {1, 2}));
-    assertThat(deserialized.replacedPositions()).isEqualTo(ByteBuffer.wrap(new 
byte[] {3, 4}));
-  }
-
-  private static TrackingStruct sourceTracking() {
-    TrackingStruct tracking = new TrackingStruct(Tracking.schema());
-    tracking.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
-    tracking.set(SNAPSHOT_ID_ORDINAL, 42L);
-    tracking.set(DATA_SEQUENCE_NUMBER_ORDINAL, 10L);
-    tracking.set(FILE_SEQUENCE_NUMBER_ORDINAL, 10L);
-    tracking.set(DV_SNAPSHOT_ID_ORDINAL, 43L);
-    tracking.set(FIRST_ROW_ID_ORDINAL, 1000L);
-    return tracking;
-  }
-
-  private static TrackingStruct sourceTrackingWithStatus(EntryStatus status) {
-    TrackingStruct tracking = sourceTracking();
-    tracking.set(STATUS_ORDINAL, status.id());
-    return tracking;
-  }
-
-  private static TrackingStruct manifestSourceTracking() {
-    TrackingStruct tracking = sourceTracking();
-    tracking.set(DV_SNAPSHOT_ID_ORDINAL, null);
-    return tracking;
+    assertThat(deserialized.dataSequenceNumber()).isEqualTo(10L);
+    assertThat(deserialized.fileSequenceNumber()).isEqualTo(11L);
+    assertThat(deserialized.dvSnapshotId()).isEqualTo(43L);
+    assertThat(deserialized.firstRowId()).isEqualTo(1000L);
+    
assertThat(deserialized.deletedPositions()).isEqualTo(ByteBuffer.wrap(DELETED_POSITIONS));
+    
assertThat(deserialized.replacedPositions()).isEqualTo(ByteBuffer.wrap(REPLACED_POSITIONS));
+    assertThat(deserialized.manifestLocation()).isEqualTo("manifest-location");
+    assertThat(deserialized.manifestPos()).isEqualTo(7L);
+  }
+
+  private static int ordinalByFieldName(String fieldName) {
+    List<Types.NestedField> fields = TrackingStruct.BASE_TYPE.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      if (fields.get(i).name().equals(fieldName)) {
+        return i;
+      }
+    }
+    throw new IllegalArgumentException("Field not found in schema: " + 
fieldName);

Review Comment:
   Style: there should be an empty line between a control flow block and the 
following statement.



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

Reply via email to