-
Notifications
You must be signed in to change notification settings - Fork 4.3k
chore(codepipeline-actions,rds): migrate tests to jest #13212
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
| }); | ||
|
|
||
| "assigns the Action's Role with read-write permissions to the Bucket if it has both inputs and outputs"(test: Test) { | ||
| testFutureBehavior("assigns the Action's Role with read-write permissions to the Bucket if it has both inputs and outputs", s3GrantWriteCtx, App, (app) => { |
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.
General comment. What's your strategy here with testFutureBehavior on v1? Are we proactively dropping tests for "legacy" behavior already, or supposed to have tests for both behaviors, or some other mix? Feels like by converting this test to testFutureBehavior, we are no longer testing the "legacy" (read: default) behavior.
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.
I don't have a general strategy. It's more applying discretion case-by-case.
In this specific case, the feature flag is changing policy action from s3:PutObject* to s3:PutObject.
This is quite well tested in the s3 module, so in consuming modules (such as the ones in this PR), I'm only testing the future behaviour. Feels redundant to test the legacy behaviour.
|
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). |
|
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 |
Move codepipeline-actions from nodeunit to jest. Most tests
now use
nodeunit-shimbut some tests have been migratedentirely to pure jest.
Move
cluster.test.tsandinstance.test.tsin the rds modulefrom
nodeunit-shimto native jest.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license