rdblue commented on code in PR #12374: URL: https://github.com/apache/iceberg/pull/12374#discussion_r1966203879
########## api/src/test/java/org/apache/iceberg/variants/TestSerializedMetadata.java: ########## @@ -44,7 +44,7 @@ public void testEmptyVariantMetadata() { @Test public void testHeaderSorted() { - SerializedMetadata metadata = SerializedMetadata.from(new byte[] {0b10001, 0x00}); + SerializedMetadata metadata = SerializedMetadata.from(new byte[] {0b10001, 0x00, 0x00}); Review Comment: We have those helpers in `VariantTestUtil`, but most of the tests here use hard-coded byte arrays for a couple reasons. First, I don't want to rely on equally complicated code. The most basic cases (like whether the sorted flag is set) should use values created directly from the spec. Second, it's hard to exercise the cases with generated values. For instance, `testHeaderOffsetSize` checks the offset size without needing to generate a metadata dictionary that has more than 65k unique values. There's a test that does this later (`testThreeByteFieldIds`) but that's testing the offsets and it is good to know from an independent test that the offset size is being interpreted correctly. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org