HDFS-12431. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-hdfs Part3. #7626
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
6db7f01 to
b78487a
Compare
|
💔 -1 overall
This message was automatically generated. |
b78487a to
cd1f9d3
Compare
cd1f9d3 to
0cf3676
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
slfan1989
reviewed
Apr 18, 2025
|
|
||
| import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_RPC_PROTECTION; | ||
| import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_ENCRYPT_DATA_OVERWRITE_DOWNSTREAM_DERIVED_QOP_KEY; | ||
| import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_SERVICE_RPC_ADDRESS_KEY; | ||
| import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_SEND_QOP_ENABLED; | ||
| import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_RPC_ADDRESS_AUXILIARY_KEY; | ||
| import static org.apache.hadoop.hdfs.client.HdfsClientConfigKeys.DFS_ENCRYPT_DATA_OVERWRITE_DOWNSTREAM_NEW_QOP_KEY; | ||
| import static org.junit.Assert.*; | ||
| import static org.junit.jupiter.api.Assertions.*; |
There was a problem hiding this comment.
Thanks for the feedback! I'll update the code as soon as possible.
|
🎊 +1 overall
This message was automatically generated. |
slfan1989
approved these changes
Apr 19, 2025
|
@zhtttylz Thanks for the contribution! @jojochuang @szetszwo @ayushtkn @cnauroth Could you help review this PR? Thank you very much! Since this change involves HDFS unit tests, I'd like to hear your thoughts. |
szetszwo
approved these changes
Apr 22, 2025
There was a problem hiding this comment.
+1 the change looks good.
Just a question: Could we remove the junit 4 dependency?
Sugguestions for the future PRs:
- The PR is very large -- the patch files is > 200KB. It took me a few days to review it. I would suggest splitting it into 4 or more PRs, 50KB each. It will improve the turn around time.
- For such a large PR, please don't change indentations. It becomes a never ending reviewing.
I do appreciate your efforts!
| @@ -241,6 +241,26 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> | |||
| <groupId>org.apache.hadoop</groupId> | |||
| <artifactId>hadoop-annotations</artifactId> | |||
| </dependency> | |||
| <dependency> | |||
There was a problem hiding this comment.
Has this pr upraded all the junit 4 cases? If yes, we should remove the junit 4 dependency.
|
@zhtttylz , it seems that you are not the current assignee HDFS-12431. If it is the case, could you put a comment on it? Then, we could assign this to you and the resolve the JIRA. Cc @slfan1989 |
@szetszwo Thank you very much for reviewing the code! I have assigned HDFS-12431 to Hualong Zhang. |
@szetszwo Thank you very much for reviewing the code and providing valuable feedback! We greatly appreciate your efforts. Currently, the HDFS project contains over 790 unit tests, and we aim to find a reasonable balance between the size of the PRs and what is acceptable for reviewers. To control the number of PRs, we are currently bundling them in batches of 50 classes, which means we will need to submit at least 15 PRs. If we don’t do this, it could result in more PRs related to the HDFS upgrade, and for users who require backporting, the large number of PRs could be less friendly. This process is indeed challenging, and we don't want the JUnit 4 upgrade to become too fragmented. Your help is especially important for the HDFS module. We may need to wait until the entire HDFS module is upgraded before we can remove the JUnit 4 dependency. cc: @zhtttylz |
|
Thanks @szetszwo @slfan1989 help review and merged! |
74 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
JIRA:HDFS-12431. Upgrade JUnit from 4 to 5 in hadoop-hdfs Part3.
How was this patch tested?
Junit Test.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?