-
Notifications
You must be signed in to change notification settings - Fork 2.4k
chore: Add transform test creation script #2551
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
Conversation
Remove unused import Address Feedback Format file Run sam-translate as a script Use main Address Feedback
7677734
to
e6dfce3
Compare
Co-authored-by: Chris Rehn <1280602+hoffa@users.noreply.github.com>
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.
Co-authored-by: Chris Rehn <1280602+hoffa@users.noreply.github.com>
Co-authored-by: Chris Rehn <1280602+hoffa@users.noreply.github.com>
A general question: If we use the script to create transform tests, we use translated templates to compare with translated templates which will for sure succeed. Are we supposed to write output templates on ourselves so we can make sure it's expected? On the other hand, if it is to test if Sam-T functionality has changed, I think this method makes sense. |
We should always verify the output is as we expect. The tool only makes the step of generating it easier. |
To answer Xia's question, I think this script just makes our live easier. We will still need to manually verify the output to make sure it's what we expect. |
Oh makes sense to me then! Could we add this to development guide too? |
Will add a message at the end to let users to check output manually. |
Issue #, if available
Description of changes
Added a script to allow easier transform tests creation.
Description of how you validated changes
Verified by manual trigger.
Tested with and without
--disable-api-configuration
, work as expected.Tested with
arn:aws...
, work as expected.Not affecting SAM T functionality.