rkhachatryan commented on a change in pull request #12714:
URL: https://github.com/apache/flink/pull/12714#discussion_r443686425
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskTest.java
##########
@@ -139,6 +142,33 @@ public void testRegularExecution() throws Exception {
taskManagerActions.validateListenerMessage(ExecutionState.FINISHED, task, null);
}
+ @Test
+ public void testTaskTerminatesOnlyAfterInvokable() throws Exception {
Review comment:
> It's definitely not testing for the reported bug.
A proper test for the reported bug would be an end-to-end test involving
running a remote cluster, syncing source thread with class unloading and then
checking the logs.
I think we agreed offline that such automated test would be an overkill and
could rather be done manually. And the rest should be covered with unit tests.
Now, this unit test checks that `SourceStreamTask` returns completion future
of its thread.
This is what changed - and this is what I think **should** be tested.
If we also want to test `LegacySourceFunctionThread` future completion, it
can be done independently now after extraction (however, I think this is out of
the scope of this PR).
Furthermore, it's not possible to reliably test that a program does not
perform some action if there is no time boundary (in this case test that task
never completes if source thread never completes).
----------------------------------------------------------------
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]
