-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Added null checks and tests to constructors #6915
Added null checks and tests to constructors #6915
Conversation
@clementkng Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
@clementkng Thank you for signing the Contributor License Agreement! |
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.
@clementkng thanks for the PR! I've left one piece of feedback inline.
@Test | ||
public void authorityWithNullRoleFailsAssertion() { | ||
try { | ||
JaasGrantedAuthority authority = |
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.
Thank you for adding these tests. For consistency with other tests in the codebase, would you please change the way you are handling these exceptions to use AssertJ instead? Something similar to:
assertThatThrownBy(() -> ...)
.isInstanceOf(IllegalArgumentException.class)
RequestKey, JaasGrantedAuthority, and SwitchUserGrantedAuthority assume certain final members are non-null. Issue: spring-projectsgh-6892
947de70
to
a9fbb70
Compare
@jzheaux Thanks for the review! I made the changes to the tests. |
Thanks, @clementkng, this is now merged into |
RequestKey, JaasGrantedAuthority, and SwitchUserGrantedAuthority
assume certain final members are non-null.
Issue: gh-6892