Aws test container #134
Aws test container #134
Conversation
There was a problem hiding this comment.
For 1) I agree that you can use an env var to set the AWS endpoint. Something like MAGPIE_AWS_ENDPOINT. My first thought is to create a static utility method that will return the endpoint (defaulting to aws.amazon.com if not specified). Then each service will pass along that result to it's constructor.
For 3) Don't sweat the localstack demo env. We'll cross that bridge when we get to it.
belosh59
force-pushed
the
aws-test-container
branch
2 times, most recently
from
45083bc
to
defe72e
Compare
magpie-aws/src/main/java/io/openraven/magpie/plugins/aws/discovery/services/S3Discovery.java
Outdated
Show resolved
Hide resolved
magpie-aws/src/main/java/io/openraven/magpie/plugins/aws/discovery/AWSUtils.java
Outdated
Show resolved
Hide resolved
magpie-aws/src/main/java/io/openraven/magpie/plugins/aws/discovery/AWSUtils.java
Outdated
Show resolved
Hide resolved
belosh59
force-pushed
the
aws-test-container
branch
from
defe72e
to
d8c910d
Compare
|
Added:
|
belosh59
force-pushed
the
aws-test-container
branch
2 times, most recently
from
0c73d55
to
d1388b1
Compare
belosh59
force-pushed
the
aws-test-container
branch
from
d1388b1
to
506709e
Compare
| } | ||
|
|
||
| @Test | ||
| public void testEC2Discovery() { |
There was a problem hiding this comment.
whenRunningEc2DiscoveryShouldReturnInstanceEipSecurityGroupVolumeAndSnapshotInformation
It's probably good idea to split this test into 5. Each testing one thing
There was a problem hiding this comment.
Got your point @PrzemyslawTusinski . We rather have here a complete testing for chosen component from magpie discovery perspective, instead of the unit testing when we could write tests isolated for various corner cases.
Splitting works good but we are going to expose our private methods for component discovery of the various aspects.
For example how we made with IAM - but I dont really like to change private method to protected just for the test availability.
I threat those tests as e2e AWS component discovery based on localstack. It like an IT with WireMocks when you passing from the controller request to database layer and back verifying the response.
In any case I propose to change method naming convention here once we will extend them with additional coverage
There was a problem hiding this comment.
I didn't meant exposing private methods.
I meant running full discovery in each test case, then during asset phase assert only one resource on each test case
| } | ||
|
|
||
| @Test | ||
| public void testDynamoDBDiscovery() { |
There was a problem hiding this comment.
whenDiscoveringDynamoDbTablesShouldReturnProperContentInfo
| } | ||
|
|
||
| @Test | ||
| public void testKMSDiscovery() { |
| } | ||
|
|
||
| @Test | ||
| public void testCloudWatchDiscovery() { |
| private ArgumentCaptor<MagpieEnvelope> envelopeCapture; | ||
|
|
||
| @Test | ||
| public void testAccountDiscovery() { |
| } | ||
|
|
||
| @Test | ||
| public void testRoute53Discovery() { |
| } | ||
|
|
||
| @Test | ||
| public void testS3Discovery() { |
| } | ||
|
|
||
| @Test | ||
| public void testSNSDiscovery() { |
| } | ||
|
|
||
| @Test | ||
| public void testSecretManagerDiscovery() { |
| assertTopic(invocations.get(0)); | ||
| assertSubscription(invocations.get(1)); |
There was a problem hiding this comment.
Please consider splitting this to 2 test cases
There was a problem hiding this comment.
This will be possible only if we will expose methods discoverTopics and discoverSubscriptions.
Not sure if we want it for test purpose. Initially I was designing cumulative approach to test input/output for available exposed method.
This decrease execution time and leading to the comparable coverage.
But it could fail because issue in either topic or subscription - at this point of IT testing that should be acceptable.
I will propose separate refactoring if required later.
|
Builded and tested against the real AWS Services |
Preliminary code which expose the current problems during test containers setup
DynamoDB discovery is working, you could try it.
At the moment we are creating dependencies for our selves (clients), so that it becomes difficult to inject localstack clients into the code. We need to expose private methods. But that also not working. Even deeper we also create such dependancy (S3 discovery is not working due to that perspective)
Otherwise with current clients we are trying to reach the real AWS.
1.1. I'm thinking about lightweight ServiceDiscovery pattern, but we need to keep clients being configured for different Regions being requested (e.g DynamoDB_AWS_GLOBAL / DynamoDB_US_WEST_1 )
Seems like AWS Java SDK doesn't provide possibility to override AWS Endpoint globally for all clients link
For a demo purpose we could setup localstack separately but we need to bypass localstack docker URL to reach it. Meanwhile there no possibility for that in code
Proposal:
We could create our AWS Clients based on env variable being passed on the stage of execution - > if nothing passed fallback to the default one - https://aws.amazon.com/
[To not merge yet]