-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19691. [JDK17] Disallow JUnit4 Imports After JUnit5 Migration. #7976
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
@TaoYang526 We are currently working on upgrading the project to JUnit 5, and I have added a validation rule to prevent users from reintroducing JUnit 4 dependencies. During the review, I found that the testAsyncScheduleThreadExit method used JUnit 4 features, so I made some modifications. Could you please take a look and let me know if the changes are reasonable? |
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. Thanks @slfan1989 .
I was going to suggest also banning org.hamcrest
, but it looks like there is still a tiny amount of hamcrest remaining in YARN. Maybe this is a topic for a different PR.
> grep -r --include '*.java' 'org.hamcrest' *
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/src/test/java/org/apache/hadoop/yarn/appcatalog/controller/AppListControllerTest.java:import static org.hamcrest.MatcherAssert.assertThat;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/src/test/java/org/apache/hadoop/yarn/appcatalog/controller/AppListControllerTest.java:import static org.hamcrest.core.Is.is;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/src/test/java/org/apache/hadoop/yarn/appcatalog/controller/AppDetailsControllerTest.java:import static org.hamcrest.MatcherAssert.assertThat;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/src/test/java/org/apache/hadoop/yarn/appcatalog/controller/AppDetailsControllerTest.java:import static org.hamcrest.core.Is.is;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/src/test/java/org/apache/hadoop/yarn/appcatalog/controller/AppStoreControllerTest.java:import static org.hamcrest.MatcherAssert.assertThat;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/src/test/java/org/apache/hadoop/yarn/appcatalog/controller/AppStoreControllerTest.java:import static org.hamcrest.core.Is.is;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java:import static org.hamcrest.MatcherAssert.assertThat;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java:import static org.hamcrest.core.IsInstanceOf.instanceOf;
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java:import static org.hamcrest.core.IsSame.sameInstance;
💔 -1 overall
This message was automatically generated. |
@cnauroth Thank you for reviewing the code! I’ll submit a separate PR to replace the usage of |
💔 -1 overall
This message was automatically generated. |
dd91edc
to
c161c9b
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
This PR enforces import restrictions to prohibit the use of JUnit4. The previously reported shade error has already been resolved in #7995. Given the lengthy compilation time, I will not re-trigger the build for this PR. I will continue to investigate and address the YARN crash issue separately. @cnauroth Thanks for the review! |
Description of PR
JIRA: HADOOP-19691. [JDK17] Disallow JUnit4 Imports After JUnit5 Migration.
As our project has fully migrated to JUnit5, we should now enforce a rule that prevents the import and usage of JUnit4 classes (such as org.junit.Test, org.junit.Assert, etc.) to ensure consistency, avoid regressions, and allow safe removal of legacy dependencies.
This task involves identifying and eliminating any remaining JUnit4 imports, and introducing static code analysis or linting rules to ban future usage.
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?