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

Reply via email to