zhijiangW commented on a change in pull request #6809: [FLINK-10491][network] 
Pass BufferPoolOwner in the constructor of LocalBufferPool
URL: https://github.com/apache/flink/pull/6809#discussion_r226523253
 
 

 ##########
 File path: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/ResultPartitionTest.java
 ##########
 @@ -205,6 +216,66 @@ protected void testAddOnPartition(final 
ResultPartitionType pipelined)
                }
        }
 
+       @Test
+       public void testReleaseMemoryOnBlockingPartition() throws Exception {
+               testReleaseMemory(ResultPartitionType.BLOCKING);
+       }
+
+       @Test
+       public void testReleaseMemoryOnPipelinedPartition() throws Exception {
+               testReleaseMemory(ResultPartitionType.PIPELINED);
+       }
+
+       /**
+        * Tests {@link ResultPartition#releaseMemory(int)} on a working 
partition.
+        *
+        * @param resultPartitionType the result partition type to set up
+        */
+       private void testReleaseMemory(final ResultPartitionType 
resultPartitionType) throws Exception {
+               final int numBuffers = 10;
+               final NetworkEnvironment network = new NetworkEnvironment(
+                       new NetworkBufferPool(numBuffers, 128),
+                       new LocalConnectionManager(),
+                       new ResultPartitionManager(),
+                       new TaskEventDispatcher(),
+                       new KvStateRegistry(),
+                       null,
+                       null,
+                       IOManager.IOMode.SYNC,
+                       0,
+                       0,
+                       2,
+                       8,
+                       true);
+               final ResultPartitionConsumableNotifier notifier = 
mock(ResultPartitionConsumableNotifier.class);
 
 Review comment:
   I checked these two implementations and they are exactly the same with empty 
operation, so I think we should open a separate JIRA for unifying current two 
existing implementations and make it public for outer usage.
   
   Considering the mock issue, I have some concerns:
   
   1. Do you think there are any differences between mock and empty operations 
in real class? 
   
   2. There are already many usages of mock in existing tests, do you think it 
is necessary to polish them in real classes? If so, I am willing to open JIRA 
for them. :)
   
   3. I think the unit tests are mainly for verifying partial logics or 
processes, so if the behaviors of some classes are not cared about in the 
target test, we can mock these objects to make simple. Otherwise we can develop 
the ITCase for end-to-end verify with all real classes.

----------------------------------------------------------------
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

Reply via email to