GJL commented on issue #7544: [FLINK-11366][tests] Port TaskManagerMetricsTest to new code base URL: https://github.com/apache/flink/pull/7544#issuecomment-457181080 I think it is better to delete the original test: - `TaskManagerMetricsTest` was asserting that the MetricRegistry is not closed when a TaskManager reconnected to the JobManager. However, in the new code, the Taskmanager (TaskExecutor) does not have a direct reference to the MetricRegistry anymore. The responsibility of closing the MetricRegistry is now in the TaskManagerRunner. - Resource cleanup happens in `TaskExecutor#postStop()`, not on disconnect/reconnect. - The TaskManager now connects to more than one instance, i.e., ResourceManager and JobManager. However, your ported test, only tests the behavior for ResourceManager reconnects. - Lastly, you are piggybacking on `testHeartbeatTimeoutWithResourceManager` which is not good practice because every test should only test one aspect/concept. I will revert your changes in `testHeartbeatTimeoutWithResourceManager`. Let me know if you think otherwise.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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
