aljoscha commented on issue #10286: [FLINK-14890] [tests] Add missing test 
harnesses for broadcast functions
URL: https://github.com/apache/flink/pull/10286#issuecomment-558149223
 
 
   Thanks for your contribution!
   
   I think this mixes up to (potentially) orthogonal issues:
    1. The addition of convenience methods for creating harnesses for 
`(Keyed)BroadcastProcessFunction`. These don't exist for regular 
`(Keyed)ProcessFunctions` as of now, where we just have operators and the 
`(Keyed)TwoInputStreamOperatorTestHarness`.
    2. The addition of an operator test harness for operators with broadcast 
input. Now that I look at the code I think it's even possible to re-use the 
`(Keyed)TwoInputStreamOperatorTestHarness` for this, it is just that the method 
will be called `processElement2()` and not `processBroadcastElement()`.
   
   I would not be opposed to fixing both issues, but they should be considered 
separately (at least separate commits). If we want to fix 1. we should also add 
convenience methods for the other user functions but that seems to be a bigger 
undertaking.
   
   Also, we should have tests for newly added code.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to