zhijiangW commented on a change in pull request #7713: [FLINK-10995][network] Copy intermediate serialization results only once for broadcast mode URL: https://github.com/apache/flink/pull/7713#discussion_r265996143
########## File path: flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/BroadcastRecordWriterTest.java ########## @@ -21,30 +21,30 @@ import org.apache.flink.core.io.IOReadableWritable; Review comment: Not really, the new added test in `BroadcastRecordWriterTest` for 2nd commit actually tests the function for `BroadcastRecordWriter`. In this commit, the `BroadcastRecordWriterTest` also extends the `RecordWriterTest`, so it would run all the existing tests in `RecordWriterTest`, but the created writer is still the `SelectorRecordWriter`. In short words, `BroadcastRecordWriterTest` extends all the existing tests in `RecordWriterTest` and run them in the way of `SelectorRecordWriter`. So in the 3rd commit, I introduced the `isBroadcastWriter` in the constructor for creating the specific `RecordWriter` instance based on this tag. Then all the existing tests would run for `BroadcastRecordWriterTest` in the way of `BroadcastRecordWriter`. Anyway, I think it still makes sense to squash these two commits. The separation is just for distinguishing the new added test and refactoring all the old tests. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
