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

[kaizencc]

The current return type of findResources(), findOutputs(), and findMappings() omits the logicalId. This creates ambiguity because it is not trivial to figure out which resource/output/mapping is being matched. For example,

const result = assert.findOutputs('*', {
  Value: 'Fred',
});

Could return a list like:

expect(result).toEqual([
  { Value: 'Fred', Description: 'FooFred' },
  { Value: 'Fred', Description: 'BarFred' },
]);

This does not provide information on which output(s) are being matched to the Value: 'Fred' property. Under the new return type proposed in this PR, the output would look like this:

expect(result).toEqual({
  Foo: { Value: 'Fred', Description: 'FooFred' },
  Bar: { Value: 'Fred', Description: 'BarFred' },
});

Returning the logicalId in this way for all find*() APIs will provide the full picture back to the user.

BREAKING CHANGE: the findResources() API previously returned a list of resources, but now returns a map of logical id to resource.

• assertions: the findOutputs() API previously returned a list of outputs, but now returns a map of logical id to output.
• assertions: the findMappings() API previously returned a list of mappings, but now returns a map of logical id to mapping.

---

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

[gitpod-io]

[nija-at]

The breaking change entries needs to be fixed -

the findOutputs() API previously returned a list of resources, but now returns a map of logical id to resource

"the findOutputs() API previously returned a list of outputs, but now returns a map of logical id to output"

The other breaking change entry also needs to be fixed.

[kaizencc]

Thanks for the review @nija-at, I'm ready for another pass. Sorry about the continued copy/paste mishaps in the PR description... there should be no more. Build failure has nothing to do with me (I don't think), anything I can do to get it to build correctly?

[nija-at]

Just one comment about return type when no matches.

The rest look good.

[nija-at]

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?

[kaizencc]

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? :)

[nija-at]

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

[nija-at]

This is what I mean.

Would be nicer if we can do -

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

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: cbf3d7b
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).