refactor(cli): require approval is a part of the CliIoHost#151
refactor(cli): require approval is a part of the CliIoHost#151
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #151 +/- ##
==========================================
+ Coverage 84.84% 85.01% +0.17%
==========================================
Files 207 207
Lines 35761 35816 +55
Branches 4601 4626 +25
==========================================
+ Hits 30340 30449 +109
+ Misses 5269 5217 -52
+ Partials 152 150 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -53,10 +54,9 @@ describe('deploy', () => { | |||
|
|
|||
| test('request response when require approval is set', async () => { | |||
There was a problem hiding this comment.
this test tests that we are getting the approval prompt... do you also want me to test that we can respond to it? or is that self-explanitory
There was a problem hiding this comment.
I think that's enough. Otherwise we're just testing that the TestIoHost works.
I'd add a data prop to the expected message though.
Signed-off-by: github-actions <github-actions@github.com>
|
|
||
| test('request response when require approval is set', async () => { | ||
| // WHEN | ||
| ioHost.requireDeployApproval = RequireApproval.ANY_CHANGE; |
There was a problem hiding this comment.
I don't understand that effect this has on the test?
There was a problem hiding this comment.
this is how we set require approval now. i can add a comment in the code
| @@ -53,10 +54,9 @@ describe('deploy', () => { | |||
|
|
|||
| test('request response when require approval is set', async () => { | |||
There was a problem hiding this comment.
I think that's enough. Otherwise we're just testing that the TestIoHost works.
I'd add a data prop to the expected message though.
CliIoHostnow governs when to ask for approval or not. This was previously passed into deploy via deployOptions.deployOptions.requireApprovalwas deprecated before, and now it is no longer being used (this may have to be pulled into a different change that removes therequireApprovaloption entirely in a minor version bump of toolkit-lib.CliIoHosthas a property,requireApprovalthat can be set. When therequestResponseAPI is called, we check to see if the message code is a knownrequireApprovalcode and if so, see if the request is necessary or if the default can be used.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license