fix(cli): progress bar overshoots count by 1 for stack updates (aws#1…

…6168)

Currently, the `resourcesTotal` output is one short as it doesn't account for the `UPDATE_COMPLETE` event emitted when updating a stack. This PR increases the `resourcesTotal` variable depending on whether the stack is being updated or created.

Noticed this bug when using the CDK on private projects.

This has had a minor fix previously to address the `CREATE_COMPLETE` event emitted when creating a stack, however this did not address the `UPDATE_COMPLETE` event emitted when updating a stack. This caused updated events to produce the following output:

![image](https://user-images.githubusercontent.com/57939433/130373537-5dfacd3c-df7d-4272-abac-a4cf7c04cc47.png)

To address this issue, I:
- Added `+1` to the `resourcesTotal` prop in `packages/aws-cdk/lib/api/deploy-stack.ts` for the `StackActivityMonitor` class depending on whether the stack being deployed already exists using the `cloudFormationStack.exists` boolean.

I also addressed a spacing issue between the pipe (`|`) and the timestamp, as seen in the image above.

Collaborators:
- @JWK95: Provided code review & valid suggestions

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

CONTRIBUTING.md

@@ -16,9 +16,9 @@ let us know if it's not up-to-date (even better, submit a PR with your  correcti
 - [Getting Started](#getting-started)
 - [Pull Requests](#pull-requests)
   - [Step 1: Find something to work on](#step-1-find-something-to-work-on)
-  - [Step 2: Design (optional)](#step-2-design-optional)
+  - [Step 2: Design (optional)](#step-2-design)
   - [Step 3: Work your Magic](#step-3-work-your-magic)
-  - [Step 4: Pull Request](#step-5-pull-request)
+  - [Step 4: Pull Request](#step-4-pull-request)
   - [Step 5: Merge](#step-5-merge)
 - [Breaking Changes](#breaking-changes)
 - [Documentation](#documentation)

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -340,8 +340,10 @@ async function prepareAndExecuteChangeSet(
     await cfn.executeChangeSet({ StackName: deployName, ChangeSetName: changeSetName, ...disableRollback }).promise();
 
     // eslint-disable-next-line max-len
+    const changeSetLength: number = (changeSetDescription.Changes ?? []).length;
     const monitor = options.quiet ? undefined : StackActivityMonitor.withDefaultPrinter(cfn, deployName, stackArtifact, {
-      resourcesTotal: (changeSetDescription.Changes ?? []).length,
+      // +1 for the extra event emitted from updates.
+      resourcesTotal: cloudFormationStack.exists ? changeSetLength + 1 : changeSetLength,
       progress: options.progress,
       changeSetCreationTime: changeSetDescription.CreationTime,
     }).start();

packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts

@@ -524,7 +524,7 @@ export class HistoryActivityPrinter extends ActivityPrinterBase {
     this.stream.write(
       util.format(
         ' %s%s | %s | %s | %s %s%s%s\n',
-        (progress !== false ? ` ${this.progress()} |` : ''),
+        (progress !== false ? ` ${this.progress()} | ` : ''),
         new Date(e.Timestamp).toLocaleTimeString(),
         color(padRight(STATUS_WIDTH, (e.ResourceStatus || '').substr(0, STATUS_WIDTH))), // pad left and trim
         padRight(this.props.resourceTypeColumnWidth, e.ResourceType || ''),

packages/aws-cdk/test/api/stack-activity-monitor.test.ts

@@ -32,7 +32,7 @@ test('prints 0/4 progress report, when addActivity is called with an "IN_PROGRES
     });
   });
 
-  expect(output[0].trim()).toStrictEqual(`0/4 |${HUMAN_TIME} | ${reset('IN_PROGRESS         ')} | AWS::CloudFormation::Stack | ${reset(bold('stack1'))}`);
+  expect(output[0].trim()).toStrictEqual(`0/4 | ${HUMAN_TIME} | ${reset('IN_PROGRESS         ')} | AWS::CloudFormation::Stack | ${reset(bold('stack1'))}`);
 });
 
 test('prints 1/4 progress report, when addActivity is called with an "UPDATE_COMPLETE" ResourceStatus', () => {
@@ -56,7 +56,7 @@ test('prints 1/4 progress report, when addActivity is called with an "UPDATE_COM
     });
   });
 
-  expect(output[0].trim()).toStrictEqual(`1/4 |${HUMAN_TIME} | ${green('UPDATE_COMPLETE     ')} | AWS::CloudFormation::Stack | ${green(bold('stack1'))}`);
+  expect(output[0].trim()).toStrictEqual(`1/4 | ${HUMAN_TIME} | ${green('UPDATE_COMPLETE     ')} | AWS::CloudFormation::Stack | ${green(bold('stack1'))}`);
 });
 
 test('prints 1/4 progress report, when addActivity is called with an "UPDATE_COMPLETE_CLEAN_IN_PROGRESS" ResourceStatus', () => {
@@ -80,7 +80,7 @@ test('prints 1/4 progress report, when addActivity is called with an "UPDATE_COM
     });
   });
 
-  expect(output[0].trim()).toStrictEqual(`1/4 |${HUMAN_TIME} | ${green('UPDATE_COMPLETE_CLEA')} | AWS::CloudFormation::Stack | ${green(bold('stack1'))}`);
+  expect(output[0].trim()).toStrictEqual(`1/4 | ${HUMAN_TIME} | ${green('UPDATE_COMPLETE_CLEA')} | AWS::CloudFormation::Stack | ${green(bold('stack1'))}`);
 });
 
 
@@ -105,7 +105,7 @@ test('prints 1/4 progress report, when addActivity is called with an "ROLLBACK_C
     });
   });
 
-  expect(output[0].trim()).toStrictEqual(`1/4 |${HUMAN_TIME} | ${yellow('ROLLBACK_COMPLETE_CL')} | AWS::CloudFormation::Stack | ${yellow(bold('stack1'))}`);
+  expect(output[0].trim()).toStrictEqual(`1/4 | ${HUMAN_TIME} | ${yellow('ROLLBACK_COMPLETE_CL')} | AWS::CloudFormation::Stack | ${yellow(bold('stack1'))}`);
 });
 
 test('prints 0/4 progress report, when addActivity is called with an "UPDATE_FAILED" ResourceStatus', () => {
@@ -129,7 +129,7 @@ test('prints 0/4 progress report, when addActivity is called with an "UPDATE_FAI
     });
   });
 
