fix(cli): array arguments in cdk.json are ignored #33107
Conversation
github-actions
bot
added
bug
kaizencc
requested a deployment
to
test-pipeline
GitHub Actions
Waiting
kaizencc
changed the title
kaizencc
added
the
pr-linter/exempt-integ-test
| @@ -112,3 +113,33 @@ test('Can specify the `quiet` key in the user config', async () => { | |||
|
|
|||
| expect(config.settings.get(['quiet'])).toBe(true); | |||
| }); | |||
|
|
|||
| test('array settings are not overridden by yarg defaults', async () => { | |||
There was a problem hiding this comment.
@mrgrain is this a reasonable end-to-end test?
There was a problem hiding this comment.
Is cdk ls --plugin [] really what we expect users to write for an empty array?
kaizencc
temporarily deployed
to
test-pipeline
GitHub Actions
Inactive
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33107 +/- ##
==========================================
+ Coverage 80.77% 80.79% +0.01%
==========================================
Files 232 232
Lines 14110 14110
Branches 2453 2453
==========================================
+ Hits 11398 11400 +2
+ Misses 2432 2430 -2
Partials 280 280
Flags with carried forward coverage won't be shown. Click here to find out more.
|
mrgrain
force-pushed
the
conroy/fix-array-args
branch
from
fc4917c to
c5d4908
Compare
|
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
mrgrain
added
pr-linter/cli-integ-tested
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
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). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #32814
Reason for this change
yargs treats arrays weirdly. setting
default: undefinedresults in the default array as['undefined']. So instead, I set the default to bedefault: []. However, this triggers an issue with the order of combining all arguments, and the CLI default was overriding any values incdk.jsonfor array types. So instead, we must omit thedefaultproperty entirely for arrays, in order to achieve the desired behavior ofundefinedas default.Description of changes
Updated code generation to generate NO default property for array types
Description of how you validated changes
tests in
user-input-genand the diff ofparse-command-line-arguments.tsshow that unless already defined, we are omitting defaults for arraysChecklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license