Skip to content

Commit

Permalink
chore(assertions): replace absentProperty() with absent() and sup…
Browse files Browse the repository at this point in the history
…port it as a `Matcher` type (#16653)

Currently `Match.absentProperty()` is not of type `Matcher` and this introduces a few flaws. For example, `Matcher.isMatcher(Match.absentProperty())` is `false`.

This PR fixes this issue by replacing `Match.absentProperty()` with `Match.absent()` that is of type `Matcher`. The decision to change the name is due to supporting this as a general matcher and not just for Properties.

BREAKING CHANGE: `Match.absentProperty()` becomes `Match.absent()`, and its type changes from `string` to `Matcher`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Oct 5, 2021
1 parent 0343e58 commit c980185
Show file tree
Hide file tree
Showing 17 changed files with 167 additions and 102 deletions.
10 changes: 5 additions & 5 deletions packages/@aws-cdk/assertions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ match.

### Presence and Absence

The `Match.absentProperty()` matcher can be used to specify that a specific
property should not exist on the target. This can be used within `Match.objectLike()`
The `Match.absent()` matcher can be used to specify that a specific
value should not exist on the target. This can be used within `Match.objectLike()`
or outside of any matchers.

```ts
Expand All @@ -216,15 +216,15 @@ or outside of any matchers.
// The following will NOT throw an assertion error
assert.hasResourceProperties('Foo::Bar', {
Fred: Match.objectLike({
Bob: Match.absentProperty(),
Bob: Match.absent(),
}),
});

// The following will throw an assertion error
assert.hasResourceProperties('Foo::Bar', {
Fred: Match.objectLike({
Wobble: Match.absentProperty(),
})
Wobble: Match.absent(),
}),
});
```

Expand Down
39 changes: 21 additions & 18 deletions packages/@aws-cdk/assertions/lib/match.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { Matcher, MatchResult } from './matcher';
import { getType } from './private/type';

const ABSENT = '{{ABSENT}}';

/**
* Partial and special matching during template assertions.
*/
export abstract class Match {
/**
* Use this matcher in the place of a field's value, if the field must not be present.
*/
public static absentProperty(): string {
return ABSENT;
public static absent(): Matcher {
return new AbsentMatch('absent');
}

/**
Expand Down Expand Up @@ -129,10 +127,6 @@ class LiteralMatch extends Matcher {
return result;
}

if (this.pattern === ABSENT) {
throw new Error('absentProperty() can only be used in an object matcher');
}

if (actual !== this.pattern) {
result.push(this, [], `Expected ${this.pattern} but received ${actual}`);
}
Expand Down Expand Up @@ -185,9 +179,10 @@ class ArrayMatch extends Matcher {
const patternElement = this.pattern[patternIdx];

const matcher = Matcher.isMatcher(patternElement) ? patternElement : new LiteralMatch(this.name, patternElement);
if (this.subsequence && matcher instanceof AnyMatch) {
// array subsequence matcher is not compatible with anyValue() matcher. They don't make sense to be used together.
throw new Error('The Matcher anyValue() cannot be nested within arrayWith()');
const matcherName = matcher.name;
if (this.subsequence && (matcherName == 'absent' || matcherName == 'anyValue')) {
// array subsequence matcher is not compatible with anyValue() or absent() matcher. They don't make sense to be used together.
throw new Error(`The Matcher ${matcherName}() cannot be nested within arrayWith()`);
}

const innerResult = matcher.test(actual[actualIdx]);
Expand Down Expand Up @@ -253,13 +248,7 @@ class ObjectMatch extends Matcher {
}

for (const [patternKey, patternVal] of Object.entries(this.pattern)) {
if (patternVal === ABSENT) {
if (patternKey in actual) {
result.push(this, [`/${patternKey}`], 'Key should be absent');
}
continue;
}
if (!(patternKey in actual)) {
if (!(patternKey in actual) && !(patternVal instanceof AbsentMatch)) {
result.push(this, [`/${patternKey}`], 'Missing key');
continue;
}
Expand Down Expand Up @@ -339,4 +328,18 @@ class AnyMatch extends Matcher {
}
return result;
}
}

class AbsentMatch extends Matcher {
constructor(public readonly name: string) {
super();
}

public test(actual: any): MatchResult {
const result = new MatchResult(actual);
if (actual !== undefined) {
result.push(this, [], `Received ${actual}, but key should be absent`);
}
return result;
}
}
31 changes: 26 additions & 5 deletions packages/@aws-cdk/assertions/test/match.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('Matchers', () => {
});

test('absent', () => {
expect(() => Match.exact(Match.absentProperty()).test('foo')).toThrow(/absentProperty/);
expect(() => Match.exact(Match.absent())).toThrow(/cannot directly contain another matcher/);
});
});

Expand Down Expand Up @@ -125,8 +125,9 @@ describe('Matchers', () => {
expectFailure(matcher, [{ foo: 'baz', fred: 'waldo' }], [/Missing element at pattern index 0/]);
});

test('absent', () => {
expect(() => Match.arrayWith([Match.absentProperty()]).test(['foo'])).toThrow(/absentProperty/);
test('incompatible with absent', () => {
matcher = Match.arrayWith(['foo', Match.absent()]);
expect(() => matcher.test(['foo', 'bar'])).toThrow(/absent\(\) cannot be nested within arrayWith\(\)/);
});

test('incompatible with anyValue', () => {
Expand Down Expand Up @@ -184,9 +185,9 @@ describe('Matchers', () => {
});

test('absent', () => {
matcher = Match.objectLike({ foo: Match.absentProperty() });
matcher = Match.objectLike({ foo: Match.absent() });
expectPass(matcher, { bar: 'baz' });
expectFailure(matcher, { foo: 'baz' }, [/Key should be absent at \/foo/]);
expectFailure(matcher, { foo: 'baz' }, [/key should be absent at \/foo/]);
});
});

Expand Down Expand Up @@ -363,6 +364,26 @@ describe('Matchers', () => {
expectFailure(matcher, '{ "Foo"', [/invalid JSON string/i]);
});
});

describe('absent', () => {
let matcher: Matcher;

test('simple', () => {
matcher = Match.absent();
expectFailure(matcher, 'foo', ['Received foo, but key should be absent']);
expectPass(matcher, undefined);
});

test('nested in object', () => {
matcher = Match.objectLike({ foo: Match.absent() });
expectFailure(matcher, { foo: 'bar' }, [/key should be absent at \/foo/]);
expectFailure(matcher, { foo: [1, 2] }, [/key should be absent at \/foo/]);
expectFailure(matcher, { foo: null }, [/key should be absent at \/foo/]);

expectPass(matcher, { foo: undefined });
expectPass(matcher, {});
});
});
});

function expectPass(matcher: Matcher, target: any): void {
Expand Down
47 changes: 44 additions & 3 deletions packages/@aws-cdk/assertions/test/template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,11 @@ describe('Template', () => {

const inspect = Template.fromStack(stack);
inspect.hasResource('Foo::Bar', {
Properties: Match.objectLike({ foo: Match.absentProperty() }),
Properties: Match.objectLike({ foo: Match.absent() }),
});
expect(() => inspect.hasResource('Foo::Bar', {
Properties: Match.objectLike({ baz: Match.absentProperty() }),
})).toThrow(/Key should be absent at \/Properties\/baz/);
Properties: Match.objectLike({ baz: Match.absent() }),
})).toThrow(/key should be absent at \/Properties\/baz/);
});

test('incorrect types', () => {
Expand All @@ -269,6 +269,47 @@ describe('Template', () => {
});
});

describe('hasResourceProperties', () => {
test('absent', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
properties: { baz: 'qux' },
});

const inspect = Template.fromStack(stack);
inspect.hasResourceProperties('Foo::Bar', {
bar: Match.absent(),
});
expect(() => inspect.hasResourceProperties('Foo::Bar', {
baz: Match.absent(),
})).toThrow(/key should be absent at \/Properties\/baz/);
});

test('absent - no properties on template', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
});

const inspect = Template.fromStack(stack);
inspect.hasResourceProperties('Foo::Bar', Match.absent());
});

test('not', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
properties: { baz: 'qux' },
});

const inspect = Template.fromStack(stack);
inspect.hasResourceProperties('Foo::Bar', Match.not({
baz: 'boo',
}));
});
});

describe('getResources', () => {
test('matching resource type', () => {
const stack = new Stack();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('HttpLambdaAuthorizer', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Authorizer', {
AuthorizerPayloadFormatVersion: '1.0',
EnableSimpleResponses: Match.absentProperty(),
EnableSimpleResponses: Match.absent(),
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('HttpApi', () => {
new HttpApi(stack, 'HttpApi');

Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Api', {
CorsConfiguration: Match.absentProperty(),
CorsConfiguration: Match.absent(),
});
});

Expand Down Expand Up @@ -447,7 +447,7 @@ describe('HttpApi', () => {
Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Route', {
RouteKey: 'GET /chickens',
AuthorizationType: 'NONE',
AuthorizerId: Match.absentProperty(),
AuthorizerId: Match.absent(),
});
});

Expand All @@ -469,7 +469,7 @@ describe('HttpApi', () => {
});

Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Route', {
AuthorizationScopes: Match.absentProperty(),
AuthorizationScopes: Match.absent(),
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe('Alarm', () => {
Namespace: 'CDK/Test',
Period: 300,
Statistic: 'Maximum',
ExtendedStatistic: Match.absentProperty(),
ExtendedStatistic: Match.absent(),
Threshold: 1000,
});

Expand All @@ -152,7 +152,7 @@ describe('Alarm', () => {
MetricName: 'Metric',
Namespace: 'CDK/Test',
Period: 300,
Statistic: Match.absentProperty(),
Statistic: Match.absent(),
ExtendedStatistic: 'p99',
Threshold: 1000,
});
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('Alarm', () => {

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::CloudWatch::Alarm', {
Statistic: Match.absentProperty(),
Statistic: Match.absent(),
ExtendedStatistic: 'tm99.9999999999',
});

Expand Down
Loading

0 comments on commit c980185

Please sign in to comment.