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

    https://github.com/apache/flink/pull/4533#discussion_r146013922
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/netty/PartitionRequestClientHandlerTest.java
 ---
    @@ -208,6 +211,53 @@ public void testCancelBeforeActive() throws Exception {
                client.cancelRequestFor(inputChannel.getInputChannelId());
        }
     
    +   /**
    +    * Verifies that {@link RemoteInputChannel} is enqueued in the 
pipeline, and
    +    * {@link AddCredit} message is sent to the producer.
    +    */
    +   @Test
    +   public void testNotifyCreditAvailable() throws Exception {
    +           final CreditBasedClientHandler handler = new 
CreditBasedClientHandler();
    +           final EmbeddedChannel channel = new EmbeddedChannel(handler);
    +
    +           final RemoteInputChannel inputChannel = 
mock(RemoteInputChannel.class);
    --- End diff --
    
    Thanks for sharing your idea. 
    I think I can understand your consideration and agree with that to some 
extent.
    
    In my current new added tests, I only want to verify the behaviors for 
released channel and un-released channel in handler. So it is easy to just 
control `isReleased` property in mocked `InputChannel`, and does not concern 
the details in `InputChannel`.
    
    As you said, if someone adds more interactions with `InputChannel` in the 
future or so, the previous tests with mocked `InputChannel` may bring potential 
risks. Considering this, I am willing to modify the new added tests with real 
`InputChannel`. :)


---

Reply via email to