- Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-12431. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-hdfs Part6. #7739
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. |
| 🎊 +1 overall
This message was automatically generated. |
21ab637 to 6898a70 Compare | 🎊 +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. |
| import static org.apache.hadoop.hdfs.server.common.HdfsServerConstants.NodeType.DATA_NODE; | ||
| import static org.apache.hadoop.hdfs.server.common.HdfsServerConstants.NodeType.NAME_NODE; | ||
| import static org.junit.Assert.*; | ||
| import static org.junit.jupiter.api.Assertions.*; |
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.
Expand *
| import static org.hamcrest.CoreMatchers.not; | ||
| import static org.junit.Assert.*; | ||
| import static org.hamcrest.core.StringContains.containsString; | ||
| import static org.junit.jupiter.api.Assertions.*; |
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.
Expand *
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 feedback! I’ll update the code soon.
| 🎊 +1 overall
This message was automatically generated. |
| "same", | ||
| expected[startOffset + i], buf[i]); | ||
| Assertions.assertEquals(expected[startOffset + i], buf[i], | ||
| "Byte at " + (startOffset + i) + " should be the " + "same"); |
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’s better to use static imports to avoid situations like Assertions.*.
| @Test(expected=IOException.class) | ||
| @Test | ||
| public void testUpgradeFromPreUpgradeLVFails() throws IOException { | ||
| Assertions.assertThrows(IOException.class, () -> { |
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’s better to use static imports to avoid situations like Assertions.*.
| "localhost:9001"); | ||
| DFSUtil.getNamenodeNameServiceId(conf); | ||
| fail("Expected exception is not thrown"); | ||
| Assertions.assertThrows(HadoopIllegalArgumentException.class, () -> { |
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.
avoid Assertions.assertThrows
| Assert.assertNotNull(conf); | ||
| Assert.assertNotNull(fs); | ||
| Assert.assertEquals(0, ToolRunner.run(conf, | ||
| Assertions.assertNotNull(conf); |
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.
avoid Assertions.*
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 feedback—I'll update the code shortly.
| 🎊 +1 overall
This message was automatically generated. |
24e1822 to 03b991e Compare | 💔 -1 overall
This message was automatically generated. |
| 🎊 +1 overall
This message was automatically generated. |
| @zhtttylz Thanks for the contribution! Merged into trunk. |
| @slfan1989 for reviewing and merging! The remaining parts will be submitted soon. |
Description of PR
JIRA:HDFS-12431. Upgrade JUnit from 4 to 5 in hadoop-hdfs Part6.
How was this patch tested?
Junit Test.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?