-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19415. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-common Part2. #7347
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. |
@cnauroth Can you help review this PR? Thank you very much! |
💔 -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.
Changes look good!
[ERROR] Failures:
[ERROR] org.apache.hadoop.fs.viewfs.TestViewFSOverloadSchemeCentralMountTableConfig.testLocalFsLinkSlashMergeWithOtherMountLinks
[ERROR] Run 1: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsLinkSlashMergeWithOtherMountLinks:149 Unexpected exception type thrown ==> expected: <java.io.IOException> but was: <java.lang.NullPointerException>
[ERROR] Run 2: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsLinkSlashMergeWithOtherMountLinks:149 Unexpected exception type thrown ==> expected: <java.io.IOException> but was: <java.lang.NullPointerException>
[ERROR] Run 3: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsLinkSlashMergeWithOtherMountLinks:149 Unexpected exception type thrown ==> expected: <java.io.IOException> but was: <java.lang.NullPointerException>
[INFO]
[ERROR] Errors:
[ERROR] org.apache.hadoop.fs.viewfs.TestViewFSOverloadSchemeCentralMountTableConfig.testLocalFsCreateAndDelete
[ERROR] Run 1: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsCreateAndDelete:113->addMountLinks:67 NullPointer
[ERROR] Run 2: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsCreateAndDelete:113->addMountLinks:67 NullPointer
[ERROR] Run 3: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsCreateAndDelete:113->addMountLinks:67 NullPointer
[INFO]
[ERROR] org.apache.hadoop.fs.viewfs.TestViewFSOverloadSchemeCentralMountTableConfig.testLocalFsLinkSlashMerge
[ERROR] Run 1: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsLinkSlashMerge:132->addMountLinks:67 NullPointer
[ERROR] Run 2: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsLinkSlashMerge:132->addMountLinks:67 NullPointer
[ERROR] Run 3: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalFsLinkSlashMerge:132->addMountLinks:67 NullPointer
[INFO]
[ERROR] org.apache.hadoop.fs.viewfs.TestViewFSOverloadSchemeCentralMountTableConfig.testLocalTargetLinkWriteSimple
[ERROR] Run 1: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalTargetLinkWriteSimple:93->addMountLinks:67 NullPointer
[ERROR] Run 2: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalTargetLinkWriteSimple:93->addMountLinks:67 NullPointer
[ERROR] Run 3: TestViewFSOverloadSchemeCentralMountTableConfig>TestViewFileSystemOverloadSchemeLocalFileSystem.testLocalTargetLinkWriteSimple:93->addMountLinks:67 NullPointer
TestViewFSOverloadSchemeCentralMountTableConfig
is a subclass of TestViewFileSystemOverloadSchemeLocalFileSystem
. In your patch, the base class has been converted to JUnit 5 with the Jupiter annotations. The subclass though is still using org.junit.Before
, so maybe its test setup is not getting triggered. Do we need to convert that one to org.junit.jupiter.api.BeforeEach
?
@cnauroth Thank you very much for reviewing the code! This is an issue we are already aware of—there are some abstract test classes in hadoop-common that are used by multiple modules, such as hadoop-azure, hadoop-aws, hadoop-distcp, etc. Our plan is to first upgrade the parts without dependencies, and then submit a unified PR to handle the dependent parts. Additionally, I will fix the checkstyle and blank issues. We hope that the feedback from Yetus will be positive, and the build will pass successfully. The fix using the method you provided is fine. We will replace it with |
💔 -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 !
@cnauroth Thank you very much for reviewing the code! I will merge this PR into the trunk branch and begin submitting the code for PART3. |
I just noticed that some tests are no longer running after this patch, e.g.
@slfan1989 , I guess the other upcoming hadoop-common patches will address remaining conversions like these file system contract tests? |
…2. (apache#7347) Co-authored-by: Chris Nauroth <[email protected]> Reviewed-by: Chris Nauroth <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
Description of PR
JIRA: HADOOP-19415. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-common Part2.
How was this patch tested?
Junit Test & mvn clean test.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?