feat(toolkit): add typed return to bootstrap action #249

iankhou · 2025-03-18T14:46:22Z

Fixes #192

Description

The bootstrap action in the Toolkit Library currently does not have a returned value. Although there is logged output, the Toolkit instance does not get any information about the result of the bootstrap action.

Solution

Make Toolkit.bootstrap() Return an instance of type Promise<BootstrapResult>, which also uses new type EnvironmentBootstrapResult with metrics for each environment.

Testing

Added unit tests.

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

codecov-commenter · 2025-03-18T14:55:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.40%. Comparing base (d8eeea8) to head (51dea87).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #249      +/-   ##
==========================================
+ Coverage   85.26%   85.40%   +0.14%     
==========================================
  Files         220      220              
  Lines       36619    36619              
  Branches     4448     4461      +13     
==========================================
+ Hits        31222    31275      +53     
+ Misses       5297     5244      -53     
  Partials      100      100

Flag	Coverage Δ
suite.unit	`85.40% <ø> (+0.14%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: github-actions <[email protected]>

mrgrain · 2025-03-18T18:06:54Z

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

+          environment: environment.name,
+          status: bootstrapResult.noOp ? 'no-op' : 'success',
+          message: bootstrapResult.noOp ? 'No changes required' : 'Successfully bootstrapped',
+          duration: Date.now() - envStartTime,


duration can be access from the return value of await bootstrapSpan.end();

I see, I've removed this field.

We can have it, let's just don't time twice and instead do

const envTime = await bootstrapSpan.end(); // ... { duration: envTime.asMs }

Sorry, I had misunderstood what you meant, I'll use the time from bootstrapSpan.

mrgrain · 2025-03-18T18:07:29Z

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

+        const result: EnvironmentBootstrapResult = {
+          environment: environment.name,
+          status: 'failed',
+          error: e,
+          message: formatErrorMessage(e),
+          duration: Date.now() - envStartTime,
+        };


we throw an exception three lines further down, so this will just never be returned.

Got it, removed this code. I also removed 'failed' as a status case because it throws an error.

mrgrain · 2025-03-18T18:08:30Z

packages/@aws-cdk/toolkit-lib/lib/actions/bootstrap/index.ts

+  environment: string;
+  status: 'success' | 'failed' | 'no-op';
+  message?: string;
+  error?: Error;


See other comment, this will just never be returned.

It's still there

mrgrain

Looks good! Let's do a little less and re-use what's already there.

packages/@aws-cdk/toolkit-lib/lib/actions/bootstrap/index.ts

mrgrain · 2025-03-18T18:10:03Z

packages/@aws-cdk/toolkit-lib/lib/actions/bootstrap/index.ts

 }

+export interface EnvironmentBootstrapResult {
+  environment: string;


Would this be better types as ?

Suggested change

environment: string;

environment: cxapi.Environment;

Replaced and updated tests as well.

mrgrain · 2025-03-18T18:11:18Z

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

+
+    return {
+      environments: results,
+      summary,


I think let's just loose the summary. If we return, it's successful. If someone really cares about the counts for success vs. no-op, they can do that themselves.

Yup, makes sense. Removed the summary.

…d of string environment

mrgrain · 2025-03-20T11:30:44Z

packages/@aws-cdk/toolkit-lib/lib/actions/bootstrap/index.ts

+export interface EnvironmentBootstrapResult {
+  environment: cxapi.Environment;
+  status: 'success' | 'no-op';
+  error?: Error;


Suggested change

error?: Error;

Removed this, it will never be used.

mrgrain · 2025-03-20T15:12:13Z

packages/@aws-cdk/toolkit-lib/lib/actions/bootstrap/index.ts

+export interface EnvironmentBootstrapResult {
+  environment: cxapi.Environment;
+  status: 'success' | 'no-op';
+  duration: Number;


Suggested change

duration: Number;

duration: number;

mrgrain · 2025-03-20T15:13:08Z

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

          },
        );
+
+        const envTime = await bootstrapSpan.end();


you now have bootstrapSpan.end(); twice, causing the message to be printed twice. your reporting block needs to move to where the original end message was.

fix(toolkit): add typed return to bootstrap action

e3d52fd

iankhou self-assigned this Mar 18, 2025

iankhou had a problem deploying to integ-approval March 18, 2025 14:46 — with GitHub Actions Failure

github-actions bot added the p2 label Mar 18, 2025

aws-cdk-automation requested a review from a team March 18, 2025 14:46

aws-cdk-automation enabled auto-merge March 18, 2025 14:46

don't use jasmine global method fail()

b400257

iankhou had a problem deploying to integ-approval March 18, 2025 17:08 — with GitHub Actions Failure

chore: self mutation

b7d6cd3

Signed-off-by: github-actions <[email protected]>

aws-cdk-automation temporarily deployed to integ-approval March 18, 2025 17:25 — with GitHub Actions Inactive

iankhou changed the title ~~fix(toolkit): add typed return to bootstrap action~~ feat(toolkit): add typed return to bootstrap action Mar 18, 2025

mrgrain reviewed Mar 18, 2025

View reviewed changes

mrgrain requested changes Mar 18, 2025

View reviewed changes

aws-cdk-automation temporarily deployed to run-tests March 18, 2025 18:14 — with GitHub Actions Inactive

aws-cdk-automation temporarily deployed to run-tests March 18, 2025 18:15 — with GitHub Actions Inactive

remove summary, remove unreachable code, use cxapi.Environment instea…

998d8bd

…d of string environment

iankhou had a problem deploying to integ-approval March 18, 2025 20:28 — with GitHub Actions Failure

remove merge markers

34dc3d1

iankhou had a problem deploying to integ-approval March 18, 2025 21:50 — with GitHub Actions Failure

mrgrain reviewed Mar 20, 2025

View reviewed changes

remove optional error field for EnvironmentBootstrapResult

ce897d3

iankhou temporarily deployed to integ-approval March 20, 2025 13:59 — with GitHub Actions Inactive

add back duration for bootstrap environment result

505f4b4

iankhou had a problem deploying to integ-approval March 20, 2025 14:41 — with GitHub Actions Failure

lint

0fe9fa6

iankhou temporarily deployed to integ-approval March 20, 2025 14:42 — with GitHub Actions Inactive

iankhou had a problem deploying to run-tests March 20, 2025 15:00 — with GitHub Actions Failure

iankhou temporarily deployed to run-tests March 20, 2025 15:00 — with GitHub Actions Inactive

mrgrain reviewed Mar 20, 2025

View reviewed changes

mrgrain requested changes Mar 20, 2025

View reviewed changes

remove extra bootstrapSpan.end() call

4328e88

iankhou had a problem deploying to integ-approval March 20, 2025 15:51 — with GitHub Actions Failure

use number instead of Number

07bd5ed

iankhou had a problem deploying to integ-approval March 20, 2025 15:52 — with GitHub Actions Failure

mrgrain approved these changes Mar 20, 2025

View reviewed changes

mrgrain added the pr/exempt-integ-test Skips the integ test steps if set. label Mar 20, 2025

Merge branch 'main' into iankhou-typed-return-bootstrap

51dea87

aws-cdk-automation added this pull request to the merge queue Mar 20, 2025

Merged via the queue into main with commit b23b2da Mar 20, 2025
11 checks passed

aws-cdk-automation deleted the iankhou-typed-return-bootstrap branch March 20, 2025 18:11

feat(toolkit): add typed return to bootstrap action #249

feat(toolkit): add typed return to bootstrap action #249

Conversation

iankhou commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Testing

Uh oh!

codecov-commenter commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iankhou commented Mar 18, 2025 •

edited

Loading

codecov-commenter commented Mar 18, 2025 •

edited

Loading

mrgrain Mar 19, 2025 •

edited

Loading

mrgrain Mar 18, 2025 •

edited

Loading

mrgrain Mar 20, 2025 •

edited

Loading