refactor(cli): inject CLI version as a parameter to Notices

- Remove direct dependency on versionNumber import in Notices class
- Add cliVersion as a required parameter in NoticesProps interface
- Store cliVersion as a class property and use it in display method
- Improve testability by removing the need to mock version functions

Author: mrgrain

packages/@aws-cdk/tmp-toolkit-helpers/src/util/index.ts

@@ -9,6 +9,7 @@ export * from './format-error';
 export * from './json';
 export * from './objects';
 export * from './parallel';
+export * from './package-info';
 export * from './serialize';
 export * from './string-manipulation';
 export * from './type-brands';

packages/@aws-cdk/tmp-toolkit-helpers/src/util/package-info.ts

@@ -0,0 +1,20 @@
+import * as path from 'path';
+import { bundledPackageRootDir } from './directories';
+
+export function displayVersion() {
+  return `${versionNumber()} (build ${commit()})`;
+}
+
+export function isDeveloperBuild(): boolean {
+  return versionNumber() === '0.0.0';
+}
+
+export function versionNumber(): string {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  return require(path.join(bundledPackageRootDir(__dirname), 'package.json')).version.replace(/\+[0-9a-f]+$/, '');
+}
+
+function commit(): string {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  return require(path.join(bundledPackageRootDir(__dirname), 'build-info.json')).commit;
+}

packages/aws-cdk/lib/cli/cli.ts

@@ -117,6 +117,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
       proxyAddress: configuration.settings.get(['proxy']),
       caBundlePath: configuration.settings.get(['caBundlePath']),
     },
+    cliVersion: version.versionNumber(),
   });
   await notices.refresh();
 

packages/aws-cdk/lib/context-providers/index.ts

@@ -19,7 +19,6 @@ import { TRANSIENT_CONTEXT_KEY } from '../api/context';
 import { replaceEnvPlaceholders } from '../api/environment';
 import { PluginHost } from '../api/plugin';
 import type { ContextProviderPlugin } from '../api/plugin/context-provider-plugin';
-import { CliIoHost } from '../cli/io-host';
 import { formatErrorMessage } from '../util';
 
 type ContextProviderFactory = ((sdk: SdkProvider, io: IContextProviderMessages) => ContextProviderPlugin);
@@ -98,7 +97,6 @@ export async function provideContextValues(
       }
     }
 
-    ioHelper = ioHelper ?? CliIoHost.instance().asIoHelper();
     const provider = factory(sdk, new ContextProviderMessages(ioHelper, providerName));
 
     let value;

packages/aws-cdk/lib/notices.ts

@@ -10,7 +10,6 @@ import { ProxyAgentProvider } from './api/aws-auth';
 import type { Context } from './api/context';
 import type { ConstructTreeNode } from './api/tree';
 import { loadTreeFromDir } from './api/tree';
-import { versionNumber } from './cli/version';
 import { cdkCacheDir, formatErrorMessage } from './util';
 import type { IIoHost } from '../../@aws-cdk/tmp-toolkit-helpers/src/api';
 import { ToolkitError } from '../../@aws-cdk/tmp-toolkit-helpers/src/api';
@@ -48,6 +47,11 @@ export interface NoticesProps {
    * Where messages are going to be sent
    */
   readonly ioHost: IIoHost;
+
+  /**
+   * The version of the CLI
+   */
+  readonly cliVersion: string;
 }
 
 export interface NoticesPrintOptions {
@@ -307,6 +311,7 @@ export class Notices {
   private readonly httpOptions: SdkHttpOptions;
   private readonly ioHelper: IoHelper;
   private readonly ioMessages: IoDefaultMessages;
+  private readonly cliVersion: string;
 
   private data: Set<Notice> = new Set();
 
@@ -321,6 +326,7 @@ export class Notices {
     this.httpOptions = props.httpOptions ?? {};
     this.ioHelper = asIoHelper(props.ioHost, 'notices' as any /* forcing a CliAction to a ToolkitAction */);
     this.ioMessages = new IoDefaultMessages(this.ioHelper);
+    this.cliVersion = props.cliVersion;
   }
 
   /**
@@ -361,7 +367,7 @@ export class Notices {
   public display(options: NoticesPrintOptions = {}) {
     const filteredNotices = new NoticesFilter(this.ioMessages).filter({
       data: Array.from(this.data),
-      cliVersion: versionNumber(),
+      cliVersion: this.cliVersion,
       outDir: this.output,
       bootstrappedEnvironments: Array.from(this.bootstrappedEnvironments.values()),
     });

packages/aws-cdk/test/notices.test.ts

@@ -630,6 +630,8 @@ describe(CachedDataSource, () => {
 });
 
 describe(Notices, () => {
+  const cliVersion = '2.1005.0';
+
   beforeEach(() => {
     // disable caching
     jest.spyOn(CachedDataSource.prototype as any, 'save').mockImplementation((_: any) => Promise.resolve());
@@ -642,7 +644,7 @@ describe(Notices, () => {
 
   describe('addBootstrapVersion', () => {
     test('can add multiple values', async () => {
-      const notices = Notices.create({ context: new Context(), ioHost });
+      const notices = Notices.create({ context: new Context(), ioHost, cliVersion });
       notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } });
       notices.addBootstrappedEnvironment({ bootstrapStackVersion: 11, environment: { account: 'account', region: 'region', name: 'env' } });
 
@@ -656,13 +658,10 @@ describe(Notices, () => {
     });
 
     test('deduplicates', async () => {
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } });
       notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } });
 
-      // mock cli version number
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
       notices.display();
 
       const filter = jest.spyOn(NoticesFilter.prototype, 'filter');
@@ -687,10 +686,7 @@ describe(Notices, () => {
 
   describe('refresh', () => {
     test('deduplicates notices', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] },
       });
@@ -700,10 +696,7 @@ describe(Notices, () => {
     });
 
     test('clears notices if empty', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [] },
       });
@@ -713,7 +706,7 @@ describe(Notices, () => {
     });
 
     test('doesnt throw', async () => {
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion });
       await notices.refresh({
         dataSource: {
           fetch: async () => {
@@ -724,12 +717,9 @@ describe(Notices, () => {
     });
 
     test('filters out acknowledged notices by default', async () => {
-      // within the affected version range of both notices
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0');
-
       const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) });
 
-      const notices = Notices.create({ ioHost, context });
+      const notices = Notices.create({ ioHost, context, cliVersion: '1.126.0'  });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] },
       });
@@ -740,12 +730,9 @@ describe(Notices, () => {
     });
 
     test('preserves acknowledged notices if requested', async () => {
-      // within the affected version range of both notices
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0');
-
       const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) });
 
-      const notices = Notices.create({ ioHost, context, includeAcknowledged: true });
+      const notices = Notices.create({ ioHost, context, includeAcknowledged: true, cliVersion: '1.126.0'  });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] },
       });
@@ -758,10 +745,7 @@ describe(Notices, () => {
 
   describe('display', () => {
     test('notices envelop', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] },
       });
@@ -772,10 +756,7 @@ describe(Notices, () => {
     });
 
     test('deduplicates notices', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context() , cliVersion: '1.0.0'});
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] },
       });
@@ -786,22 +767,19 @@ describe(Notices, () => {
     });
 
     test('nothing when there are no notices', async () => {
-      Notices.create({ ioHost, context: new Context() }).display();
+      Notices.create({ ioHost, context: new Context(), cliVersion }).display();
       // expect a single trace that the tree.json was not found, but nothing else
       expect(ioHost.messages.length).toBe(1);
       ioHost.expectMessage({ level: 'trace', containing: 'Failed to get tree.json file' });
     });
 
     test('total count when show total is true', async () => {
-      Notices.create({ ioHost, context: new Context() }).display({ showTotal: true });
+      Notices.create({ ioHost, context: new Context(), cliVersion }).display({ showTotal: true });
       ioHost.expectMessage({ containing: 'There are 0 unacknowledged notice(s).' });
     });
 
     test('warning', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_WARNING_NOTICE] },
       });
@@ -811,10 +789,7 @@ describe(Notices, () => {
     });
 
     test('error', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_ERROR_NOTICE] },
       });
@@ -824,10 +799,7 @@ describe(Notices, () => {
     });
 
     test('only relevant notices', async () => {
-      // within the affected version range of the notice
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');
-
-      const notices = Notices.create({ ioHost, context: new Context() });
+      const notices = Notices.create({ ioHost, context: new Context(), cliVersion: '1.0.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE] },
       });
@@ -837,12 +809,9 @@ describe(Notices, () => {
     });
 
     test('only unacknowledged notices', async () => {
-      // within the affected version range of both notices
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0');
-
       const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) });
 
-      const notices = Notices.create({ ioHost, context });
+      const notices = Notices.create({ ioHost, context, cliVersion: '1.126.0'  });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] },
       });
@@ -852,11 +821,8 @@ describe(Notices, () => {
     });
 
     test('can include acknowledged notices if requested', async () => {
-      // within the affected version range of both notices
-      jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0');
-
       const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) });
-      const notices = Notices.create({ ioHost, context, includeAcknowledged: true });
+      const notices = Notices.create({ ioHost, context, includeAcknowledged: true, cliVersion: '1.126.0' });
       await notices.refresh({
         dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] },
       });