flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] pnowojski commented on a change in pull request #6705: [FLINK-10356][network] add sanity checks to SpillingAdaptiveSpanningRecordDeserializer
Date Fri, 01 Mar 2019 14:20:20 GMT
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_r261612661
 
 

 ##########
 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 {
 
 Review comment:
   But this test doesn't test for anything. Is our contract: if "deserialization reads one
byte too many" the deserialisation must "succeed"? If this behaviour changes, and after refactoring
we somehow throw an exception here would it be a bad thing? It looks to me like a no-op test
that if it fails, it can be safely removed or changed to actually expecting the exception.
   
   If at all, this should be tested in the `SpillingAdaptiveSpanningRecordDeserializer`'s
test, where this "additional check" is kicking in.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message