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-32843][JUnit5 Migration] Migrate the jobmaster package of flink-runtime module to JUnit5 #24723
Conversation
…ngComponentMainThreadExecutor.Resource
…ension.java to replace the PhysicalSlotProviderResource.java
Hi, @Jiabao-Sun @caicancai Could you help have a check on it if you had the free time ? |
9da947b
to
f795064
Compare
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.
Thanks @RocMarshal for this contribution which is overall good to me.
We can continue to minimize the visibility of methods and fields.
The methods visibility can be package-private.
@BeforeEach
public void setup()
@BeforeClass
public static void setup()
The fields visibility can be private.
@RegisterExtension
public final TestingFatalErrorHandlerExtension testingFatalErrorHandlerExtension =
new TestingFatalErrorHandlerExtension();
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterQueryableStateTest.java
Show resolved
Hide resolved
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterQueryableStateTest.java
Show resolved
Hide resolved
…k-runtime module to JUnit5
…viderResource.java
…Executor.Resource
f795064
to
bf8d5cf
Compare
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.
Thanks @Jiabao-Sun for the review.
I made some change based on your comments.
Would you mind taking a look ? many thanks~ :)
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.
LGTM, I have no other opinions here
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.
Thanks @RocMarshal for the quick update and @caicancai for the review.
LGTM.
What is the purpose of the change
[FLINK-32843][JUnit5 Migration] Migrate the jobmaster package of flink-runtime module to JUnit5
Brief change log
Remove the TestingComponentMainThreadExecutor.Resource
Remove the deprecated PhysicalSlotProviderResource.java
Migrate the jobmaster package of flink-runtime module to JUnit5
Introduce the PhysicalSlotProviderExtension.java to replace the PhysicalSlotProviderResource.java
Add the comments about deprecated details for TestingComponentMainThreadExecutor.Resource
Verifying this change
Please make sure both new and modified tests in this PR follow the conventions for tests defined in our code quality guide.
This change is already covered by existing tests, such as (please describe tests).
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation