pnowojski commented on a change in pull request #6705: [FLINK-10356][network]
add sanity checks to SpillingAdaptiveSpanningRecordDeserializer
URL: https://github.com/apache/flink/pull/6705#discussion_r261614343
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/serialization/SpanningRecordSerializationTest.java
##########
@@ -105,11 +119,235 @@ public void testHandleMixedLargeRecords() throws
Exception {
testSerializationRoundTrip(originalRecords, segmentSize);
}
+ /**
+ * Non-spanning, deserialization reads one byte too many and succeeds -
failure report comes
+ * from an additional check in {@link
SpillingAdaptiveSpanningRecordDeserializer}.
+ */
+ @Test
+ public void testHandleDeserializingTooMuchNonSpanning1() throws
Exception {
+ testHandleWrongDeserialization(
+ DeserializingTooMuch.getValue(),
+ 32 * 1024);
+ }
+
+ /**
+ * Non-spanning, deserialization reads one byte too many and fails.
+ */
+ @Test
+ public void testHandleDeserializingTooMuchNonSpanning2() throws
Exception {
+ testHandleWrongDeserialization(
+ DeserializingTooMuch.getValue(),
+ (serializedLength) -> serializedLength,
+ isA(IndexOutOfBoundsException.class));
Review comment:
Did you mean `Exception` instead of `IOException`? I think that we should
either clean this up and throw only one type of an exception
(`DeserialisationException`?) or if we are throwing "unspecified" exceptions,
we should test just for that (`isA(Exception.class)`).
Again I would like to avoid the scenario when someone in the future makes a
good refactor/clean up, one of those tests starts to fail, and our future
programmer will have to spend time thinking whether throwing exactly
`IndexOutOfBoundsException` in one case and `EOFException` in other was part of
some important contract. This is what I meant by "codifying a contract". Such
kind of tests might cause someone to tip-toe/triple think "what is happening
here?".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
