Skip to content

Conversation

sannya-singal
Copy link
Contributor

For DocumentationPart API in apigateway :

  • Wrote create, list, delete, update, invalid operations, etc. test cases
  • Resolved issues for json patching, invalid messages, conflicts in responses, etc.

@sannya-singal sannya-singal temporarily deployed to localstack-ext-tests March 23, 2023 10:19 — with GitHub Actions Inactive
@sannya-singal sannya-singal requested a review from bentsku March 23, 2023 11:12
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this new resource! That's a lot of snapshot tests! 🎉 I have one comment regarding modifying generated code, and some other minor ones.

We're covering a lot of ground here, and explicitly validating the location type when creating DocumentationPart. We could also add a note that we're not covering the whole validation of the location (and adding the link below), as it seems to be very extensive, and it's outside of the scope for now, see https://docs.aws.amazon.com/apigateway/latest/api/API_DocumentationPartLocation.html
There are a lot of different cases, depending on each other, well, it's a lot, and we don't need that. Thanks for tackling the location type validation for now 😄

Thanks again for separating the tests into a class, and writing so many! 🎉

@sannya-singal sannya-singal temporarily deployed to localstack-ext-tests March 23, 2023 12:51 — with GitHub Actions Inactive
@sannya-singal
Copy link
Contributor Author

Thanks @bentsku for the review, have updated the PR 🙏🎉

@github-actions
Copy link

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 34m 55s ⏱️ - 13m 56s
1 850 tests +6  1 458 ✔️ +5  392 💤 +1  0 ±0 
2 586 runs  +6  1 823 ✔️ +5  763 💤 +1  0 ±0 

Results for commit a1e1cf58. ± Comparison against base commit 85a00cb.

@coveralls
Copy link

Coverage Status

Coverage: 85.013% (+0.1%) from 84.865% when pulling a1e1cf5 on apigw2 into 85a00cb on master.

Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀 awesome set of changes, thanks for quickly working on the comments! Really awesome to see it thoroughly validated. Thank you!

@sannya-singal sannya-singal merged commit 0cae781 into master Mar 24, 2023
@sannya-singal sannya-singal deleted the apigw2 branch March 24, 2023 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants