You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/08/25 13:40:26 UTC
[GitHub] [cloudstack] chenhao-work opened a new issue, #6673: Refactor `TestHttp.testHttpclient` to avoid the Exception Suppression
chenhao-work opened a new issue, #6673:
URL: https://github.com/apache/cloudstack/issues/6673
##### ISSUE TYPE
* Improvement Request
##### COMPONENT NAME
unit-test
##### OS / ENVIRONMENT
N/A
##### SUMMARY
When I am running the test for [testHttpclient](https://github.com/apache/cloudstack/blob/ad0ae8397460c243e34d4017639d19a379c9e995/engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/TestHttp.java#L44) I noticed that the test case is using the Try-Catch block with `e.printStackTrace()` to suppress some of the Exceptions in the function.
```java
public void testHttpclient(String templateUrl) {
try {
...
final HttpGet getMethod = new HttpGet(templateUrl);
response = client.execute(getMethod);
final HttpEntity entity = response.getEntity();
output = new BufferedOutputStream(new FileOutputStream(localFile));
entity.writeTo(output);
} catch (final ClientProtocolException e) {
// TODO Auto-generated catch block
e.printStackTrace();
} catch (final IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
} finally {
try {
if (output != null) {
output.close();
}
} catch (final IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
...
}
```
These three catch blocks will only print the the Exception information and will not raise any failure in the testing process. This behavior will lead the developer harder to notice the issue in the CI\CD reports.
I suggest **removing** the try-catch structure and adding some additional assertion(if possible). This will improve the readability of the test cases, and avoid missing the test failure. Even if the exception does not raise by the component that we are testing here, we can still know that we need to fix the outside conditions first to make sure the test case running under stable and consistent conditions.
After refactoring
```java
public void testHttpclient(String templateUrl) throws ClientProtocolException IOException{
...
try {
HttpResponse response = client.execute(method);
length = Long.parseLong(response.getFirstHeader("Content-Length").getValue());
System.out.println(response.getFirstHeader("Content-Length").getValue());
final File localFile = new File("/tmp/test");
if (!localFile.exists()) {
localFile.createNewFile();
}
final HttpGet getMethod = new HttpGet(templateUrl);
response = client.execute(getMethod);
final HttpEntity entity = response.getEntity();
output = new BufferedOutputStream(new FileOutputStream(localFile));
entity.writeTo(output);
} finally {
if (output != null) {
output.close();
}
...
}
```
###### Operational impact
The testing refactoring won’t affect any part of the production code behavior. Also, only the original cases are replaced with unit tests.
--
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: commits-unsubscribe@cloudstack.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on issue #6673: Refactor `TestHttp.testHttpclient` to avoid the Exception Suppression
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on issue #6673:
URL: https://github.com/apache/cloudstack/issues/6673#issuecomment-1227300840
@chenhao-work thanks for the issue - are you going to work on them? You can directly raise pull requests for your ideas, no need to open issues if you've any pull requests in coming.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud closed issue #6673: Refactor `TestHttp.testHttpclient` to avoid the Exception Suppression
Posted by GitBox <gi...@apache.org>.
rohityadavcloud closed issue #6673: Refactor `TestHttp.testHttpclient` to avoid the Exception Suppression
URL: https://github.com/apache/cloudstack/issues/6673
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] chenhao-work commented on issue #6673: Refactor `TestHttp.testHttpclient` to avoid the Exception Suppression
Posted by GitBox <gi...@apache.org>.
chenhao-work commented on issue #6673:
URL: https://github.com/apache/cloudstack/issues/6673#issuecomment-1227982515
thanks for the reply! I will submit the PR ASAP! I have tried some initial PR drafts but the some of the local tests didn't passed so I haven't submitted it yet.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] boring-cyborg[bot] commented on issue #6673: Refactor `TestHttp.testHttpclient` to avoid the Exception Suppression
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #6673:
URL: https://github.com/apache/cloudstack/issues/6673#issuecomment-1227277108
Thanks for opening your first issue here! Be sure to follow the issue template!
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org