Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mocking AWS Calls for Unit Testing for API Gateway #497

Merged
merged 1 commit into from Jul 13, 2023
Merged

Conversation

hongil0316
Copy link
Contributor

Description

Making changes in code to have proper unit testing without calling AWS API - https://aws.amazon.com/blogs/developer/mocking-out-then-aws-sdk-for-go-for-unit-testing/.

Currently, we call AWS API within unit tests and it causes several problems:

  • QuotaLimit
  • Timeout

Also, I believe we can reduce the code inside GetAllResources function significantly with the new pattern of encapsulating "get resource" logic inside the ResourceType. Afterall, a better name for ResourceType would be just Resource but will make changes incrementally once this change looks good for everyone.

Tackling issue: #494

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.
  • Attention Grunts - if this PR adds support for a new resource, ensure the nuke_sandbox and nuke_phxdevops jobs in .circleci/config.yml have been updated with appropriate exclusions (either directly in the job or via the .circleci/nuke_config.yml file) to prevent nuking IAM roles, groups, resources, etc that are important for the test accounts.

Release Notes (draft)

Added / Removed / Updated [X].

Updated [Mocking AWS Calls for Unit Testing for API Gateway]

Migration Guide

@@ -68,6 +68,7 @@ type ResourceType struct {

type FilterRule struct {
NamesRegExp []Expression `yaml:"names_regex"`
// TODO(james): consider adding time filter rule here instead of passing in as function argument in other place.
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this todo idea, it always seemed weird how that got tossed into ever function call

"github.com/stretchr/testify/require"
)

func GetTestSession(t *testing.T, approvedRegions []string, forbiddenRegions []string) *session.Session {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, maybe I miss something, but the function GetTestSession() is not invoked in any place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the implementation a bit afterward. It's no longer being used. Will delete it.

@hongil0316 hongil0316 merged commit 4a477e6 into master Jul 13, 2023
2 of 3 checks passed
@hongil0316 hongil0316 deleted the issues/494 branch July 13, 2023 22:32
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.

None yet

3 participants