Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5588#discussion_r171004092
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/serialization/EventSerializerTest.java
 ---
    @@ -117,49 +116,55 @@ public void testIsEventPeakOnly() throws Exception {
        }
     
        /**
    -    * Tests {@link EventSerializer#isEvent(Buffer, Class, ClassLoader)} 
returns
    +    * Tests {@link EventSerializer#isEvent(Buffer, Class)} returns
         * the correct answer for various encoded event buffers.
         */
        @Test
        public void testIsEvent() throws Exception {
                AbstractEvent[] events = {
                        EndOfPartitionEvent.INSTANCE,
    -                   EndOfSuperstepEvent.INSTANCE,
                        new CheckpointBarrier(1678L, 4623784L, 
CheckpointOptions.forCheckpointWithDefaultLocation()),
                        new TestTaskEvent(Math.random(), 12361231273L),
    -                   new CancelCheckpointMarker(287087987329842L)
    +                   new CancelCheckpointMarker(287087987329842L),
    +                   EndOfSuperstepEvent.INSTANCE
    +           };
    +
    +           Class[] expectedClasses = {
    +                   EndOfPartitionEvent.class,
    +                   CheckpointBarrier.class,
    +                   CancelCheckpointMarker.class,
    +                   EndOfSuperstepEvent.class
    --- End diff --
    
    This extra array seems a bit error-prone and requires maintenance in case 
the events are extended - wouldn't it be equally clear if we used your new 
naming with the original array?


---

Reply via email to