gzhao9 opened a new issue, #8087:
URL: https://github.com/apache/cloudstack/issues/8087
<!--
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release and main branch are affected too.
Always add information AFTER of these HTML comments, but no need to delete
the comments.
-->
##### ISSUE TYPE
<!-- Pick one below and delete the rest -->
* Improvement Request
* Enhancement Request
##### COMPONENT NAME
<!--
Categorize the issue, e.g. API, VR, VPN, UI, etc.
-->
~~~
Testing
~~~
##### CLOUDSTACK VERSION
<!--
New line separated list of affected versions, commit ID for issues on main
branch.
-->
[4.18-reflections-update](https://github.com/apache/cloudstack/tree/4.18-reflections-update)
~~~
4.18-reflections-update
~~~
---------
Hi CloudStack developers,
I have noticed that a dependency `TungstenAnswer` is mocked a total 151
times, with similar/identical behavior across three different test suites:
- In class
[TungstenElementTest.java](https://github.com/apache/cloudstack/blob/8a34afa8ab8c310f24038c0069fd1ce53b4b81d5/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenElementTest.java),
there were 41 identical duplicate creations in 22 test cases.
- In class
[TungstenFabricUtilsTest.java](https://github.com/apache/cloudstack/blob/8a34afa8ab8c310f24038c0069fd1ce53b4b81d5/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenFabricUtilsTest.java),
there were 2 identical duplicate creations in 2 test cases.
- In class
[TungstenServiceImplTest.java](https://github.com/apache/cloudstack/blob/8a34afa8ab8c310f24038c0069fd1ce53b4b81d5/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenServiceImplTest.java),
there were 53 identical duplicate creations and 55 similar duplicate creations
in 68 test cases.
The following is the duplicate code snippet I just mentioned. It has been
repeated 96 times with completely identical logic in the 3 testclasses:
```java
TungstenAnswer answer=mock(TungstenAnswer.class);
when(answer.getResult()).thenReturn(Result);
```
We can eliminate this duplication by creating a
[CreateMockTungstenAnswer]((https://github.com/gzhao9/cloudstack/blob/refactoring-org.apache.cloudstack.network.tungsten.service/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/CreateMockTungstenAnswer.java))
class.
The code snippet of `CreateMockTungstenAnswer`:
```java
public class CreateMockTungstenAnswer {
//Attribute is for reuse mock creation.
TungstenAnswer answer;
//Implement duplicate mock and stub behavior in the constructor.
CreateMockTungstenAnswer(boolean Result){
answer=mock(TungstenAnswer.class);
when(answer.getResult()).thenReturn(Result);
}
TungstenAnswer get(){return this.answer;}
}
```
When a mock needs to be created in a test case, we can create it with this
class like:
```java
TungstenAnswer answer=new CreateMockTungstenAnswer(true).get();
```
Also, in `TungstenServiceImplTest.java`, the creation of mock for
`TungstenAnswer` is repeated another 55 times, but with a slightly different
stub method in mock creation. For example, in test case
`addTungstenDefaultNetworkPolicyTest()`:
```java
TungstenAnswer createTungstenPolicyAnswer=mock(TungstenAnswer.class);
when(createTungstenPolicyAnswer.getResult()).thenReturn(true);
// following are additional different stub from other mock creation
when(createTungstenPolicyAnswer.getApiObjectBase()).thenReturn(networkPolicy);
```
In comparison, in the test `addTungstenDefaultNetworkPolicyTest()`, the stub
for the 1st method is same, but there will be an additional 2nd or 3rd method
compared to the 96 identical duplicate mock creations.
We can still eliminate repetition by creating a reusable, "basic" mock
object, but at each mock creation, we can customize the stub of the other
methods.
```java
TungstenAnswer createTungstenPolicyAnswer=new
CreateMockTungstenAnswer(true).get();
when(createTungstenPolicyAnswer.getApiObjectBase()).thenReturn(networkPolicy);//
Add a different stub behavior.
```
To be more intuitive, the following shows the comparison of how a test case
looks before and after the refactoring, with the code snippet of
`syncTungstenDbWithCloudstackProjectsAndDomainsTest()` in test suit
[TungstenServiceImplTest](https://github.com/apache/cloudstack/blob/8a34afa8ab8c310f24038c0069fd1ce53b4b81d5/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenServiceImplTest.java#L654)
as an example:
Code before refactoring:
```java
@Test
public void syncTungstenDbWithCloudstackProjectsAndDomainsTest() {
// The code before this comment is the same
TungstenAnswer createTungstenDomainAnswer = mock(TungstenAnswer.class);
TungstenAnswer createTungstenProjectAnswer = mock(TungstenAnswer.class);
TungstenAnswer createTungstenDefaultProjectAnswer =
mock(TungstenAnswer.class);
TungstenAnswer updateTungstenDefaultSecurityGroupAnswer =
mock(TungstenAnswer.class);
when(createTungstenDomainAnswer.getResult()).thenReturn(true);
when(createTungstenProjectAnswer.getResult()).thenReturn(true);
when(updateTungstenDefaultSecurityGroupAnswer.getResult()).thenReturn(true);
when(createTungstenDefaultProjectAnswer.getResult()).thenReturn(true);
//The rest of the sentence is exactly the same before and after the
refactoring.
}
```
Code after refactoring:
```java
@Test
public void syncTungstenDbWithCloudstackProjectsAndDomainsTest() {
// The code before this comment is the same
TungstenAnswer createTungstenDomainAnswer = new
CreateMockTungstenAnswer(true).get();
TungstenAnswer createTungstenProjectAnswer = new
CreateMockTungstenAnswer(true).get();
TungstenAnswer createTungstenDefaultProjectAnswer = new
CreateMockTungstenAnswer(true).get();
TungstenAnswer updateTungstenDefaultSecurityGroupAnswer = new
CreateMockTungstenAnswer(true).get();
//The rest of the sentence is exactly the same before and after the
refactoring.
}
```
We've applied a similar refactoring approach to three test suites,
[TungstenServiceImplTest.java](https://github.com/gzhao9/cloudstack/blob/refactoring-org.apache.cloudstack.network.tungsten.service/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenServiceImplTest.java),
[TungstenFabricUtilsTest.java]() and [TungstenElementTest.java
](https://github.com/gzhao9/cloudstack/blob/refactoring-org.apache.cloudstack.network.tungsten.service/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenElementTest.java).
**As a result, we can reduce 321 lines of code, which makes the test cases
smaller, easier to understand, and better to maintain.**
**After refactoring, if the `TungstenAnswer` mock creation needs to be
modified in the future, the developer only needs to modify one time in
`CreateMockTungstenAnswer` instead of 151 places of duplicate mock creation in
3 testclasses.**
#### Benefits of the Proposed Refactoring:
- **Modularity:** The code is now more modular with specific class for
creating mock object.
- **Readability:** Test cases are more readable, focusing on specific test
logic rather than the setup code.
- **Maintainability:** Easier maintenance due to the reduction of redundancy
and a cleaner, more structured codebase.
- **Scalability:** Facilitates the addition of new test cases and makes
modifications to existing ones more efficient.
I'd love to hear your thoughts on this proposal! Do you agree with any of
these changes, or do you have other insights or preferences?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
