APIGW: expand coverage for API Keys and Usage Plans#13201
Conversation
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 17m 50s ⏱️ - 1h 43m 38s Results for commit f5684ac. ± Comparison against base commit 1913637. This pull request removes 3708 and adds 7 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 34m 19s ⏱️ Results for commit f5684ac. ♻️ This comment has been updated with latest results. |
bentsku
left a comment
There was a problem hiding this comment.
Nice work! Great added validation 🚀
Just 2 comments: one test name looks like it should fail, so maybe this should be updated. The other one is the PR description mentioning pagination, I think we could remove it from the scope of this PR 👍
Once this is addressed, we're good to merge! 👌
|
|
||
| create_api_key_2 = apigw_create_api_key(name=api_key_name_2) | ||
| snapshot.match("create-api-key-2", create_api_key) | ||
| snapshot.match("create-api-key-2", create_api_key_2) |
| if limit is not None: | ||
| if limit < 1 or limit > 500: | ||
| limit = None |
There was a problem hiding this comment.
nit: I think the PR description mentions pagination, but there are no tests for it, so I'd suggest to just drop this new code and merge it like that, the newly added validation is already really good 👍
There was a problem hiding this comment.
there was actually a test for the -1 value 👍 added it back
a665549 to
0b56bda
Compare
cloutierMat
left a comment
There was a problem hiding this comment.
Awesome! 🚀
Thanks for the extra validation and added tests! 💪
Motivation
We expand coverage for API Keys and Usage Plans.
Changes
get_api_keys/get_usage_plan_keysnameQuery (prefix/postfix/infix, case-insensitive) and pagination (limit+position)