stevenzwu commented on code in PR #16285:
URL: https://github.com/apache/iceberg/pull/16285#discussion_r3741920853
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -175,10 +204,22 @@ void doNotInheritSequenceNumberForModifiedEntries() {
assertThat(tracking.fileSequenceNumber()).isEqualTo(6L);
}
+ @Test
+ void inheritDataSequenceNumberAfterColumnFilesChange() {
+ // Adding column files should set data sequence number to null
Review Comment:
I thought the idea is to avoid creating Builder only for testing purpose. If
builder is legitimately needed, I don't see why I can't use them in the test
code.
But since `columnFilesUpdated` API is already tested in
`TestTrackingBuilder`, this is fine. I think the comment seems to suggest that
we should construct the tracking with `columnFilesUpdated`. Maybe we should
remove the comment. Also maybe rename the test method name from
`inheritDataSequenceNumberAfterColumnFilesChange` to
`inheritDataSequenceNumberWithModifiedStatus`, since this is strictly testing
inheritance for modified status.
--
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]
