From 77aede2502fb2ce5dbc559a96e0e6d2d2b7419a0 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 13 Sep 2023 14:48:59 +0900 Subject: [PATCH 1/3] chore(cloudwatch): add validation for start and end in dashboard --- .../aws-cloudwatch/lib/dashboard.ts | 6 +++- .../aws-cloudwatch/test/dashboard.test.ts | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts index bf11d4b055d55..594870c8d7d4c 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts @@ -133,7 +133,11 @@ export class Dashboard extends Resource { } if (props.start !== undefined && props.defaultInterval !== undefined) { - throw ('both properties defaultInterval and start cannot be set at once'); + throw new Error('both properties defaultInterval and start cannot be set at once'); + } + + if (props.end !== undefined && props.start === undefined) { + throw new Error('You must specify a start if you specify an end'); } const dashboard = new CfnDashboard(this, 'Resource', { diff --git a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts index 9c278ea600b37..b1c69fcd0e354 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts @@ -161,6 +161,35 @@ describe('Dashboard', () => { }); + test('throws if both defaultInterval and start are specified', () => { + // GIVEN + const stack = new Stack(); + // WHEN + const toThrow = () => { + new Dashboard(stack, 'Dash', { + start: '-P7D', + defaultInterval: Duration.days(7), + }); + }; + + // THEN + expect(() => toThrow()).toThrow(/both properties defaultInterval and start cannot be set at once/); + }); + + test('throws if end is specified but start is not', () => { + // GIVEN + const stack = new Stack(); + // WHEN + const toThrow = () => { + new Dashboard(stack, 'Dash', { + end: '2018-12-17T06:00:00.000Z', + }); + }; + + // THEN + expect(() => toThrow()).toThrow(/You must specify a start if you specify an end/); + }); + test('DashboardName is set when provided', () => { // GIVEN const app = new App(); From 56d823884588552417c7d63f8df165a4106a3c12 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 13 Sep 2023 15:01:46 +0900 Subject: [PATCH 2/3] change error messages --- packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts | 2 +- packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts index 594870c8d7d4c..e442ca423fef2 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts @@ -137,7 +137,7 @@ export class Dashboard extends Resource { } if (props.end !== undefined && props.start === undefined) { - throw new Error('You must specify a start if you specify an end'); + throw new Error('You must also specify a start if you specify an end'); } const dashboard = new CfnDashboard(this, 'Resource', { diff --git a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts index b1c69fcd0e354..a91d81746fa44 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts @@ -187,7 +187,7 @@ describe('Dashboard', () => { }; // THEN - expect(() => toThrow()).toThrow(/You must specify a start if you specify an end/); + expect(() => toThrow()).toThrow(/You must also specify a start if you specify an end/); }); test('DashboardName is set when provided', () => { From d3d0b2bb6413073e76a10837eecadbbc633680cf Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Thu, 14 Sep 2023 00:33:27 +0900 Subject: [PATCH 3/3] change error messages for the review --- packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts | 2 +- packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts index e442ca423fef2..632580d6be9df 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts @@ -137,7 +137,7 @@ export class Dashboard extends Resource { } if (props.end !== undefined && props.start === undefined) { - throw new Error('You must also specify a start if you specify an end'); + throw new Error('If you specify a value for end, you must also specify a value for start.'); } const dashboard = new CfnDashboard(this, 'Resource', { diff --git a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts index a91d81746fa44..e2798347b1c96 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts @@ -187,7 +187,7 @@ describe('Dashboard', () => { }; // THEN - expect(() => toThrow()).toThrow(/You must also specify a start if you specify an end/); + expect(() => toThrow()).toThrow(/If you specify a value for end, you must also specify a value for start./); }); test('DashboardName is set when provided', () => {