-
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
feat(logger): add e2e tests for logger #529
Conversation
The tests spin up actual infrastructure on AWS, invoke Lambda function, verify that the logs are printed correctly, and destroy the stack afterward.
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 except cdk link
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.
Already looking good!
Just left some comments on the files, plus this one:
- Some files contain a copyright notice while others don't, the rest of the project also doesn't contain any so we should settle on one
Co-authored-by: Florian Chazal <florianchazal@gmail.com>
@flochaz @dreamorosi Thanks for the catches. They have been fixed. |
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.
Just ran the e2e tests here on GitHub and it failed, it seems there are issues with the stack deployment based on the errors.
@dreamorosi Logger uses I am considering changing everything to |
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 !
fyi, 1 check didn't pass (e2e) test. It's a flaky test from metric, not logger. I have rerun and everything is green https://github.com/awslabs/aws-lambda-powertools-typescript/actions/runs/1846184047 |
Description of your changes
Add e2e tests for logger
Currently, we only have e2e tests for metrics and tracer. This PR adds e2e tests similar to the other two modules.
How to verify this change
Run
AWS_PROFILE=<YOUR_PROFILE_NAME> AWS_REGION=eu-central-1 npx jest --group=e2e/logger/
Here's the output:
Related issues, RFCs
#353
PR status
Is this ready for review?: YES
Is it a breaking change?: NO
Checklist
Breaking change checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.