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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