-  expect(output[0].trim()).toStrictEqual(`0/4 |${HUMAN_TIME} | ${red('UPDATE_FAILED       ')} | AWS::CloudFormation::Stack | ${red(bold('stack1'))}`);
+  expect(output[0].trim()).toStrictEqual(`0/4 | ${HUMAN_TIME} | ${red('UPDATE_FAILED       ')} | AWS::CloudFormation::Stack | ${red(bold('stack1'))}`);
 });
 
 
@@ -178,9 +178,9 @@ test('does not print "Failed Resources:" list, when all deployments are successf
   });
 
   expect(output.length).toStrictEqual(3);
-  expect(output[0].trim()).toStrictEqual(`0/2 |${HUMAN_TIME} | ${reset('IN_PROGRESS         ')} | AWS::CloudFormation::Stack | ${reset(bold('stack1'))}`);
-  expect(output[1].trim()).toStrictEqual(`1/2 |${HUMAN_TIME} | ${green('UPDATE_COMPLETE     ')} | AWS::CloudFormation::Stack | ${green(bold('stack1'))}`);
-  expect(output[2].trim()).toStrictEqual(`2/2 |${HUMAN_TIME} | ${green('UPDATE_COMPLETE     ')} | AWS::CloudFormation::Stack | ${green(bold('stack2'))}`);
+  expect(output[0].trim()).toStrictEqual(`0/2 | ${HUMAN_TIME} | ${reset('IN_PROGRESS         ')} | AWS::CloudFormation::Stack | ${reset(bold('stack1'))}`);
+  expect(output[1].trim()).toStrictEqual(`1/2 | ${HUMAN_TIME} | ${green('UPDATE_COMPLETE     ')} | AWS::CloudFormation::Stack | ${green(bold('stack1'))}`);
+  expect(output[2].trim()).toStrictEqual(`2/2 | ${HUMAN_TIME} | ${green('UPDATE_COMPLETE     ')} | AWS::CloudFormation::Stack | ${green(bold('stack2'))}`);
 });
 
 test('prints "Failed Resources:" list, when at least one deployment fails', () => {
@@ -217,8 +217,8 @@ test('prints "Failed Resources:" list, when at least one deployment fails', () =
   });
 
   expect(output.length).toStrictEqual(4);
-  expect(output[0].trim()).toStrictEqual(`0/2 |${HUMAN_TIME} | ${reset('IN_PROGRESS         ')} | AWS::CloudFormation::Stack | ${reset(bold('stack1'))}`);
-  expect(output[1].trim()).toStrictEqual(`0/2 |${HUMAN_TIME} | ${red('UPDATE_FAILED       ')} | AWS::CloudFormation::Stack | ${red(bold('stack1'))}`);
+  expect(output[0].trim()).toStrictEqual(`0/2 | ${HUMAN_TIME} | ${reset('IN_PROGRESS         ')} | AWS::CloudFormation::Stack | ${reset(bold('stack1'))}`);
+  expect(output[1].trim()).toStrictEqual(`0/2 | ${HUMAN_TIME} | ${red('UPDATE_FAILED       ')} | AWS::CloudFormation::Stack | ${red(bold('stack1'))}`);
   expect(output[2].trim()).toStrictEqual('Failed resources:');
   expect(output[3].trim()).toStrictEqual(`${HUMAN_TIME} | ${red('UPDATE_FAILED       ')} | AWS::CloudFormation::Stack | ${red(bold('stack1'))}`);
 });