-
Notifications
You must be signed in to change notification settings - Fork 30
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
chore: Add integration test for Python stack #307
Conversation
' dotnet tool install -g Amazon.Lambda.Tools' + | ||
' && dotnet build' + | ||
' && dotnet lambda package --output-package /asset-output/function.zip' |
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.
🟠 Code Quality Violation
use f-string or .format to format strings (...read more)
Concatenation of multiple strings is not efficient and make the code hard to read and understand.
Instead of concatenating multiple strings, use an f-string or a format string.
Learn More
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.
Is this file suppose to be empty?
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.
Good catch! 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.
Is this file suppose to be empty?
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.
Good catch! 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.
Oh, tests are failing. Let me normalize DD_TAGS in the json.
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.
Done!
35b6f5e
to
6557fe3
Compare
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.
Does it seem weird to you that these integration_tests/snapshots/test-*
files are committed to git? Seems like they're overwritten by the test runner anyway.
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.
Good point. I'm already working on that. Coming soon in the next PR!
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.
There's still an integration test failure, but other than that this looks great!
6557fe3
to
2f9c536
Compare
What does this PR do?
Add a Python stack to integration tests, so that the test will compare the snapshots generated after vs before, to check if a PR causes unexpected changes on the stack to be deployed.
The stack was copied from
examples/python-stack
. I had to copy it because I was unable to find a way to just reference the existing stack.Motivation
Adding more tests to make it safer to deploy changes.
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply