-
Notifications
You must be signed in to change notification settings - Fork 9k
HADOOP-19433. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-extras. #7586
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
base: trunk
Are you sure you want to change the base?
Conversation
💔 -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.
TestGetGroups
ran tests in the base class GetGroupsTestBase
, which failed. Does JUnit 5 @Disabled
not disable tests in superclasses? Would we need to override each method and apply method-level @Disabled
annotations?
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@cnauroth Thank you for reviewing the code. Apologies for not noticing your comment earlier, as I was focused on upgrading other modules to JUnit 5. I believe your suggestion is correct, and I have made the necessary changes. However, I noticed that the unit tests are still being executed. I suspect this may be related to the fact that we are still in a mixed testing mode using both JUnit 4 and JUnit 5. |
|
||
/** | ||
* Tests for the MR implementation of {@link GetGroups} | ||
*/ | ||
@Ignore | ||
@Disabled |
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.
The test was disabled 14 years ago (https://issues.apache.org/jira/browse/MAPREDUCE-3860)
Maybe we should just remove it.... just saying.
@@ -52,4 +53,33 @@ protected Tool getTool(PrintStream o) { | |||
return new GetGroups(conf, o); | |||
} | |||
|
|||
@Test |
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.
Just curious. Are these test method overrides necessary?
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.
Nvm just noticed your comments above!
Description of PR
JIRA: HADOOP-19433. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-extras.
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?