Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(assertions): findXxx() APIs now includes the logical id as part of its result #16454

Merged
merged 9 commits into from
Sep 16, 2021
Merged

feat(assertions): findXxx() APIs now includes the logical id as part of its result #16454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
db42c11
return matches as dictionary with logicalId key
kaizencc Sep 9, 2021
0d00bf4
linter
kaizencc Sep 9, 2021
208c842
make readme changes
kaizencc Sep 10, 2021
9d61d44
fix cloudwatch findresource tests
kaizencc Sep 10, 2021
975c8cf
fix apigwv2 findresource test
kaizencc Sep 10, 2021
1b8df57
incorporate review comments
kaizencc Sep 14, 2021
4a78761
Merge branch 'master' into assert-find
kaizencc Sep 14, 2021
0f409cb
Merge branch 'master' into assert-find
kaizencc Sep 15, 2021
cbf3d7b
Merge branch 'master' into assert-find
mergify[bot] Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion packages/@aws-cdk/assertions/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,17 @@ assert.hasOutput('*', {
});
```

`findOutputs()` will return a list of outputs that match the `logicalId` and `props`,
`findOutputs()` will return a set of outputs that match the `logicalId` and `props`,
and you can use the `'*'` special case as well.

```ts
nija-at marked this conversation as resolved.
Show resolved Hide resolved
const result = assert.findOutputs('*', {
Value: 'Fred',
});
expect(result.Foo).toEqual({ Value: 'Fred', Description: 'FooFred' });
expect(result.Bar).toEqual({ Value: 'Fred', Description: 'BarFred' });
```

The APIs `hasMapping()` and `findMappings()` provide similar functionalities.

## Special Matchers
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assertions/lib/private/mappings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { StackInspector } from '../vendored/assert';
import { filterLogicalId, formatFailure, matchSection } from './section';

export function findMappings(inspector: StackInspector, logicalId: string, props: any = {}): { [key: string]: any }[] {
export function findMappings(inspector: StackInspector, logicalId: string, props: any = {}): { [key: string]: { [key: string]: any } } {
const section: { [key: string] : {} } = inspector.value.Mappings;
const result = matchSection(filterLogicalId(section, logicalId), props);

if (!result.match) {
return [];
return {};
}

return result.matches;
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assertions/lib/private/outputs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { StackInspector } from '../vendored/assert';
import { filterLogicalId, formatFailure, matchSection } from './section';

export function findOutputs(inspector: StackInspector, logicalId: string, props: any = {}): { [key: string]: any }[] {
export function findOutputs(inspector: StackInspector, logicalId: string, props: any = {}): { [key: string]: { [key: string]: any } } {
const section: { [key: string] : {} } = inspector.value.Outputs;
const result = matchSection(filterLogicalId(section, logicalId), props);

if (!result.match) {
return [];
return {};
}

return result.matches;
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assertions/lib/private/resources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ type Resource = {
Type: string;
}

export function findResources(inspector: StackInspector, type: string, props: any = {}): { [key: string]: any }[] {
export function findResources(inspector: StackInspector, type: string, props: any = {}): { [key: string]: { [key: string]: any } } {
const section: { [key: string] : Resource } = inspector.value.Resources;
const result = matchSection(filterType(section, type), props);

if (!result.match) {
return [];
return {};
}

return result.matches;
Expand Down
15 changes: 7 additions & 8 deletions packages/@aws-cdk/assertions/lib/private/section.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { Match } from '../match';
import { Matcher, MatchResult } from '../matcher';

export type MatchSuccess = { match: true, matches: any[] };
export type MatchSuccess = { match: true, matches: {[key: string]: any} };
export type MatchFailure = { match: false, closestResult?: MatchResult, analyzedCount: number };

export function matchSection(section: any, props: any): MatchSuccess | MatchFailure {
const matcher = Matcher.isMatcher(props) ? props : Match.objectLike(props);
let closestResult: MatchResult | undefined = undefined;
let matching: any[] = [];
let matching: {[key: string]: any} = {};
let count = 0;

eachEntryInSection(
section,

(entry) => {
(logicalId, entry) => {
const result = matcher.test(entry);
if (!result.hasFailed()) {
matching.push(entry);
matching[logicalId] = entry;
} else {
count++;
if (closestResult === undefined || closestResult.failCount > result.failCount) {
Expand All @@ -25,8 +25,7 @@ export function matchSection(section: any, props: any): MatchSuccess | MatchFail
}
},
);

if (matching.length > 0) {
if (Object.keys(matching).length > 0) {
return { match: true, matches: matching };
} else {
return { match: false, closestResult, analyzedCount: count };
Expand All @@ -35,11 +34,11 @@ export function matchSection(section: any, props: any): MatchSuccess | MatchFail

function eachEntryInSection(
section: any,
cb: (entry: {[key: string]: any}) => void): void {
cb: (logicalId: string, entry: {[key: string]: any}) => void): void {

for (const logicalId of Object.keys(section ?? {})) {
const resource: { [key: string]: any } = section[logicalId];
cb(resource);
cb(logicalId, resource);
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/assertions/lib/template.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class Template {
* When a literal is provided, performs a partial match via `Match.objectLike()`.
* Use the `Match` APIs to configure a different behaviour.
*/
public findResources(type: string, props: any = {}): { [key: string]: any }[] {
public findResources(type: string, props: any = {}): { [key: string]: { [key: string]: any } } {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, I missed this in the last revision. We should be returning undefined here when there are no matches, not an empty object.

That would make the user experience better. Users can do -

const result = template.find...(...);
if (result !== undefined) {
  // do something
}

With what we have today, it'll be

const result = template.find...(...);
if (Object.keys(result).length > 0) {
   // do something
}

WDYT?

Copy link
Contributor Author

@kaizencc kaizencc Sep 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this in principle but it will mean that since result can be undefined, something like expect(result.Foo).toEqual({}) becomes expect(result!.Foo).toEqual({}). I could be wrong but forcing users to use the ! escape or otherwise dealing with the possibility of undefined might not make the user experience better.

WDYT, @nija-at? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's go with what you have now. It might actually be the better way.

return findResources(this.inspector, type, props);
}

Expand All @@ -126,7 +126,7 @@ export class Template {
* When a literal object is provided, performs a partial match via `Match.objectLike()`.
* Use the `Match` APIs to configure a different behaviour.
*/
public findOutputs(logicalId: string, props: any = {}): { [key: string]: any }[] {
public findOutputs(logicalId: string, props: any = {}): { [key: string]: { [key: string]: any } } {
return findOutputs(this.inspector, logicalId, props);
}

Expand All @@ -151,7 +151,7 @@ export class Template {
* When a literal object is provided, performs a partial match via `Match.objectLike()`.
* Use the `Match` APIs to configure a different behaviour.
*/
public findMappings(logicalId: string, props: any = {}): { [key: string]: any }[] {
public findMappings(logicalId: string, props: any = {}): { [key: string]: { [key: string]: any } } {
return findMappings(this.inspector, logicalId, props);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/assertions/test/private/section.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ describe('section', () => {
// THEN
expect(result.match).toEqual(true);
const success = result as MatchSuccess;
expect(success.matches.length).toEqual(2);
expect(Object.keys(success.matches).length).toEqual(2);
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
expect(success.matches.Entry1).toEqual({ foo: 'bar' });
expect(success.matches.Entry2).toEqual({ foo: 'bar', baz: 'qux' });
});

test('failure', () => {
Expand Down
62 changes: 34 additions & 28 deletions packages/@aws-cdk/assertions/test/template.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,12 @@ describe('Template', () => {
});

const inspect = Template.fromStack(stack);
expect(inspect.findResources('Foo::Bar')).toEqual([{
Type: 'Foo::Bar',
Properties: { baz: 'qux', fred: 'waldo' },
}]);
expect(inspect.findResources('Foo::Bar')).toEqual({
Foo: {
Type: 'Foo::Bar',
Properties: { baz: 'qux', fred: 'waldo' },
},
});
});

test('no matching resource type', () => {
Expand All @@ -292,7 +294,7 @@ describe('Template', () => {
});

const inspect = Template.fromStack(stack);
expect(inspect.findResources('Foo::Baz')).toEqual([]);
expect(inspect.findResources('Foo::Baz')).toEqual({});
});

test('matching resource props', () => {
Expand All @@ -303,9 +305,9 @@ describe('Template', () => {
});

const inspect = Template.fromStack(stack);
expect(inspect.findResources('Foo::Bar', {
expect(Object.keys(inspect.findResources('Foo::Bar', {
Properties: { baz: 'qux' },
}).length).toEqual(1);
})).length).toEqual(1);
});

test('no matching resource props', () => {
Expand All @@ -318,7 +320,7 @@ describe('Template', () => {
const inspect = Template.fromStack(stack);
expect(inspect.findResources('Foo::Bar', {
Properties: { baz: 'waldo' },
})).toEqual([]);
})).toEqual({});
});

test('multiple matching resources', () => {
Expand All @@ -327,7 +329,10 @@ describe('Template', () => {
new CfnResource(stack, 'Bar', { type: 'Foo::Bar' });

const inspect = Template.fromStack(stack);
expect(inspect.findResources('Foo::Bar').length).toEqual(2);
const result = inspect.findResources('Foo::Bar');
expect(Object.keys(result).length).toEqual(2);
expect(result.Foo).toEqual({ Type: 'Foo::Bar' });
expect(result.Bar).toEqual({ Type: 'Foo::Bar' });
});
});

Expand Down Expand Up @@ -433,10 +438,9 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findOutputs('*', { Value: 'Fred' });
expect(result).toEqual([
{ Value: 'Fred', Description: 'FooFred' },
{ Value: 'Fred', Description: 'BarFred' },
]);
expect(Object.keys(result).length).toEqual(2);
expect(result.Foo).toEqual({ Value: 'Fred', Description: 'FooFred' });
expect(result.Bar).toEqual({ Value: 'Fred', Description: 'BarFred' });
});

test('not matching', () => {
Expand All @@ -447,7 +451,7 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findOutputs('*', { Value: 'Waldo' });
expect(result.length).toEqual(0);
expect(Object.keys(result).length).toEqual(0);
});

test('matching specific output', () => {
Expand All @@ -461,9 +465,11 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findOutputs('Foo', { Value: 'Fred' });
expect(result).toEqual([
{ Value: 'Fred' },
]);
expect(result).toEqual({
Foo: {
Value: 'Fred',
},
});
});

test('not matching specific output', () => {
Expand All @@ -477,7 +483,7 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findOutputs('Foo', { Value: 'Waldo' });
expect(result.length).toEqual(0);
expect(Object.keys(result).length).toEqual(0);
});
});

Expand Down Expand Up @@ -592,13 +598,13 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findMappings('*', { Foo: { Bar: 'Lightning' } });
expect(result).toEqual([
{
expect(result).toEqual({
Foo: {
Foo: { Bar: 'Lightning', Fred: 'Waldo' },
Baz: { Bar: 'Qux' },
},
{ Foo: { Bar: 'Lightning' } },
]);
Fred: { Foo: { Bar: 'Lightning' } },
});
});

test('not matching', () => {
Expand All @@ -611,7 +617,7 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findMappings('*', { Foo: { Bar: 'Waldo' } });
expect(result.length).toEqual(0);
expect(Object.keys(result).length).toEqual(0);
});

test('matching with specific outputName', () => {
Expand All @@ -630,15 +636,15 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findMappings('Foo', { Foo: { Bar: 'Lightning' } });
expect(result).toEqual([
{
expect(result).toEqual({
Foo: {
Foo: { Bar: 'Lightning', Fred: 'Waldo' },
Baz: { Bar: 'Qux' },
},
]);
});
});

test('not matching', () => {
test('not matching specific output name', () => {
const stack = new Stack();
new CfnMapping(stack, 'Foo', {
mapping: {
Expand All @@ -654,7 +660,7 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
const result = inspect.findMappings('Fred', { Baz: { Bar: 'Qux' } });
expect(result.length).toEqual(0);
expect(Object.keys(result).length).toEqual(0);
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ describe('HttpApi', () => {
Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::VpcLink', {
Name: 'Link-1',
});
expect(Template.fromStack(stack).findResources('AWS::ApiGatewayV2::VpcLink', {
expect(Object.keys(Template.fromStack(stack).findResources('AWS::ApiGatewayV2::VpcLink', {
Name: 'Link-2',
}).length).toEqual(0);
})).length).toEqual(0);
Comment on lines -270 to +272
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I mean.

Would be nicer if we can do -

expect(Template.fromStack(stack).findResources('AWS::ApiGatewayV2::VpcLink', {
  Name: 'Link-2',
})).toBeUndefined();

});

test('apiEndpoint is exported', () => {
Expand Down
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-cloudwatch/test/dashboard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ describe('Dashboard', () => {

// THEN
const resources = Template.fromStack(stack).findResources('AWS::CloudWatch::Dashboard');
expect(resources.length).toEqual(1);
hasWidgets(resources[0].Properties, [
expect(Object.keys(resources).length).toEqual(1);
const key = Object.keys(resources)[0];
hasWidgets(resources[key].Properties, [
{ type: 'text', width: 10, height: 2, x: 0, y: 0, properties: { markdown: 'first' } },
{ type: 'text', width: 1, height: 4, x: 0, y: 2, properties: { markdown: 'second' } },
{ type: 'text', width: 4, height: 1, x: 0, y: 6, properties: { markdown: 'third' } },
Expand Down Expand Up @@ -63,8 +64,9 @@ describe('Dashboard', () => {

// THEN
const resources = Template.fromStack(stack).findResources('AWS::CloudWatch::Dashboard');
expect(resources.length).toEqual(1);
hasWidgets(resources[0].Properties, [
expect(Object.keys(resources).length).toEqual(1);
const key = Object.keys(resources)[0];
hasWidgets(resources[key].Properties, [
{ type: 'text', width: 10, height: 2, x: 0, y: 0, properties: { markdown: 'first' } },
{ type: 'text', width: 1, height: 4, x: 10, y: 0, properties: { markdown: 'second' } },
{ type: 'text', width: 4, height: 1, x: 11, y: 0, properties: { markdown: 'third' } },
Expand Down