stevenzwu commented on PR #18853: URL: https://github.com/apache/iceberg/pull/18853#issuecomment-6790412663
Thanks, I rewrote the tests following this plan and updated the test plan in the description. The tests call get on the wrappers with mocked TrackedFile and Tracking, and cover the substitutions and wrap validations in your list. A few differences: - I didn't add get tests for every field. The wrappers will be embedded in the leaf and root manifest writers, whose round-trip tests will write and read back every field for data files, data manifests, and delete manifests, so testing them here too would duplicate those. Happy to add them here if you'd prefer. - Checks shared by both wrappers (partition projection, content stats, and sequence number validation) are tested once through the leaf. The root tests cover only what is specific to the root. - I dropped the mdvBuffer test since it only passes the buffer through. - I added a few cases that weren't in the list: projecting partitions to the union type and rejecting a union type missing a spec field, unknown spec IDs, null for missing stats, a file modified in the commit (the leaf keeps a null data sequence number and the root writes the commit's), the root requiring a snapshot ID, and the status checks for null sequence numbers. - - -- 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]
