gaborkaszab commented on code in PR #16408:
URL: https://github.com/apache/iceberg/pull/16408#discussion_r3347633862
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -166,28 +192,271 @@ void testInheritFromRejectsUnequalSequenceNumbers() {
@Test
void testNoDefaultingWithoutInheritance() {
- TrackingStruct tracking =
TrackingStruct.builder().status(EntryStatus.ADDED).build();
+ TrackingStruct tracking = new TrackingStruct(Tracking.schema());
+ tracking.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
// no inheritance, nulls stay null
assertThat(tracking.snapshotId()).isNull();
assertThat(tracking.dataSequenceNumber()).isNull();
assertThat(tracking.fileSequenceNumber()).isNull();
}
+ @Test
+ void testInheritFromNullIsNoOp() {
+ TrackingStruct tracking = new TrackingStruct(Tracking.schema());
+ tracking.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
+
+ tracking.inheritFrom(null);
+
+ // null source is a no-op; all unset fields stay null
+ assertThat(tracking.snapshotId()).isNull();
+ assertThat(tracking.dataSequenceNumber()).isNull();
+ assertThat(tracking.fileSequenceNumber()).isNull();
+ }
+
private static Tracking createManifestTracking(long snapshotId, long
sequenceNumber) {
- return TrackingStruct.builder()
- .status(EntryStatus.ADDED)
- .snapshotId(snapshotId)
- .dataSequenceNumber(sequenceNumber)
- .fileSequenceNumber(sequenceNumber)
- .build();
+ TrackingStruct tracking = new TrackingStruct(Tracking.schema());
Review Comment:
As the design looks now for the builder, we create ADDED status where the
sequence numbers are null. Maybe this is different for Tracking of manifests,
but the builder currently is not capable to produce such a Tracking object.
Shouldn't we test with Tracking objects that the builder can produce? Minor
coverage is fine to see what happens when we read from storage such a state
that is against the builder, but I see this setup for a number of tests.
Do I miss something?
--
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]
