Validate cloudwatch2 registry using localstack #4979
Conversation
sullis
force-pushed
the
sean/cloudwatch2-localstack
branch
4 times, most recently
from
38acc5c
to
dfc9c72
Compare
sullis
force-pushed
the
sean/cloudwatch2-localstack
branch
from
dfc9c72
to
670574d
Compare
|
Ready for review |
|
Thank you for the pull request. This is something we've been wanting to do for a long time, and this looks like it should be a good approach. |
shakuzen
added
registry: cloudwatch
There was a problem hiding this comment.
Thank you for the PR, we definitely would like to have more integration tests!
Though I think in this form this might not be a real integration test yet or I might misunderstand what is happening here.
As the PR looks right now, I don’t get why localstack is needed. It seems verification happens against the mock (spy) so wether there was an http API call to CloudWatch (what would make this an integration test) is irrelevant from the test's point of view.
I think in order to make this an integration test and justify the usage of localstack, the test should call the AWS SDK to check if the published data is in localstack (CloudWatch) or not otherwise this could be a unit test with a different mocked/fake client without localstack (we might already have that kind of tests).
Could you please check if you can call the same cloudwatch client to get the data that was published by the registry and run verification against that data?
| @@ -95,7 +95,8 @@ assertj = { module = "org.assertj:assertj-core", version.ref = "assertj" } | |||
| awaitility = { module = "org.awaitility:awaitility", version.ref = "awaitility" } | |||
| aws-javaSdkCloudwatch = { module = "com.amazonaws:aws-java-sdk-cloudwatch", version.ref = "aws-cloudwatch" } | |||
| caffeine = { module = "com.github.ben-manes.caffeine:caffeine", version.ref = "caffeine" } | |||
| cloudwatch2 = { module = "software.amazon.awssdk:cloudwatch", version.ref = "cloudwatch2" } | |||
| cloudwatch2 = { module = "software.amazon.awssdk:cloudwatch", version.ref = "awsSdk2" } | |||
| awsSdk2NettyClient = { module = "software.amazon.awssdk:netty-nio-client", version.ref = "awsSdk2" } | |||
There was a problem hiding this comment.
Can't we use the default client shipped with the cloudwatch dependency?
| @@ -8,4 +8,7 @@ dependencies { | |||
|
|
|||
| testImplementation project(':micrometer-test') | |||
| testImplementation(libs.mockitoCore) | |||
| testImplementation(libs.testcontainers.localstack) | |||
| testImplementation(libs.awsSdk2NettyClient) | |||
| testImplementation(libs.logbackLatest) | |||
this PR uses the testcontainers-localstack module
https://java.testcontainers.org/modules/localstack/