-
Notifications
You must be signed in to change notification settings - Fork 4k
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: remove testLegacyBehavior and update testFutureBehavior tests #21949
Conversation
}, | ||
}, | ||
}); | ||
test('use the TLSv1.2_2021 security policy by default', () => { |
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.
Don't we still need to test the legacy behavior though? For some of the feature flags it is possible to still set the flag to false
.
Not sure if it would be better to keep testLegacyBehavior
or just set the feature flag through node.setContext()
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 are places that the feature flags are set as false to do that very thing. testLegacyBehavior, however, just skips the tests in v2, it doesn't set the flags to false.
Thank you for contributing! Your pull request will be updated from main 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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
testLegacyBehavior only runs tests in v1 and skips them in v2, so none of these tests were running anyway. testFutureBehavior just runs the tests with the feature-flag as true, which is the default to these in v2 anyway. I'm removing these functions and their uses so that contributors aren't mislead to think they should use them.
I also fixed some spacing issues in test packages. The diff on this is kind of hard to follow but the summary of the change is this:
All Submissions:
Adding new Unconventional Dependencies:
New Features
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