chore(tmp-toolkit-helpers): `formatStackDiff` and `formatSecurityDiff` moved to tmp-toolkit-helpers #278

kaizencc · 2025-03-24T20:35:57Z

moves two functions that are relevant to diff into tmp-toolkit-helpers. this will facilitate the adoption of diff in the programmatic toolkit as it will now be able to reuse these functions.

existing tests pass to ensure no breakage. additional tests have been added to tmp-toolkit-helpers to test formatXxxDiff, which were previously not directly tested.

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

kaizencc · 2025-03-24T20:36:50Z

packages/@aws-cdk/tmp-toolkit-helpers/src/api/io/private/format/format-stack-diff.ts

+  try {
+    // must output the stack name if there are differences, even if quiet
+    if (stackName && (!quiet || !diff.isEmpty)) {
+      stream.write(format('Stack %s\n', chalk.bold(stackName)));


welp, these prints don't work yet and im worried im not doing the right thing here:

format and chalk don't exist in this context and i'm not sure how to replicate them.

import { format } from 'node:util'l; import * as chalk from 'chalk';

But format you can also just replace with string literals:

`Stack ${chalk.bold(stackName)}\n`

i guess a better question to ask was: are we certain we want it in tmp-toolkit-helpers and not imported here like a lot of other heavy duty imports.

but it sounds like your answer also answers that question too

kaizencc · 2025-03-25T19:09:15Z

packages/aws-cdk/lib/api/cloudformation/nested-stack-helpers.ts

-    [nestedStackLogicalId: string]: NestedStackTemplates;
-  };
-}
+export { NestedStackTemplates } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api';


is it reasonable to both export and import NestedStackTemplate here?

yes, it's just going to annoy me when I do the move :D

kaizencc · 2025-03-25T19:10:27Z

packages/aws-cdk/test/cli/cdk-toolkit.test.ts

      await cdkToolkit.deploy({
        selector: { patterns: ['Test-Stack-A-Display-Name'] },
-        requireApproval: RequireApproval.Never,
+        requireApproval: RequireApproval.NEVER,


this is kind of silly but also a breaking change right? what should i have done instead?

why is it breaking?

i suppose its not because the CLI users have no access to this..

kaizencc · 2025-03-25T19:11:05Z

.projenrc.ts

    ],
    deps: [
      cloudAssemblySchema.name,
+      cloudFormationDiff,


elsewhere we are depending on cloudformationDiff.customizeReference({ versionType: 'exact' }), and i'm not sure why. just thought i'd flag

kaizencc · 2025-03-25T19:12:31Z

packages/@aws-cdk/tmp-toolkit-helpers/src/api/diff/diff.ts

+export function formatStackDiff(
+  ioHelper: IoHelper,
+  oldTemplate: any,
+  newTemplate: cxapi.CloudFormationStackArtifact,
+  strict: boolean,
+  context: number,
+  quiet: boolean,
+  stackName?: string,
+  changeSet?: DescribeChangeSetOutput,
+  isImport?: boolean,
+  nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): FormatStackDiffOutput {


i agree that this sucks and should be part of a class with formatSecurityDiff. but i don't wanna do it in this PR, it's already too complicated.

codecov-commenter · 2025-03-25T19:13:56Z

Codecov Report

Attention: Patch coverage is 85.12658% with 47 lines in your changes missing coverage. Please review.

Project coverage is 85.39%. Comparing base (a3b9762) to head (e148e1e).

Files with missing lines	Patch %	Lines
.../@aws-cdk/tmp-toolkit-helpers/src/api/diff/diff.ts	85.47%	43 Missing ⚠️
packages/aws-cdk/lib/cli/cdk-toolkit.ts	73.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #278      +/-   ##
==========================================
+ Coverage   85.22%   85.39%   +0.16%     
==========================================
  Files         221      222       +1     
  Lines       36764    36761       -3     
  Branches     4457     4471      +14     
==========================================
+ Hits        31333    31391      +58     
+ Misses       5331     5273      -58     
+ Partials      100       97       -3

Flag	Coverage Δ
suite.unit	`85.39% <85.12%> (+0.16%)`	⬆️

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]>

kaizencc · 2025-03-25T21:58:08Z

packages/@aws-cdk/tmp-toolkit-helpers/test/api/diff/diff.test.ts

AI disclosure: this file was originally intended to be full AI generated and started off as such but it insisted on mocking @aws-cdk/cloudformation-diff functions and I really didn't want it to. So I spent twice as much time bashing the code into what I wanted. At the end of the day this is <50% AI generated.

There are much easier ways to achieve the same mocking results you were after. Check some other files for inspirations.

Specifically all calls end up in an IoHost, so all the helpers could just stay real.

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

mrgrain · 2025-03-25T23:13:32Z

packages/aws-cdk/lib/commands/diff.ts

-    });
-  }
-}
+export { formatStackDiff, formatSecurityDiff, RequireApproval } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api';


