-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19617. [JDK17] Remove JUnit4 Dependency. #7957
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
@cnauroth @ayushtkn @steveloughran @Hexiaoqiao @anujmodi2021 @ahmarsuhail @szetszwo @jojochuang This is our final PR related to the JUnit 5 upgrade. Once this is merged, the upgrade on the trunk branch will be complete. Could you help review this PR ? Thank you very much! cc: @zhtttylz |
💔 -1 overall
This message was automatically generated. |
4878f2b
to
9665314
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
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 !
The build result of this PR is as expected, and we are now able to remove the dependency on JUnit 4. There is only one remaining test case with an issue, which we plan to address in a follow-up PR. Many thanks for everyone's support! With this change, the Hadoop trunk branch has completely removed the dependency on JUnit 4 and has fully migrated to JUnit 5. |
Many thanks to @anujmodi2021 and @cnauroth for reviewing this PR! |
Congratulations for getting this huge overhaul through! |
@slfan1989 and @zhtttylz , thank you so much for all of your efforts on the JUnit5 patches! |
That's a great contribution. Thanks everyone! |
BTW, filed HADOOP-19692 for excluding junit 4 transitive dependency. |
@cnauroth @h-vetinari @szetszwo @zhtttylz @anujmodi2021 Sorry for the late reply, and thank you again for your help! |
Description of PR
HADOOP-19617 - [JDK17] Remove JUnit4 Dependency.
This is our final PR for the
JUnit 5
upgrade. In this PR, we are completely removing all relatedJUnit
4 dependencies. As a result,org.hamcrest
, which was previously brought in viaJUnit 4
, will no longer be available. This leads to a few minor code changes across three modules. Given the small scope of these changes, I’ve decided to include them in this PR instead of creating multiple separate ones.The details are as follows:
The original code made use of
org.hamcrest
. I have replaced it with AssertJ assertions to align withJUnit 5
standards.This class is currently unused in the project, so I’ve removed it.
The original use of JUnit 4’s
@RunWith(Parameterized.class)
has been migrated to JUnit 5’s@ParameterizedClass
to support parameterized testing.Similarly, since this class is not being referenced anywhere, I’ve removed it as well.
How was this patch tested?
CI Test.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?