-
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-31876][QS] Migrate flink-queryable-state-runtime tests to JUnit5 #22457
Conversation
21aefe1
to
eb85a9b
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 for the pr! PTAL my comments.
BTW, the commit message should be in a format of [FLINK-12345][tests][JUnit5 migration] Module:XXXX
.
...ueryable-state-runtime/src/test/java/org/apache/flink/queryablestate/network/ClientTest.java
Outdated
Show resolved
Hide resolved
...me/src/test/java/org/apache/flink/queryablestate/itcases/AbstractQueryableStateTestBase.java
Show resolved
Hide resolved
...me/src/test/java/org/apache/flink/queryablestate/itcases/AbstractQueryableStateTestBase.java
Outdated
Show resolved
Hide resolved
203b934
to
3e5d7f9
Compare
@reswqa could you please help take a look? |
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 @fredia for creating this. I left some comments, PTAL.
Note: I have only commented for every similar issues in one place. But we should find and fix them all.
...e/src/test/java/org/apache/flink/queryablestate/client/proxy/KvStateClientProxyImplTest.java
Outdated
Show resolved
Hide resolved
...me/src/test/java/org/apache/flink/queryablestate/itcases/AbstractQueryableStateTestBase.java
Outdated
Show resolved
Hide resolved
...me/src/test/java/org/apache/flink/queryablestate/itcases/AbstractQueryableStateTestBase.java
Outdated
Show resolved
Hide resolved
...me/src/test/java/org/apache/flink/queryablestate/itcases/AbstractQueryableStateTestBase.java
Outdated
Show resolved
Hide resolved
...me/src/test/java/org/apache/flink/queryablestate/itcases/AbstractQueryableStateTestBase.java
Outdated
Show resolved
Hide resolved
...-runtime/src/test/java/org/apache/flink/queryablestate/network/KvStateClientHandlerTest.java
Outdated
Show resolved
Hide resolved
...time/src/test/java/org/apache/flink/queryablestate/network/KvStateRequestSerializerTest.java
Outdated
Show resolved
Hide resolved
...-runtime/src/test/java/org/apache/flink/queryablestate/network/KvStateServerHandlerTest.java
Outdated
Show resolved
Hide resolved
...-runtime/src/test/java/org/apache/flink/queryablestate/network/KvStateServerHandlerTest.java
Outdated
Show resolved
Hide resolved
...e-state-runtime/src/test/java/org/apache/flink/queryablestate/network/KvStateServerTest.java
Outdated
Show resolved
Hide resolved
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 for the update, LGTM.
Let's squash all commits and waiting for AZP passed.
8c4b9dd
to
f563e7d
Compare
@reswqa Thanks for the review and suggestions, squashed. |
@flinkbot run azure |
@reswqa CI turns green, but the result is not synchronized to flinkbot, could you help merge it? thanks. https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=48819&view=results |
What is the purpose of the change
Migrate flink-queryable-state-runtime tests to JUnit5