spring-security icon indicating copy to clipboard operation
spring-security copied to clipboard

Factor out some common object mocking in tests

Open gzhao9 opened this issue 1 year ago • 5 comments

Hi there,

This PR is to address issue #14768, which involves reducing repeated mock object creation in tests.

I mistakenly closed PR #15256 and then discovered that I do not have permission to reopen it, receiving the message this branch was force pushed or recreated. I'm not sure if I should reopen the original or create this new PR

Although my issue submission mentioned 4 separate draft PRs, I combined them into this PR, since they all address the same problem.

gzhao9 avatar Jul 11 '24 05:07 gzhao9

@jesperronn Please feel free to let me know if there is anything I need to change. I look forward to your suggestions.

gzhao9 avatar Jul 11 '24 05:07 gzhao9

Thanks for the cleanup, @gzhao9. I think there is already a MockSecurityContextHolderStrategy available in spring-security-core. Can you please enhance that instead?

I think it would be reasonable to add the default constructor and one that takes a SecurityContext to that class.

Please also run ./gradlew format && ./gradlew check before committing next. It appears there are some formatting issues that those two tasks will help you work through.

When you are done, will you please change your commits to be more descriptive in the following way. For each commit, describe what you've done and include the issue number like so:

Use MockSecurityContextHolderStrategy

Issue gh-14768

For the final commit, please use Closes gh-14768 instead as this integrates with our build system to close the issue automatically.

I found MockSecurityContextHolderStrategy in spring-security-core. However, since they are not in the same package, I would need to change the dependency if I want to reference MockSecurityContextHolderStrategy in org.springframework.security.authorization.method. I believe adding new dependencies should be considered carefully. Therefore, I only added a private method in each test class.

gzhao9 avatar Sep 23 '24 20:09 gzhao9

I first focus on each file code modification. I will rebase commits when there is no need to modify the code or format.

gzhao9 avatar Sep 24 '24 14:09 gzhao9

Thanks for the updates, @gzhao9! I think there are two other items from the review that haven't been addressed yet. Are you able to do those as well?

jzheaux avatar Sep 26 '24 18:09 jzheaux

Thanks for the updates, @gzhao9! I think there are two other items from the review that haven't been addressed yet. Are you able to do those as well?

I have modified the code as required.

gzhao9 avatar Sep 28 '24 12:09 gzhao9

Thanks, @gzhao9! This is now merged into main.

jzheaux avatar Oct 25 '24 03:10 jzheaux