-
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-32850][flink-runtime][JUnit5 Migration] The io.disk package of flink-runtime module #23572
Conversation
Hi @XComp, @1996fanrui, @RocMarshal. |
4d78633
to
7ff770f
Compare
Hi @RocMarshal, please help review it when you have time. |
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, @Jiabao-Sun Thanks for your hard work.
LGTM on the whole.
Only left a comment.
PTAL in your free time.
|
||
import java.io.EOFException; | ||
import java.util.List; | ||
|
||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
import static org.assertj.core.api.AssertionsForClassTypes.assertThat; |
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.
Shoud the line be repalced by import static org.assertj.core.api.Assertions.assertThat
?
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 hard review.
Fixed.
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.
@Jiabao-Sun Thanks for the update.
LGTM +1 (non-binding)
@1996fanrui, please help review it when you have time. |
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 your effort, and @RocMarshal for your review!
What is the purpose of the change
[FLINK-32850][flink-runtime][JUnit5 Migration] The io.disk package of flink-runtime module
Brief change log
[FLINK-32850][flink-runtime][JUnit5 Migration] The io.disk package of flink-runtime module
Verifying this change
This change is already covered by existing tests
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation