test(cli): ensure that all commands have explicit bundling configurations

[iankhou]

Issue #31999

Reason for this change

BUNDLING_COMMANDS is a list used to specify CLI commands that require code to be bundled. The CDK Import feature was not added to this list, but requires bundling for certain constructs (like NodeJSFunction lambda).

Description of changes

Removed list BUNDLING_COMMANDS.

Added interface ConfiguredCommand, which requires bundling to be set to a boolean value.

Added type CommandType, which ensures that each Command enum member has a corresponding key in ConfiguredCommands.

Describe any new or updated permissions being added

No permissions changes.

Description of how you validated changes

This is a refactor. The compiler validated the accuracy of the changes.

Integration test run.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[aws-cdk-automation]

(This review is outdated)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.92%. Comparing base (5160796) to head (cabbb70).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #33342   +/-   ##
=======================================
  Coverage   80.92%   80.92%           
=======================================
  Files         236      236           
  Lines       14253    14254    +1     
  Branches     2490     2490           
=======================================
+ Hits        11534    11535    +1     
  Misses       2434     2434           
  Partials      285      285           

Flag	Coverage Δ
suite.unit	80.92% <100.00%> (+<0.01%)	⬆️

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

Components	Coverage Δ
packages/aws-cdk	79.73% <100.00%> (+<0.01%)	⬆️
packages/aws-cdk-lib/core	82.20% <ø> (ø)

[iliapolo]

I like the idea but i'm not a fan of exporting just for the sake of a unit test. It makes the unit test somewhat brittle because they rely heavily on implementation, and not on behavior. A better option would be to enforce each command to specify whether it is bundling or not, during command creation. So for example:

export interface Command {
  bundling: boolean;
  name: string;
}

export const Commands: Command[] = [
  { name: 'ls', bundling: false },
  ....
]

const bundlingCommands = Commands.filter(c => c.bundling);

That way we don't really need a test for it, because its enforced by the compiler.

This also opens the door for additional command specific configuration we might need in the future.
I recognize this would be a larger change, and maybe its worth syncing with @mrgrain as part of toolkit refactoring. Let me know what you think.

[mrgrain]

+1 to @iliapolo suggestion.

This will have to work very differently for the Toolkit, because bundling is something that controls CloudAssembly behavior. In the CLI we have a strict 1:1 mapping between CloudAssembly and CLI command. However in the toolkit, one can use the same CloudAssembly for different Toolkit actions. Not quite sure how to go about this yet. 🤔

[aws-cdk-automation]

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: e7004e7
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository