-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Provide helpful message if parameter cannot be set. #1465
Provide helpful message if parameter cannot be set. #1465
Conversation
@@ -73,6 +73,10 @@ private Object createTestUsingFieldInjection() throws Exception { | |||
int index = annotation.value(); | |||
try { | |||
field.set(testClassInstance, parameters[index]); | |||
} catch (IllegalAccessException e) { | |||
throw new Exception("Cannot set parameter '" + field.getName() |
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.
Are you sure we shouldn't set the cause of this exception?
Also, throwing ``Exceptionis rare. Perhaps we should throw a new
IllegalAccessException`
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.
You're right with the cause. The problem with creating a new IllegalAccessException
is that it doesn't have a constructor with a cause
argument.
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.
You can use https://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#initCause(java.lang.Throwable) to set the cause.on the IllegalAccessException
. Can we do that?
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.
And again you're right. I fixed it and already squashed the commits.
ba59698
to
46c6ea9
Compare
When I have your okay for the pull request I will squash the commits and merge to master. |
46c6ea9
to
7e3a6b9
Compare
ad3d20c
to
709a705
Compare
For private @parameter fields is users get an exception like "java.lang.IllegalAccessException: Class ... can not access a member of class X with modifiers private" The new message "Cannot set parameter 'parameter'. Ensure that the the field 'parameter' is public." tells the user what they should do. The reason for adding this feature is the Stackoverflow question https://stackoverflow.com/questions/44522046/reflection-exception-in-parameterized-junit-test-using-array-parameter/44522988
709a705
to
6324fde
Compare
For private @parameter fields is users get an exception like "java.lang.IllegalAccessException: Class ... can not access a member of class X with modifiers private" The new message "Cannot set parameter 'parameter'. Ensure that the the field 'parameter' is public." tells the user what they should do.
The reason for adding this feature is the Stackoverflow question https://stackoverflow.com/questions/44522046/reflection-exception-in-parameterized-junit-test-using-array-parameter/44522988