-
Notifications
You must be signed in to change notification settings - Fork 13.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-11594][tests] Remove TaskManagerRegistrationTest #7725
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. Bot commandsThe @flinkbot bot supports the following commands:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}, | ||
rpcService.getExecutor()); | ||
|
||
// upon failure, retry, unless this is cancelled | ||
resourceManagerAcceptFuture.whenCompleteAsync( | ||
rpcGatewayAcceptFuture.whenCompleteAsync( | ||
(Void v, Throwable failure) -> { | ||
if (failure != null && !canceled) { | ||
final Throwable strippedFailure = ExceptionUtils.stripCompletionException(failure); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good refactorings!
…sManager in TaskExecutorTest
…rsIfLeaderIsKnown
Thanks for the review. I added a commit to remove |
What is the purpose of the change
This removes
TaskManagerRegistrationTest
. Tests were mostly covered already.TaskManagerRegistrationTest
testSimpleRegistration
TaskExecutorTest#testImmediatelyRegistersIfLeaderIsKnown
,TaskExecutorTest#testOfferSlotToJobMasterAfterTimeout
,TaskExecutorTest#testHeartbeatTimeoutWithJobManager
,ResourceManagerTaskExecutorTest#testRegisterTaskExecutor
,TaskExecutorToResourceManagerConnectionTest
testDelayedRegistration
RetryingRegistrationTest#testRetryConnectOnFailure
testShutdownAfterRegistrationDurationExpired
TaskExecutor#testMaximumRegistrationDuration
,TaskManagerRunnerTest#testShouldShutdownIfRegistrationWithJobManagerFails
,TaskManagerRunnerTest#testShouldShutdownOnFatalError
testTaskManagerResumesConnectAfterRefusedRegistration
RetryingRegistrationTest#testDecline
testTaskManagerNoExcessiveRegistrationMessages
RetryingRegistrationTest
testTaskManagerResumesConnectAfterJobManagerFailure
TaskExecutorTest#testHeartbeatTimeoutWithJobManager
,TaskExecutorTest#testHeartbeatTimeoutWithResourceManager
,ZooKeeperLeaderElectionITCase#testJobExecutionOnClusterWithLeaderChange
testCheckForValidRegistrationSessionIDs
TaskExecutorTest#testJobLeaderDetection
)Brief change log
Verifying this change
This change is already covered by existing tests, such as
TaskExecutorTest
,RetryingRegistrationTest
.This change added tests and can be verified as follows:
TaskExecutorToResourceManagerConnection
to test that the correct arguments are passed in the TM -> RM registration.Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation