[GitHub] [flink] aljoscha commented on issue #10286: [FLINK-14890] [tests] Add missing test harnesses for broadcast functions

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services