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

    https://github.com/apache/flink/pull/5581#discussion_r171181261
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/SubpartitionTestBase.java
 ---
    @@ -182,7 +182,7 @@ private static void assertNextBufferOrEvent(
                        ResultSubpartitionView readView,
                        int expectedReadableBufferSize,
                        boolean expectedIsBuffer,
    -                   Class<? extends AbstractEvent> expectedEventClass,
    +                   @Nullable Class<? extends AbstractEvent> 
expectedEventClass,
    --- End diff --
    
    I would prefer for this method to return `Buffer` and:
    ```
    void assertNextBuffer(...) {
      Buffer buffer = assertNextBufferOrEvent(...);
      assertTrue(buffer.isBuffer());
      buffer.recycleBuffer();
    }
    
    void assertNextEvent(...) {
      Buffer buffer = assertNextBufferOrEvent(...);
      assertFalse(buffer.isBuffer());
      assertThat(EventSerializer.fromBuffer(buffer, ...), ...);
      buffer.recycleBuffer();
    }
    ```
    
    btw, you are not recycling the buffer in case of failure


---

Reply via email to