amogh-jahagirdar commented on code in PR #16285:
URL: https://github.com/apache/iceberg/pull/16285#discussion_r4328269255
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -421,7 +438,8 @@ void
serializationRoundTrip(RoundTripSerializer<TrackedFileStruct> serializer) t
null, // DeletionVector has its own serialization tests
null, // ManifestInfo has its own serialization tests
ByteBuffer.wrap(new byte[] {1, 2, 3}),
- ImmutableList.of(50L));
+ ImmutableList.of(50L),
+ null); // ColumnFile has its own serialization tests
Review Comment:
I think we should pass actual column files here instead of null.
`ColumnFileStruct` has its own serialization tests, but the list field is owned
by `TrackedFileStruct`, so right now nothing checks that `column_files` behaves
as expected after serialization.
Could we add a couple of real `ColumnFileStruct`s and compare them with the
`ColumnFile.schema()` comparator? Same idea as these two comments on the
original `TrackedFileStruct` PR: [real values instead of
nulls](https://github.com/apache/iceberg/pull/15854#discussion_r3033610033) and
[serialization tests having the same validations as
copy](https://github.com/apache/iceberg/pull/15854#discussion_r3113615068).
--
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]
