-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29819 Upgrade hbase-asyncfs to use junit5 #7615
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
|
JUnit6 has been released, we should speed up for upgrading to junit 5. In this PR I also implemented a HBaseParameterizedTemplateProvider for simulating the parameterized test in JUnit4, which will be easier for us to migrate from junit4 to junit5. @stoty PTAL. Thanks. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
liuxiaocs7
left a comment
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 for the good start to migrate module by module at a time. I could help with the migration if needed.
| // get parameters | ||
| Method method; | ||
| try { | ||
| method = testClass.getDeclaredMethod("parameters"); |
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.
nit: IMO defining an annotation for the parameter provider method would be more in line with the Junit API than requiring a hardcoded method name.
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.
For now I think a hard coded method name is enough for our usage in HBase.
In JUnit5, we need to use @MethodSource("<method_name>") to reference the method, which does not have compile time check either...
We can improve this later.
stoty
left a comment
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 LGTM
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Signed-off-by: Istvan Toth <stoty@apache.org> Reviewed-by: Liu Xiao <liuxiao2103@qq.com> (cherry picked from commit 119c0ce)
Signed-off-by: Istvan Toth <stoty@apache.org> Reviewed-by: Liu Xiao <liuxiao2103@qq.com> (cherry picked from commit 119c0ce)
Signed-off-by: Istvan Toth <stoty@apache.org> Reviewed-by: Liu Xiao <liuxiao2103@qq.com> (cherry picked from commit 119c0ce)
Signed-off-by: Istvan Toth <stoty@apache.org> Reviewed-by: Liu Xiao <liuxiao2103@qq.com> (cherry picked from commit 119c0ce)
No description provided.