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

RFC: Integration tests mechanism #1251

Closed
1 of 2 tasks
heitorlessa opened this issue Jun 17, 2022 · 13 comments
Closed
1 of 2 tasks

RFC: Integration tests mechanism #1251

heitorlessa opened this issue Jun 17, 2022 · 13 comments
Assignees
Labels

Comments

@heitorlessa
Copy link
Contributor

heitorlessa commented Jun 17, 2022

Is this related to an existing feature request or issue?

#1009

Which AWS Lambda Powertools utility does this relate to?

Other

Summary

This RFC formalizes our thought process on integration tests (integ test). It also clarifies when functional, integration, and end-to-end tests are best suited.

Requirements

  • Setup/tear down overall process should be faster than 2m for 80% of test resources
  • Optionally setup/tear down in an independent step to ease step through debugging of multiple independent tests
  • Use local codebase while targeting Cloud resources
  • No Lambda functions involved
  • Ability to delete orphan resources
  • Use the same framework to compose and deploy/delete infra as E2E

For security, integ tests will run on either scenarios: 1/ Locally targeting one's AWS Test Account, 2/ New and Existing PRs upon two approvers, and 3/ Existing PRs labelled run-integ after considered safe by maintainers.

Use case

Integ tests can increase quality confidence in Powertools utilities with direct integration with AWS, e.g. Parameters, Feature Flags, etc.

Functional tests will remain useful to validate interfaces and pure Python logic. However, they can't help us detect regressions in contracts built on assumptions that integrations would remain backwards compatible.

NOTE: The majority of our tests will continue to be functional tests (subset of integ test) given speed and confidence ratio. Integ tests for extra confidence in direct integrations, and E2E for the complete picture we won't be able to simulate in either tests.

Proposal

Firstly, we formalize the definition of our tests. There will always be exceptions to the rule and we defer judgement to maintainers, as these can always be reverted if proven inefficient.

Test classification

Test When to write Notes Speed
Unit tests Verify the smallest possible unit works. Networking access is prohibited. Prefer Functional tests given our complexity. Lightning fast (nsec to ms)
Functional tests Guarantee functionality works as expected. It's a subset of integration test covering multiple units. No external dependency. Prefer Fake implementations (in-memory) over Mocks and Stubs. Fast (ms to few seconds at worst)
Integration tests Gain confidence that code works with one or more external dependencies. No need for a Lambda function. Use our code base against an external dependency e.g., fetch an existing SSM parameter. Moderate to slow (a few minutes)
End-to-end tests Gain confidence that a Lambda function with our code operates as expected. It simulates how customers configure, deploy, and run their Lambda function - Event Source configuration, IAM permissions, etc. Slow (minutes)
Performance tests Ensure critical operations won't increase latency and costs to customers. CI arbitrary hardware can make it flaky. We'll resume writing perf test after our new Integ/End have significant coverage. Fast to moderate (a few seconds to a few minutes)

Approach

We will follow the same approach as for E2E - Here's the most up to date structure.

Out of scope

This RFC doesn't cover E2E tests, nor contribution guide updates on which tests we will require.

Potential challenges

These are the immediate foreseen challenges:

  • Lack of good AWS testing tools. We'll likely need to create, improve, or contribute to emerging solutions to reduce boilerplate so we can focus on testing contracts only.
  • Event data source classes. At the time of writing, there isn't an official schema for AWS Lambda Events. This means we might reconsider this being out of scope until we have a scalable solution. One idea is to spin up infrastructure on a schedule to generate events for every event data source classes we support. This would be unrelated to our integ test infra.
  • Future non-Serverless resources. In the medium-term future, we will support additional Idempotency persistency store like Redis, similarly for Parameters and Feature Flags. This will increase complexity in authoring, maybe maintaining, the CloudFormation approach. We'll reconsider this approach when we cross that bridge.

Dependencies and Integrations

AWS CloudFormation to create resources required for each feature we want to integ test within a utility. Each utility will dictate which integration will be required.

Services we currently integrate regardless (excluding event sources):

  • AWS X-Ray
  • Amazon CloudWatch Logs
  • Amazon CloudWatch Metrics
  • Amazon API Gateway REST API
  • Amazon API Gateway HTTP API
  • Amazon Elastic Load Balancer
  • AWS AppSync
  • AWS Lambda
  • AWS Systems Manager Parameter Store
  • AWS Secrets Manager
  • AWS AppConfig
  • Amazon DynamoDB
  • Amazon DynamoDB Streams
  • Amazon SQS
  • Amazon SNS
  • Amazon Kinesis Data Streams

Alternative solutions

We looked into AWS SDK for speed. However, given the number of integ test and the potential AWS cost it could incur customers in the event of stateful orphan resources (e.g., DynamoDB w/ several records), this became sub-optimal. We could create a static method to list/delete orphan resources for each resource we wanted to test, but this would need more thought on keeping state locally or created resources.

Terraform (TF) was a strong candidate given the speed and state management. Because E2E tests will be done in CloudFormation, it's more appropriate to stick with the same tool despite CDK usage in E2E tests. On a minor front, TF would need additional tools and docs for maintainers and contributors unfamiliar with it. We can easily reconsider this option, especially if we can achieve 50% greater speed.

Acknowledgment

@heitorlessa heitorlessa added RFC triage Pending triage from maintainers labels Jun 17, 2022
@pcolazurdo
Copy link

Should we really have a different provisioning model between E2E and integration tests? I think having to support two different models will he harder for maintainers and difficult to maintain in-sync.

@pcolazurdo
Copy link

Suggestion: Creation/Tearing down of resources should be configurable for local tests. I can see how annoying it can be to have to run everything for a small changes that the developer knows don't have dependency on existing data (i.e. when running only specific tests in debugging a particular problem)

@pcolazurdo
Copy link

Recommendation so we don't forget: The generated cloudformation should go through a cfn-lint/cfn-guard process before executing it.

@heitorlessa
Copy link
Contributor Author

Suggestion: Creation/Tearing down of resources should be configurable for local tests. I can see how annoying it can be to have to run everything for a small changes that the developer knows don't have dependency on existing data (i.e. when running only specific tests in debugging a particular problem)

That's factored in. The only trade off we're making is to not have a different stack per utility -- this will slow down deployment/tear down considerably.

Should we really have a different provisioning model between E2E and integration tests?

They're both CloudFormation. The differences are that E2E will use CDK to synthesise the various combinations of stacks, and use CFN to deploy; they create wildly different stacks, need a building process, packaging, permissions set and more.

For Integ tests, we'll only create the resources our local codebase will integrate with -- a DynamoDB table, a SSM Parameter, a Secret -- no Lambda functions, no build or packaging process necessary. These will be considerably few resources making (up to 10x less than E2E).

I personally find CDK an overkill for Integ tests in this context, but happy to hear it differently. The context switching is a good point tho - maybe it's worth exploring for one day to test ergonomics when we start implementation.

@heitorlessa
Copy link
Contributor Author

Note to self for Monday before I forget: Experiment with stack per feature via conftest, it'll be easier to swap to CDK if we need to AND give us full test vs per feature step through debugging when we needed -- slower but more practical

@mploski
Copy link
Contributor

mploski commented Jun 19, 2022

@heitorlessa This RFC looks pretty good! Few remarks from my side:

  1. In test classification It would be great to provide specific example along with use case (as another column) - this will allow reader to visualise it and clarify some typical confusion around testing: 1/ For example functional testing may be understood as extension to integration testing. 2/ Also e2e tests might be understood as rare tests that we want to fire to cover most of the features/or most common feature from user perspective, not specifically most complex one/the one with side effect.
  2. In approach section I think we should also add points around maintainability/readability. That was one of the point to use CDK SDK for E2E testing to generate CF stack as mantainer/user can quickly read through it, without need to dive deep in verbose json/yaml files.
  3. Regarding point from @pcolazurdo about using one tool for integ/e2e tests I'm also in favour of this approach:
    1/ One way of spawning infrastructure regardless type of tests exposed to mantainer/contributor will cause less confusion/ less maintenance effort.
    2/ As long as we expose infra creation action as separate lib/fixture with agreed interface for e2e testing, it should be straightforward to reuse it directly in integration tests also
    3/ CDK SDK's packaging and uploading overhead should be relatively low (although we might measure it to confirm this assumption) .In integration tests we won't create any additional assets like lambda or layer code that needs to be uploaded into s3 bucket first. The only thing CDK SDK would be responsible for is creating CF stack yaml file in local environment. I can help with conducting such tests if you want

@heitorlessa
Copy link
Contributor Author

heitorlessa commented Jun 19, 2022 via email

@heitorlessa heitorlessa removed the triage Pending triage from maintainers label Jul 4, 2022
@heitorlessa heitorlessa added the need-more-information Pending information to continue label Jul 25, 2022
@heitorlessa
Copy link
Contributor Author

Finally finished the E2E refactoring work. We can now use Integ tests with the same framework. Test definitions have been updated too - I'll update the RFC body as soon as I get a chance:

Test When to write Notes Speed
Unit tests Verify the smallest possible unit works. Networking access is prohibited. Prefer Functional tests given our complexity. Lightning fast (nsec to ms)
Functional tests Guarantee functionality works as expected. It's a subset of integration test covering multiple units. No external dependency. Prefer Fake implementations (in-memory) over Mocks and Stubs. Fast (ms to few seconds at worst)
Integration tests Gain confidence that code works with one or more external dependencies. No need for a Lambda function. Use our code base against an external dependency e.g., fetch an existing SSM parameter. Moderate to slow (a few minutes)
End-to-end tests Gain confidence that a Lambda function with our code operates as expected. It simulates how customers configure, deploy, and run their Lambda function - Event Source configuration, IAM permissions, etc. Slow (minutes)
Performance tests Ensure critical operations won't increase latency and costs to customers. CI arbitrary hardware can make it flaky. We'll resume writing perf test after our new Integ/End have significant coverage. Fast to moderate (a few seconds to a few minutes)

@heitorlessa heitorlessa removed the need-more-information Pending information to continue label Aug 29, 2022
@heitorlessa
Copy link
Contributor Author

@mploski updated the RFC body and took the entire Approach section that now points to the E2E framework. I'd need your approval before we can work on Parameters as a first set of integ test.

Looking at the docs, we'd highly likely only have Parameters, and maybe Feature Flags (uses Parameters Stores either way), since everything else is well covered in functional and E2E tests.

@heitorlessa
Copy link
Contributor Author

Suggestion: Creation/Tearing down of resources should be configurable for local tests. I can see how annoying it can be to have to run everything for a small changes that the developer knows don't have dependency on existing data (i.e. when running only specific tests in debugging a particular problem)

I fixed that as part of the E2E refactoring work. Stacks are auto-deleted upon failures, you can use step through debugging without worrying about stack deletion, and you can also run the entire E2E suite in parallel locally w/o the need for CI.

@mploski
Copy link
Contributor

mploski commented Aug 29, 2022

@mploski updated the RFC body and took the entire Approach section that now points to the E2E framework. I'd need your approval before we can work on Parameters as a first set of integ test.

Looking at the docs, we'd highly likely only have Parameters, and maybe Feature Flags (uses Parameters Stores either way), since everything else is well covered in functional and E2E tests.

Thank you @heitorlessa. Whole doc looks great, no more remarks from my side :-)

@leandrodamascena
Copy link
Contributor

Hi everyone!

We have the end2end tests running and covering various utilities that we have, which gives the project a sense of security that things are working as expected. Adding these integration tests in place would add more complexity to the project at this point.

I am closing this issue. If there is a need in the future we can reopen it and continue the discussion/implementation.

Thank you all for contributing with this RFC.

@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants