ASF: validate full CloudWatch suite with multi-protocol #13173
Conversation
bentsku
added
aws:cloudwatch
Test Results - Alternative Providers180 tests 39 ✅ 2m 32s ⏱️ Results for commit 59885b4. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Acceptance7 tests 5 ✅ 3m 24s ⏱️ Results for commit 59885b4. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 58m 13s ⏱️ Results for commit 59885b4. ♻️ This comment has been updated with latest results. |
bentsku
changed the base branch from
main
to
cloudwatch-multi-protocol-runs-fixes
bentsku
force-pushed
the
cloudwatch-multi-protocol-runs-fixes
branch
from
bb7ab45 to
cc64c6a
Compare
bentsku
force-pushed
the
cloudwatch-multi-protocol-runs-2
branch
from
115f927 to
e992f0f
Compare
bentsku
changed the title
bentsku
force-pushed
the
cloudwatch-multi-protocol-runs-2
branch
2 times, most recently
from
1207e2f to
dcdcdd6
Compare
LocalStack Community integration with Pro 2 files 2 suites 43m 15s ⏱️ Results for commit 59885b4. ♻️ This comment has been updated with latest results. |
bentsku
force-pushed
the
cloudwatch-multi-protocol-runs-2
branch
3 times, most recently
from
0bd8183 to
17e0356
Compare
bentsku
force-pushed
the
cloudwatch-multi-protocol-runs-2
branch
from
17e0356 to
59885b4
Compare
alexrashed
left a comment
alexrashed
left a comment
There was a problem hiding this comment.
A great last step for the cloudwatch multi protocol / cbor support! Absolutely great to see that all the tests are working just fine for all the protocols in parallel now! 🤩 
🦸🏽
| super().__init__("ValidationError", message, 400, True) | ||
|
|
||
|
|
||
| class InvalidParameterCombination(CommonServiceException): |
There was a problem hiding this comment.
praise: Nice findings on the side! 🧹
There was a problem hiding this comment.
praise: Wow, this file here has seen quite a cleanup! 💯 Lots of small fixes (datetime, but also smaller changes like timeout adjustments based on the target etc.). And of course the usage of the multi protocol cloudwatch client fixture, which basically executes the tests for all different protocols! Absolutely great that we have incredible test coverage on the new ASF multi-protocol capabilities and the CBOR parsers and serializers! 💯
Once CloudWatch has been moved towards JSON and CBOR, we might want to think about decreasing the amount of tests here a bit in order to decrease the test execution time again.
There was a problem hiding this comment.
I'll create a ticket to keep track of the clean up, and will spend a bit of time selecting tests and removing the parametrization for tests that are somewhat duplicates on the parser/serializer level 😄 thanks for mentioning it!
pinzon
left a comment
pinzon
left a comment
There was a problem hiding this comment.
Awesome PR. Thank you for the clean up 👍
alexrashed
added
the
notes: skip
Motivation
This PR is part of a bigger initiative (see #13028) to implement support for the new Smithy Protocol
smithy-rpc-v2-cbor, as well as the new Botocore multi-protocol support for single services (before, a service could only define one protocol)This PR finishes to run and regenerate all snapshots for the CloudWatch test suite, now that we have everything working.
There's only one very minor fix regarding a return type, where CloudWatch specifies a string and we returned an
int.Other than this, nothing to note, and we now have the full CloudWatch suite running with multi-protocol 🚀 or almost, some of tests (metrics, and one integration with lambda) don't actually rely on CloudWatch or its protocol, so I didn't migrate them in order to reduce the runtime of the suite.
Changes
GetMetricDatadatetime.utcnow()and removed some warnings