zeroshade commented on code in PR #1435:
URL: https://github.com/apache/iceberg-go/pull/1435#discussion_r3566743825
##########
table/metadata_builder_internal_test.go:
##########
@@ -2562,3 +2564,114 @@ func
TestSetFormatVersionV2ToV3FromDeserializedMetadata(t *testing.T) {
require.Equal(t, int64(34), meta3.LastSequenceNumber())
require.Equal(t, int64(0), meta3.NextRowID())
}
+
+// sharedCloneFields names the MetadataBuilder fields that clone()
intentionally
+// shares with the original instead of copying. Keep this in sync with clone();
+// both the drift guard and its filler consult it.
+var sharedCloneFields = map[string]struct{}{
+ "base": {}, // immutable snapshot, shared by design.
+}
+
+// TestMetadataBuilderCloneCoversAllFields guards clone() against field drift:
+// it fills every field of MetadataBuilder with non-zero data and asserts the
+// clone is value-equal yet backed by independent storage. A field added to the
+// struct but omitted from clone() fails the value-equal check; a field copied
+// by reference instead of cloned fails the aliasing check. The filler fails
the
+// test on any kind it cannot populate, so a new field of an unhandled type
+// cannot slip through as a vacuous pass.
+//
+// The aliasing check is one level deep: it verifies each slice/map/pointer
+// field has its own backing storage, not that reference types nested inside
+// elements are independent (clone() shares those by design). A future field
+// nesting references inside an element would need its own dedicated coverage.
+func TestMetadataBuilderCloneCoversAllFields(t *testing.T) {
+ orig := &MetadataBuilder{}
+ fillReferenceFields(t, reflect.ValueOf(orig).Elem())
Review Comment:
Non-blocking: the reference filler leaves pointer targets, slice elements,
and map entries at their zero values, so a broken `clone()` that allocates
fresh storage but fails to copy the pointee/element *values* could still pass
the aliasing check. Populating nested pointers / slice elements / map
keys+values with non-zero data (e.g. a small recursive filler for the supported
kinds) would close that gap.
--
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]
