GJL edited a comment on issue #11148: [FLINK-16180][runtime] Replace the nullable vertexExecution in ScheduledUnit with a non-null executionVertexId URL: https://github.com/apache/flink/pull/11148#issuecomment-591970941 I would recommend for now to introduce only minimal changes to fix the NPE. The usage of mockito is discouraged for good reasons. However, by introducing `TestExecutionVertex` with the setter `setPreferredLocationFutures()`, we are conceptually not doing anything different than what a _Mockito Spy_ (or partial mock) does. Partial mocks have problems such as that we can easily violate the contract of the class. In this particular example, the results of `getPreferredLocationsBasedOnInputs()` and `getPreferredLocationBasedOnState()` will not be aligned with the result of our mocked method (`getPreferredLocations()`). Here are more opinions that I found about the _Subclass To Test_ pattern: * https://wiki.c2.com/?SubclassToTestAntiPattern * https://medium.com/android-testing-daily/subclass-and-override-1492f0564b35 Let me know what you think. Edit: The problem is that we are unable to manipulate the state of the `ExecutionVertex` easily such that `getPreferredLocations()` returns the desired result. We should think about how we can fix that.
---------------------------------------------------------------- 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
