-
Notifications
You must be signed in to change notification settings - Fork 9k
HADOOP-19417. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-minikdc. #7636
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
Conversation
💔 -1 overall
This message was automatically generated. |
@@ -89,7 +90,8 @@ private Properties getMultiAuthHandlerConfiguration() { | |||
return props; | |||
} | |||
|
|||
@Test(timeout=60000) |
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.
It looks like this file (and a few additional ones) is using junit4 library for the most part.
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.
Thank you for your help with the review. You are right, many parts of this module are still using JUnit 4. We are actively upgrading from JUnit 4 to JUnit 5. The goal of this PR is to complete the upgrade of the hadoop-minikdc module and its unit tests, as well as to upgrade other modules that depend on hadoop-minikdc.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@Hexiaoqiao @jojochuang Could you please help review this PR? Thank you very much! |
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. +1. Thanks @slfan1989 .
Some nit checkstyle issues but I think it is OK to improve it later. Such as miss blank space before and after plus here. Some line should be more readable to keep the original format, such as 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.
+1. Thank you @slfan1989 and @Hexiaoqiao .
@Hexiaoqiao @cnauroth @jojochuang Thank you very much for reviewing this PR! I will improve this PR based on your suggestions. |
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.
+1,LGTM @slfan1989
🎊 +1 overall
This message was automatically generated. |
@Hexiaoqiao @jojochuang @cnauroth @zhtttylz I have fixed the related checkstyle issues according to the suggestions. Thank you very much for reviewing the code! |
…pache#7636) * HADOOP-19417. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-minikdc. Co-authored-by: Wei-Chiu Chuang <weichiu@apache.org> Co-authored-by: He Xiaoqiao <hexiaoqiao@apache.org> Co-authored-by: Chris Nauroth <cnauroth@apache.org> Co-authored-by: Hualong Zhang <hualong.z@hotmail.com> Reviewed-by: Wei-Chiu Chuang <weichiu@apache.org> Reviewed-by: He Xiaoqiao <hexiaoqiao@apache.org> Reviewed-by: Chris Nauroth <cnauroth@apache.org> Reviewed-by: Hualong Zhang <hualong.z@hotmail.com> Signed-off-by: Shilun Fan <slfan1989@apache.org>
…pache#7636) * HADOOP-19417. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-minikdc. Co-authored-by: Wei-Chiu Chuang <weichiu@apache.org> Co-authored-by: He Xiaoqiao <hexiaoqiao@apache.org> Co-authored-by: Chris Nauroth <cnauroth@apache.org> Co-authored-by: Hualong Zhang <hualong.z@hotmail.com> Reviewed-by: Wei-Chiu Chuang <weichiu@apache.org> Reviewed-by: He Xiaoqiao <hexiaoqiao@apache.org> Reviewed-by: Chris Nauroth <cnauroth@apache.org> Reviewed-by: Hualong Zhang <hualong.z@hotmail.com> Signed-off-by: Shilun Fan <slfan1989@apache.org>
Description of PR
JIRA: HADOOP-19417. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-minikdc.
How was this patch tested?
mvn clean test & junit test.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?