adding integ tests for S3Control to get better coverage for inconsist… #2261
Conversation
| @@ -20,6 +20,7 @@ | |||
| import static org.junit.Assert.assertNotNull; | |||
| import static org.junit.Assert.assertTrue; | |||
|
|
|||
| import org.assertj.core.api.ThrowableAssert; | |||
There was a problem hiding this comment.
Looks like this dependency wasn't used?
There was a problem hiding this comment.
Missed that, built with the quick flag...
| @@ -29,6 +30,7 @@ | |||
| import software.amazon.awssdk.http.SdkHttpFullRequest; | |||
| import software.amazon.awssdk.services.s3control.model.DeletePublicAccessBlockRequest; | |||
| import software.amazon.awssdk.services.s3control.model.GetPublicAccessBlockResponse; | |||
| import software.amazon.awssdk.services.s3control.model.InvalidRequestException; | |||
There was a problem hiding this comment.
Looks like this dependency also wasn't used?
| client.getPublicAccessBlock(r -> r.accountId(accountId)); | ||
| fail("Expected exception"); | ||
| } catch (S3ControlException e) { | ||
| assertEquals("NoSuchPublicAccessBlockConfiguration", e.awsErrorDetails().errorCode()); |
There was a problem hiding this comment.
Can we change this to assertThat? And same as all the other usages below? assertThat is more readable and flexible, and it's used more often in our code base.
There was a problem hiding this comment.
I prefer assertThat too. Changed all assertEquals in the file (but did not touch the assertTrue etc)
Codecov Report
@@ Coverage Diff @@
## master #2261 +/- ##
============================================
+ Coverage 77.63% 77.64% +0.01%
- Complexity 335 367 +32
============================================
Files 1231 1237 +6
Lines 38742 38904 +162
Branches 3052 3064 +12
============================================
+ Hits 30078 30208 +130
- Misses 7202 7234 +32
Partials 1462 1462
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
…ent exception unmarshalling
cenedhryn
force-pushed
the
salande/s3control-check-error-types
branch
from
b3675cd
to
3cbc82e
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
…4ed531483 Pull request: release <- staging/235e6696-1ffa-4ba0-a994-5aa4ed531483
…ent exception unmarshalling
Description
Adds integration tests for s3control
Motivation and Context
There is an ongoing issue which is now partially fixed:
#1986
These tests show that the error code is now correctly returned, but it does not return an InvalidRequestException, it's parsed as an S3ControlException, so the fix is not complete.
Testing
Local integ tests