That's fine. But you also could have updated any imports.

wip

69711eb

kaizencc had a problem deploying to integ-approval March 24, 2025 20:36 — with GitHub Actions Failure

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

kaizencc commented Mar 24, 2025

View reviewed changes

aws-cdk-automation requested a review from a team March 24, 2025 20:37

kaizencc changed the title ~~chore(tmp-toolkit-helpers): formatStackDiff and formatSecurityDiff moved to tmp-toolkit-helpesr~~ chore(tmp-toolkit-helpers): formatStackDiff and formatSecurityDiff moved to tmp-toolkit-helpers Mar 24, 2025

kaizencc added 2 commits March 25, 2025 14:10

builds now

98394d5

refactor complete

fd47e70

kaizencc had a problem deploying to integ-approval March 25, 2025 19:07 — with GitHub Actions Failure

kaizencc commented Mar 25, 2025

View reviewed changes

chore: self mutation

ebf4338

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

aws-cdk-automation had a problem deploying to integ-approval March 25, 2025 19:17 — with GitHub Actions Failure

api generated tests

8b60fef

kaizencc had a problem deploying to integ-approval March 25, 2025 20:59 — with GitHub Actions Failure

kaizencc marked this pull request as ready for review March 25, 2025 20:59

Merge branch 'main' into conroy/diffs-into-tmp-toolkit-helpers

a006699

kaizencc had a problem deploying to integ-approval March 25, 2025 20:59 — with GitHub Actions Failure

aws-cdk-automation enabled auto-merge March 25, 2025 21:00

chore: self mutation

12cb879

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

aws-cdk-automation had a problem deploying to integ-approval March 25, 2025 21:09 — with GitHub Actions Failure

finally

dfa15e8

kaizencc had a problem deploying to integ-approval March 25, 2025 21:56 — with GitHub Actions Failure

kaizencc commented Mar 25, 2025

View reviewed changes

kaizencc requested a review from mrgrain March 25, 2025 21:59

chore: self mutation

e148e1e

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

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

aws-cdk-automation had a problem deploying to run-tests March 25, 2025 22:30 — with GitHub Actions Failure

aws-cdk-automation temporarily deployed to run-tests March 25, 2025 22:30 — with GitHub Actions Inactive

mrgrain approved these changes Mar 25, 2025

View reviewed changes

aws-cdk-automation temporarily deployed to run-tests March 26, 2025 09:29 — with GitHub Actions Inactive

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

Merged via the queue into main with commit 0db3dc2 Mar 26, 2025
20 checks passed

aws-cdk-automation deleted the conroy/diffs-into-tmp-toolkit-helpers branch March 26, 2025 10:34

chore(tmp-toolkit-helpers): formatStackDiff and formatSecurityDiff moved to tmp-toolkit-helpers #278

chore(tmp-toolkit-helpers): formatStackDiff and formatSecurityDiff moved to tmp-toolkit-helpers #278

Uh oh!

Conversation

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

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!

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!

codecov-commenter commented Mar 25, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chore(tmp-toolkit-helpers): `formatStackDiff` and `formatSecurityDiff` moved to tmp-toolkit-helpers #278

chore(tmp-toolkit-helpers): `formatStackDiff` and `formatSecurityDiff` moved to tmp-toolkit-helpers #278

kaizencc commented Mar 24, 2025 •

edited

Loading

codecov-commenter commented Mar 25, 2025 •

edited

Loading