-
Notifications
You must be signed in to change notification settings - Fork 90
Migrating to junit5 #232
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
Migrating to junit5 #232
Conversation
Import statement was changed. Also the way of handling exception was also updated
|
Looks worthwhile, thanks! I'll review soon. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #232 +/- ##
============================================
- Coverage 99.53% 98.39% -1.15%
- Complexity 171 173 +2
============================================
Files 10 10
Lines 428 435 +7
Branches 75 77 +2
============================================
+ Hits 426 428 +2
- Misses 1 3 +2
- Partials 1 4 +3 ☔ View full report in Codecov by Sentry. |
| Constructor<T> constructor; | ||
| try { | ||
| constructor = cls.getDeclaredConstructor(); | ||
| } catch (NoSuchMethodException e1) { | ||
| throw new RuntimeException(e1); | ||
| } catch (SecurityException e1) { | ||
| throw new RuntimeException(e1); | ||
| } | ||
| assertTrue(Modifier.isPrivate(constructor.getModifiers())); | ||
| constructor.setAccessible(true); | ||
| try { | ||
| constructor.newInstance(); | ||
| } catch (InstantiationException e) { | ||
| throw new RuntimeException(e); | ||
| } catch (IllegalAccessException e) { | ||
| throw new RuntimeException(e); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new RuntimeException(e); | ||
| } catch (InvocationTargetException e) { | ||
| throw new RuntimeException(e); | ||
| } |
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.
I missed this case.
Can I make change to this method?
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.
Sure
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.
@davidmoten That one missed change was also done. You can proceed with the review.
davidmoten
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.
Looks good, thanks!
|
oops, forgot to squash, I'll squash now |
|
squashed, thanks for the contribution @puthusseri! |
I tried to upgraded the code base to junit5 to get new features for unit testing. Below are the steps, I followed :
Steps: