-
Notifications
You must be signed in to change notification settings - Fork 13k
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-32848][tests][JUnit5 migration] Migrate flink-runtime/registration tests to JUnit5 #23300
Conversation
…tion tests to JUnit5
Hi @1996fanrui, could you review the change when you are free, much appreciated~ |
Hi @FangYongs, could you review the change when you are free? |
|
||
/** Tests for RegisteredRpcConnection, validating the successful, failure and close behavior. */ | ||
public class RegisteredRpcConnectionTest extends TestLogger { | ||
class RegisteredRpcConnectionTest { |
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.
You may need to add @ExtendWith(TestLoggerExtension.class)
for RegisteredRpcConnectionTest
after removing extends TestLogger
|
||
/** Tests for the {@link RetryingRegistrationConfiguration}. */ | ||
public class RetryingRegistrationConfigurationTest extends TestLogger { | ||
class RetryingRegistrationConfigurationTest { |
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.
Same as above
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.
Hi, we reached agreement with Fan Rui and Matthias Pohl previously that we should not add @ExtendWith(TestLoggerExtension.class) explicitly for each class but rather enable it globally in Junit5 configs. See https://issues.apache.org/jira/browse/FLINK-32866
@@ -59,28 +54,28 @@ | |||
* Tests for the generic retrying registration class, validating the failure, retry, and back-off | |||
* behavior. | |||
*/ | |||
public class RetryingRegistrationTest extends TestLogger { | |||
class RetryingRegistrationTest { |
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.
Same as above
Thanks @X-czh for contribution, just some minor comments |
Thanks @X-czh , LGTM +1 |
What is the purpose of the change
Migrate flink-runtime/registration tests to JUnit5.
Verifying this change
The change itself is for migrating UT.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation