jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [jclouds] nacx commented on a change in pull request #82: JCLOUDS-1552: Do not attempt to parse empty payload
Date Thu, 17 Sep 2020 11:32:14 GMT

nacx commented on a change in pull request #82:
URL: https://github.com/apache/jclouds/pull/82#discussion_r490171464



##########
File path: apis/sts/src/test/java/org/jclouds/aws/util/AWSUtilsTest.java
##########
@@ -88,6 +95,20 @@ public void testNoExceptionParsingTextPlain() {
       assertNull(utils.parseAWSErrorFromContent(command.getCurrentRequest(), response));
    }
 
+   /**
+    * Do not attempt to parse empty payload.
+    */
+   @Test
+   public void testNoExceptionEmptyPayload() {
+      utils.logger = mock(Logger.class);
+      utils.logger.warn(anyObject(Throwable.class), anyString());
+      expectLastCall().andThrow(new IllegalStateException("log spam"));
+      replay(utils.logger);
+      HttpResponse response = HttpResponse.builder().statusCode(NOT_FOUND.getStatusCode()).payload(new
StringPayload("")).build();
+      response.getPayload().getContentMetadata().setContentType("application/unknown");
+      assertNull(utils.parseAWSErrorFromContent(command.getCurrentRequest(), response));
+   }

Review comment:
       So, I think the right way to approach this is a bit different. You don't wanna check
that the code does not reach the `catch` branch. You wanna verify that it does not try to
even parse the response payload. Having a try/catch there is anecdotical; the logic you wanna
test is that if the payload is present but empty, the code doesn't attempt to parse it.
   
   For that, looking at the code you could for example mock the factory that is used to get
the apply function, or even provide a mock apply function and fail if it is called. I think
this approach would much better test the behaviour we want to fix here. WDYT?
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message