-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(cli): regression tests #7060
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
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
1 similar comment
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
packages/aws-cdk/test/integ/cli/test-cdk-deploy-nested-stack-with-parameters.sh
Show resolved
Hide resolved
eladb
left a comment
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.
The names of the two new tests needs to be changed. I couldn't wrap my head around what each one of these tests is actually doing, even after reading all the code it. I don't mind if these names are 200 characters long but it should be clear what their purpose is.
PR title/description:
- Please add some details in the commit message about why each type of new test is needed.
- Why is the title
feat? I don't think we need to advertise this in the CHANGELOG. Do you?
packages/aws-cdk/test/integ/cli/test-cdk-deploy-nested-stack-with-parameters.sh
Show resolved
Hide resolved
Thats why I added a link to the RFC, is that not sufficient?
Debatable, might be useful for contributors to know about this. But I can go either way here. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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). |
This PR adds regression testing capabilities to our CLI integration tests.
yarn integ-cli-backwardsfetches the latest published version from github and runs its integration tests against the newest code of the CLI and the framework.yarn integ-cli-frameworkfetches the latest published version from github and runs its integration tests against the newest CLI code and latest published framework.More details here: https://github.com/aws/aws-cdk-rfcs/blob/epolon/compatibility-strategy/text/00110-cli-framework-compatibility-strategy.md#breaking-changes-in-cli
Implementation Notes
We don't currently have support for installing framework packages from NPM when running CLI integ tests.
Locally, we use
run-against-repo, which uses a localnpmwrapper that always symlinks to the repo code.In CI, we use
run-against-dist, which starts up a verdaccio instance that hosts our locally packed packages.To support installing the latest published versions, I introduced the
USE_PUBLISHED_FRAMEWORK_VERSIONenv variable. Locally, this causes thenpmwrapper to simply pass through the command tonpm.In CI, its a bit more complicated. We can't simply pass through to
npmbecause the CLI and all its dependencies still needs to be installed from the local dist packages. I added some hacky code that basically publishes only the necessary packages (CLI and its deps) to verdaccio, leaving everything else to be served by thenpmjsuplink.Commit Message
added two new cli integ tests:
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license