-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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: ignore asset hashes in integ tests by default #17592
Conversation
Any random change that changes asset hashes is a pain to deal with, and slows us down massively. We have a pragma to ignore hashes, and we just turn that pragma on in most integ tests. Might as well flip the default around then: ignore the hashes UNLESS requested otherwise. Do the hashes help? Theoretically during our latest upgrade of jsii, the upgrade that incorrectly added extra statements to our Lambda's sources that *happen* to also be compiled with jsii, we could have used the hashes to detect this undesirable change. But did we? No, we just updated the hashes and went on with our life, because there are so many hashes to update all the time that we don't think twice about it. Therefore these hashes in shapshots are not a useful mechanism, they only slow us down for no gain.
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Any random change that changes asset hashes is a pain to deal with, and slows us down massively. We have a pragma to ignore hashes, and we just turn that pragma on in most integ tests. Might as well flip the default around then: ignore the hashes UNLESS requested otherwise. Do the hashes help? Theoretically during our latest upgrade of jsii, the upgrade that incorrectly added extra statements to our Lambda's sources that *happen* to also be compiled with jsii, we could have used the hashes to detect this undesirable change. But did we? No, we just updated the hashes and went on with our life, because there are so many hashes to update all the time that we don't think twice about it. Therefore these hashes in shapshots are not a useful mechanism, they only slow us down for no gain. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR remove all `pragma:ignore-assets` pragma. `pragma:ignore-assets` was obsoleted at #17592. fix: #18984 As said in this issue, this pragma has been displayed in the documentation as following: <img width="998" alt="image" src="https://user-images.githubusercontent.com/11013683/187058745-efffaf49-f51c-4a2d-988f-c9c798082aaa.png"> ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR remove all `pragma:ignore-assets` pragma. `pragma:ignore-assets` was obsoleted at aws#17592. fix: aws#18984 As said in this issue, this pragma has been displayed in the documentation as following: <img width="998" alt="image" src="https://user-images.githubusercontent.com/11013683/187058745-efffaf49-f51c-4a2d-988f-c9c798082aaa.png"> ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Any random change that changes asset hashes is a pain to deal with,
and slows us down massively. We have a pragma to ignore hashes,
and we just turn that pragma on in most integ tests.
Might as well flip the default around then: ignore the hashes UNLESS
requested otherwise.
Do the hashes help? Theoretically during our latest upgrade of jsii,
the upgrade that incorrectly added extra statements to our Lambda's
sources that happen to also be compiled with jsii, we could have
used the hashes to detect this undesirable change. But did we? No,
we just updated the hashes and went on with our life, because there
are so many hashes to update all the time that we don't think twice
about it. Therefore these hashes in shapshots are not a useful
mechanism, they only slow us down for no gain.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license