tomtongue commented on code in PR #16859:
URL: https://github.com/apache/iceberg/pull/16859#discussion_r4004263617
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java:
##########
@@ -644,19 +644,22 @@ public void testMetadataLogEntries() throws Exception {
metadataLogEntries.get(0).file(),
null,
null,
- null),
+ null,
+ tableMetadata.properties()),
Review Comment:
Thanks for calling this out. Based on your comment, I considered using
concrete expected maps and adding a comment as below:
* Concrete maps would not add meaningful history coverage because all
entries in this test intentionally have identical properties, and they would
couple the test to unrelated default-property changes.
* A comment would repeat what the immediately following
`testMetadataLogEntriesPropertyHistory` already makes explicit.
For `testMetadataLogEntries`, I would prefer to keep the existing assertions
as-is. This test checks that Spark returns the `properties` column correctly
for both `SELECT *` and filtered queries. The separate
`testMetadataLogEntriesPropertyHistory` checks that each metadata version
returns the properties stored in that version, so the two tests cover different
behavior.
I believe this separation provides the intended coverage. If you still see a
gap, please let me know and I’ll update the tests.
--
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]
