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

    https://github.com/apache/flink/pull/4649#discussion_r161799248
  
    --- Diff: 
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/DataStreamTest.java
 ---
    @@ -183,6 +183,14 @@ public Long map(Long value) throws Exception {
     
                // verify self union
                
assertTrue(streamGraph.getStreamNode(selfUnion.getId()).getInEdges().size() == 
2);
    +           assertTrue(streamGraph.getUniqueEdgeMap().size() == 12);
    --- End diff --
    
    Can not it be tested otherwise? This test shouldn't have an access to 
`getUniqueEdgeMap()` since that should be a part of the private implementation. 
If one chooses to implement this feature differently, this test would brake. 
    
    Can not this test just check that 
`org.apache.flink.streaming.api.graph.StreamGraph#getStreamEdges(selfUnion.getId(),
 selfUnion.getId())` contains two unique edges?
    
    Side note, shouldn't the `getStreamEdges()` return a `Set<StreamEdge>` 
instead of `List`?


---

Reply via email to