AHeise commented on code in PR #29370:
URL: https://github.com/apache/flink/pull/29370#discussion_r4181875237


##########
flink-core/src/test/java/org/apache/flink/types/variant/BinaryVariantInternalBuilderTest.java:
##########
@@ -201,4 +208,58 @@ void testAppendFloat() {
 
         assertThatCode(() -> 
floatList.forEach(builder::appendFloat)).doesNotThrowAnyException();
     }
+
+    @ParameterizedTest
+    @ValueSource(
+            strings = {
+                "",
+                "Grüße, 世界 🚀",
+                "A string longer than 63 bytes is stored with a 4-byte length 
header."

Review Comment:
   nit: the cases are 0, 20 and 68 bytes, so a `>=` instead of `>` in 
`appendString` would still pass. Could you add 63- and 64-byte strings, and 
call `appendString(byte[], int, int)` with a nonzero offset? That overload is 
only covered indirectly through `VariantCastUtilsTest`.



-- 
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]

Reply via email to