You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have been reviewing your test code and noticed an issue with repeated creation of mock objects in your tests. Here are four examples where this occurs:
The issue with this code is that willReturn("") is unnecessary because Mockito defaults the return of String methods to "". This piece of code is used 18 times across 8 test cases.
Thus, I propose the following solution:
SecurityContextHolderStrategystrategy = mock(SecurityContextHolderStrategy.class);
// other statementsgiven(strategy.getContext()).willReturn(newSecurityContextImpl(authentication));
The creation and given content of SecurityContextHolderStrategy mocks are very similar each time, except for the return value.
My solution is to create a MockSecurityContextHolderStrategy function, which is called each time a mock is needed:
I have created a Draft PR, and you can view the changes here.
These are the four instances of repeated mock test code I found, along with my solutions for refactoring the test code. I believe these changes will
reduce the LOC for each test case
allowing tests to focus more on testing rather than on recreating objects.
Most importantly, consolidating repetitive parts improves code maintainability.
Dear Spring Security developers, Do you like the solution proposed in the issue? I'm very much looking forward to your response, and I'm ready to start submitting PRs if you're interested. If you have any suggestions for improvements, I would greatly appreciate them.
The text was updated successfully, but these errors were encountered:
Thank you for your effort @gzhao9 to simplify these tests! To accelerate the review process, will you please instead open a PR to the main repository and I'll take a look?
Thank you for your effort @gzhao9 to simplify these tests! To accelerate the review process, will you please instead open a PR to the main repository and I'll take a look?
Thank you very much for your reply. I have submitted PR #15256. Please review it at your convenience.
I apologize for the delayed response as I've been occupied with some real-life matters over the past two months. I'm now back and actively engaged.
Dear Spring Security Developers,
I have been reviewing your test code and noticed an issue with repeated creation of mock objects in your tests. Here are four examples where this occurs:
Example 1
This example is found in ActiveDirectoryLdapAuthenticationProviderTests.
The repeated code fragment is as follows:
The issue with this code is that
willReturn("")
is unnecessary because Mockito defaults the return ofString
methods to""
. This piece of code is used 18 times across 8 test cases.Thus, I propose the following solution:
given(ctx.getNameInNamespace()).willReturn("");
DirContext ctx = mock(DirContext.class);
to@BeforeEach
This change reduces the code by 16 lines. I have created a Draft PR, and you can view the changes here.
Example 2
This code is repeated in 8 test cases across 3 test classes:
AuthorizationManagerAfterMethodInterceptorTests
AuthorizationManagerBeforeMethodInterceptorTests
PostFilterAuthorizationMethodInterceptorTests
The repeated code is as follows:
The creation and given content of
SecurityContextHolderStrategy
mocks are very similar each time, except for the return value.My solution is to create a
MockSecurityContextHolderStrategy
function, which is called each time a mock is needed:The code after calling it would look like this:
I have created a Draft PR, and you can view the changes here.
Example 3
Similar to Example 2, I found repetition in the test class ConcurrentSessionFilterTests when creating mocks for
SessionRegistry
as follows:This repetition occurs in 6 test cases.
My solution is to create a
mockSessionRegistry
:To create a
SessionRegistry
, simply call as follows:I have created
a Draft PR, and you can view the changes here.
Example 4
This repetition occurs in ExceptionTranslationFilterTests with the following code:
This occurs in 8 different test cases, each returning a different
Exception
.My solution is to create a method
mockFilterChainWithException
that takes differentException
s as parameters:To create a
FilterChain
mock, call it like this:I have created a Draft PR, and you can view the changes here.
These are the four instances of repeated mock test code I found, along with my solutions for refactoring the test code. I believe these changes will
Dear Spring Security developers, Do you like the solution proposed in the issue? I'm very much looking forward to your response, and I'm ready to start submitting PRs if you're interested. If you have any suggestions for improvements, I would greatly appreciate them.
The text was updated successfully, but these errors were encountered: