-
Notifications
You must be signed in to change notification settings - Fork 146
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
test(idempotency): improve integration tests for utility #1591
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
automation
This item relates to automation
idempotency
This item relates to the Idempotency Utility
internal
PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
tests
PRs that add or change tests
labels
Jul 8, 2023
pull-request-size
bot
added
the
size/XXL
PRs with 1K+ LOC, largely documentation related
label
Jul 8, 2023
dreamorosi
changed the title
tests(idempotency): improve integration tests for utility
test(idempotency): improve integration tests for utility
Jul 8, 2023
am29d
reviewed
Jul 10, 2023
handler: handler, | ||
environment: { | ||
IDEMPOTENCY_TABLE_NAME: ddbTableName, | ||
POWERTOOLS_LOGGER_LOG_EVENT: 'true', | ||
}, | ||
logRetention: RetentionDays.ONE_DAY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
am29d
approved these changes
Jul 10, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dreamorosi
added a commit
that referenced
this pull request
Jul 11, 2023
* chore: init workspace * chore: init workspace * Initial base class implementation * Added BatchProcessor implementation, attempted fix for async * Added unit tests * Refactoring unit tests * Lint fix, updated docstrings * Added response and identifier typings * test(idempotency): improve integration tests for utility (#1591) * docs: new name * chore: rename e2e files * tests(idempotency): expand integration tests * chore(idempotency): remove unreachable code * Removed unnecessary type casting * Moved exports for handlers and factories * Updated imports, refactored randomization in factories * Refactored EventType to be const instead of enum * Refactored and added documentation for errors * Removed debugging line * chore(ci): add canary to layer deployment (#1593) * docs(idempotency): write utility docs (#1592) * docs: base docs * wip * chore: added paths to snippets tsconfig * chore: added page to docs menu * docs(idempotency): utility docs * highlights * chore: remove CDK mention * build(internal): bump semver from 5.7.1 to 5.7.2 (#1594) Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2. - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md) - [Commits](npm/node-semver@v5.7.1...v5.7.2) --- updated-dependencies: - dependency-name: semver dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(idempotency): mark the utility ready public beta (#1595) * chore(idempotency): mark utility as public beta * chore: manually increment version in commons * docs(internal): update AWS SDK links to new docs (#1597) * chore(maintenance): remove parameters utility from layer bundling and layers e2e tests (#1599) * remove parameter from e2e tests * remove parameters from canary stack as well * chore(release): v1.11.1 [skip ci] * fix canary deploy in ci with correct workspace name (#1601) * chore: update layer ARN on documentation --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Andrea Amorosi <[email protected]> Co-authored-by: Alexander Schueren <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Release bot[bot] <[email protected]>
dreamorosi
added a commit
that referenced
this pull request
Jul 15, 2023
* chore: init workspace * chore: init workspace * Initial base class implementation * Added BatchProcessor implementation, attempted fix for async * Added unit tests * Refactoring unit tests * Lint fix, updated docstrings * Added response and identifier typings * test(idempotency): improve integration tests for utility (#1591) * docs: new name * chore: rename e2e files * tests(idempotency): expand integration tests * chore(idempotency): remove unreachable code * Removed unnecessary type casting * Moved exports for handlers and factories * Updated imports, refactored randomization in factories * Refactored EventType to be const instead of enum * Refactored and added documentation for errors * Removed debugging line * chore(ci): add canary to layer deployment (#1593) * docs(idempotency): write utility docs (#1592) * docs: base docs * wip * chore: added paths to snippets tsconfig * chore: added page to docs menu * docs(idempotency): utility docs * highlights * chore: remove CDK mention * build(internal): bump semver from 5.7.1 to 5.7.2 (#1594) Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2. - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md) - [Commits](npm/node-semver@v5.7.1...v5.7.2) --- updated-dependencies: - dependency-name: semver dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(idempotency): mark the utility ready public beta (#1595) * chore(idempotency): mark utility as public beta * chore: manually increment version in commons * docs(internal): update AWS SDK links to new docs (#1597) * chore(maintenance): remove parameters utility from layer bundling and layers e2e tests (#1599) * remove parameter from e2e tests * remove parameters from canary stack as well * chore(release): v1.11.1 [skip ci] * fix canary deploy in ci with correct workspace name (#1601) * chore: update layer ARN on documentation --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Andrea Amorosi <[email protected]> Co-authored-by: Alexander Schueren <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Release bot[bot] <[email protected]>
dreamorosi
added a commit
that referenced
this pull request
Jul 15, 2023
* chore: init workspace * chore: init workspace * Initial base class implementation * Added BatchProcessor implementation, attempted fix for async * Added unit tests * Refactoring unit tests * Lint fix, updated docstrings * Added response and identifier typings * test(idempotency): improve integration tests for utility (#1591) * docs: new name * chore: rename e2e files * tests(idempotency): expand integration tests * chore(idempotency): remove unreachable code * Removed unnecessary type casting * Moved exports for handlers and factories * Updated imports, refactored randomization in factories * Refactored EventType to be const instead of enum * Refactored and added documentation for errors * Removed debugging line * chore(ci): add canary to layer deployment (#1593) * docs(idempotency): write utility docs (#1592) * docs: base docs * wip * chore: added paths to snippets tsconfig * chore: added page to docs menu * docs(idempotency): utility docs * highlights * chore: remove CDK mention * build(internal): bump semver from 5.7.1 to 5.7.2 (#1594) Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2. - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md) - [Commits](npm/node-semver@v5.7.1...v5.7.2) --- updated-dependencies: - dependency-name: semver dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(idempotency): mark the utility ready public beta (#1595) * chore(idempotency): mark utility as public beta * chore: manually increment version in commons * docs(internal): update AWS SDK links to new docs (#1597) * chore(maintenance): remove parameters utility from layer bundling and layers e2e tests (#1599) * remove parameter from e2e tests * remove parameters from canary stack as well * chore(release): v1.11.1 [skip ci] * fix canary deploy in ci with correct workspace name (#1601) * chore: update layer ARN on documentation --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Andrea Amorosi <[email protected]> Co-authored-by: Alexander Schueren <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Release bot[bot] <[email protected]>
dreamorosi
added a commit
that referenced
this pull request
Jul 25, 2023
* chore: init workspace * chore: init workspace * feat(batch): Implementation of base batch processing classes (#1588) * chore: init workspace * chore: init workspace * Initial base class implementation * Added BatchProcessor implementation, attempted fix for async * Added unit tests * Refactoring unit tests * Lint fix, updated docstrings * Added response and identifier typings * test(idempotency): improve integration tests for utility (#1591) * docs: new name * chore: rename e2e files * tests(idempotency): expand integration tests * chore(idempotency): remove unreachable code * Removed unnecessary type casting * Moved exports for handlers and factories * Updated imports, refactored randomization in factories * Refactored EventType to be const instead of enum * Refactored and added documentation for errors * Removed debugging line * chore(ci): add canary to layer deployment (#1593) * docs(idempotency): write utility docs (#1592) * docs: base docs * wip * chore: added paths to snippets tsconfig * chore: added page to docs menu * docs(idempotency): utility docs * highlights * chore: remove CDK mention * build(internal): bump semver from 5.7.1 to 5.7.2 (#1594) Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2. - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md) - [Commits](npm/node-semver@v5.7.1...v5.7.2) --- updated-dependencies: - dependency-name: semver dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(idempotency): mark the utility ready public beta (#1595) * chore(idempotency): mark utility as public beta * chore: manually increment version in commons * docs(internal): update AWS SDK links to new docs (#1597) * chore(maintenance): remove parameters utility from layer bundling and layers e2e tests (#1599) * remove parameter from e2e tests * remove parameters from canary stack as well * chore(release): v1.11.1 [skip ci] * fix canary deploy in ci with correct workspace name (#1601) * chore: update layer ARN on documentation --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Andrea Amorosi <[email protected]> Co-authored-by: Alexander Schueren <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Release bot[bot] <[email protected]> * feat(batch): Batch processing wrapper function (#1605) * Refactored some types, added function wrapper and base test * Added record check and tests, renamed factories * Refactored type check logic in function * Refactor test to remove error ignore * feat(batch): Implement SQS FIFO processor class (#1606) * Added SQS FIFO processor and unit tests * Added docstring for pbatch processing function * feat(batch): Support for Lambda context access in batch processing (#1609) * Added types and parameter for lambda context, added unit tests * Refactor parameter checking * Added test for malformed context handling * docs: created utility docs * docs: fixed white spaces * feat(batch): add async processor (#1616) * feat(batch): add async processor * tests: improved unit tests * chore: removed docstring + edited test handler * chore: fix typos * docs: added README * chore: added package to beta release * chore: marked package as public * chore: added new batch page to docs * chore: added utility to lerna workspace * chore: added utility to main readme * chore: added utility to CI --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Erika Yao <[email protected]> Co-authored-by: Alexander Schueren <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Release bot[bot] <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
automation
This item relates to automation
idempotency
This item relates to the Idempotency Utility
internal
PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
size/XXL
PRs with 1K+ LOC, largely documentation related
tests
PRs that add or change tests
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of your changes
This PR introduces new integration tests for the idempotency utility, and improves the existing ones by expanding the test coverage and assertions.
The integration tests now include both function wrapper and Middy middleware, and across both test suites I have improved the test coverage by including tests for concurrent requests, Lambda timeouts, expired idempotency records, and different combinations of payload selection (
dataIndexArgument
) and filtering (JMESPath expressions).Fixes for
makeHandlerIdempotent
middlewareWhile working on the unit tests for the Middy middleware I found out that due to the way the middleware was written, if customers configured more than one Lambda handler in the same file, some of the Idempotency settings would leak between handlers, for example, having a single file
index.ts
with two handlers like below:Could result in the
makeHandlerIdempotent
overriding and sharing thepersistenceStore
andconfigs
. Thanks to the integration tests we were able to catch this bug before releasing the utility.The issue described above was fixed by moving the setup of these two objects in the
before
stage, and then storing references inside the Middy internal storage. In this way the objects are scoped to a specific request and can be accessed across stages of the same request.Other changes
The PR also:
makeIdempotent
) that was changed in a previous PR.run-e2e-tests
workflow so that the tests for this utility are run together with the others.Related issues, RFCs
Issue number: closes #1555
Checklist
Breaking change checklist
Is it a breaking change?: NO
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.