gzhao9 opened a new issue, #8088:
URL: https://github.com/apache/cloudstack/issues/8088
<!--
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
~~~
---------
Hello CloudStack Developers,
I've observed that the `SwiftClientCfg` dependency is repeatedly mocked with
identical behavior in
[SwiftUtilTest.java](https://github.com/apache/cloudstack/blob/8a34afa8ab8c310f24038c0069fd1ce53b4b81d5/utils/src/test/java/com/cloud/utils/SwiftUtilTest.java)
The following is the duplicate code snippet I just mentioned. The creation
of mock objects was repeated a total of 9 times across 9 different test case.
```java
SwiftClientCfg cfg = mock(SwiftClientCfg.class);//
given(cfg.getEndPoint()).willReturn("swift.endpoint");
given(cfg.getAccount()).willReturn("cs");
given(cfg.getUserName()).willReturn("sec-storage");
given(cfg.getKey()).willReturn("mypassword");
// the last stub may willReturn() different values in different tests.
given(cfg.getStoragePolicy()).willReturn('policy');
```
The last method, `getStoragePolicy()` will be stubbed to return different
values in each test cases. But the rest of the stub behavior is exactly the
same.
We can eliminate this duplication by creating a method
[CreateMockSwiftClientCfg(String
policy)](https://github.com/gzhao9/cloudstack/blob/Refactoring-SwiftUtilTest/utils/src/test/java/com/cloud/utils/SwiftUtilTest.java#L184C1-L192C6)
in `SwiftUtilTest.java`.
The code snippet of the method is as follows:
```java
private SwiftClientCfg CreateMockSwiftClientCfg(String policy){
SwiftClientCfg cfg = mock(SwiftClientCfg.class);//
given(cfg.getEndPoint()).willReturn("swift.endpoint");
given(cfg.getAccount()).willReturn("cs");
given(cfg.getUserName()).willReturn("sec-storage");
given(cfg.getKey()).willReturn("mypassword");
given(cfg.getStoragePolicy()).willReturn(policy);
return cfg;
}
```
When a mock needs to be created in a test case, we can create it with this
method and take different stub return values in `getStoragePolicy()` as
parameter:
```java
SwiftClientCfg cfg = CreateMockSwiftClientCfg("policy1");
```
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
`testGetUploadCmd()` as an example:
[Code before
refactoring](https://github.com/apache/cloudstack/blob/8a34afa8ab8c310f24038c0069fd1ce53b4b81d5/utils/src/test/java/com/cloud/utils/SwiftUtilTest.java#L146C2-L159C1):
```java
@Test
public void testGetUploadCmd() {
SwiftClientCfg cfg = mock(SwiftClientCfg.class);//
given(cfg.getEndPoint()).willReturn("swift.endpoint");
given(cfg.getAccount()).willReturn("cs");
given(cfg.getUserName()).willReturn("sec-storage");
given(cfg.getKey()).willReturn("mypassword");
given(cfg.getStoragePolicy()).willReturn(null);
String uploadCmd = SwiftUtil.getUploadObjectCommand(cfg, "swift", "T-1",
"template.vhd", 1024);
String expected = "/usr/bin/python swift -A swift.endpoint -U
cs:sec-storage -K mypassword upload T-1 template.vhd";
assertThat(uploadCmd, is(equalTo(expected)));
}
```
[Code after
refactoring](https://github.com/gzhao9/cloudstack/blob/Refactoring-SwiftUtilTest/utils/src/test/java/com/cloud/utils/SwiftUtilTest.java#L130C1-L137C6):
```java
@Test
public void testGetUploadCmd() {
SwiftClientCfg cfg = CreateMockSwiftClientCfg(null);
String uploadCmd = SwiftUtil.getUploadObjectCommand(cfg, "swift", "T-1",
"template.vhd", 1024);
String expected = "/usr/bin/python swift -A swift.endpoint -U
cs:sec-storage -K mypassword upload T-1 template.vhd";
assertThat(uploadCmd, is(equalTo(expected)));
}
```
We have applied the same refactoring approach to the other 8 test cases,
you can see the refactored code in
[SwiftUtilTest.java](https://github.com/gzhao9/cloudstack/blob/Refactoring-SwiftUtilTest/utils/src/test/java/com/cloud/utils/SwiftUtilTest.java).
**As a result, we can reduce 45 lines of code, which makes the test cases
smaller, easier to understand, and better to maintain.**
**After the refactoring, if a developer needs to modify the mock creation of
`SwiftClientCfg`, they only need to modify `CreateMockSwiftClientCfg` instead
of modifying it in 9 different test cases before the refactoring.**
#### Benefits of the Proposed Refactoring:
- **Modularity:** The code is now more modular with a 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]
