Skip to content

Commit

Permalink
Context providers: return dummy values if env is undefined
Browse files Browse the repository at this point in the history
Throwing if "env" is undefined makes it hard to unit test stacks that
use context providers. It requires dummy values for account and region,
which is boilerplate and non-intuitive.

Since we already have a concept of dummy context values, this change
short-circuits context resolution and returns the dummy values in case
the stack's env is undefined.
  • Loading branch information
Elad Ben-Israel committed Jul 3, 2018
1 parent 952473a commit 8823eac
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
22 changes: 16 additions & 6 deletions packages/@aws-cdk/core/lib/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ export class ContextProvider {
/**
* Read a provider value, verifying it's a string
*/
public getStringValue(provider: string, scope: string[], args: string[]): string {
public getStringValue(provider: string, scope: undefined | string[], args: string[], defaultValue = ''): string {
// if scope is undefined, this is probably a test mode, so we just
// return the default value
if (!scope) {
return defaultValue;
}
const key = colonQuote([provider].concat(scope).concat(args)).join(':');
const value = this.context.getContext(key);
if (value != null) {
Expand All @@ -35,13 +40,19 @@ export class ContextProvider {
}

this.stack.reportMissingContext(key, { provider, scope, args });
return '';
return defaultValue;
}

/**
* Read a provider value, verifying it's a list
*/
public getStringListValue(provider: string, scope: string[], args: string[], defaultValue = ['']): string[] {
public getStringListValue(provider: string, scope: undefined | string[], args: string[], defaultValue = ['']): string[] {
// if scope is undefined, this is probably a test mode, so we just
// return the default value
if (!scope) {
return defaultValue;
}

const key = colonQuote([provider].concat(scope).concat(args)).join(':');
const value = this.context.getContext(key);

Expand All @@ -59,7 +70,7 @@ export class ContextProvider {
/**
* Helper function to wrap up account and region into a scope tuple
*/
public accountRegionScope(providerDescription: string): string[] {
public accountRegionScope(providerDescription: string): undefined | string[] {
const stack = Stack.find(this.context);
if (!stack) {
throw new Error(`${providerDescription}: construct must be in a stack`);
Expand All @@ -69,8 +80,7 @@ export class ContextProvider {
const region = stack.env.region;

if (account == null || region == null) {
// tslint:disable-next-line:max-line-length
throw new Error(`${providerDescription}: requires account and region information, but ${stack.name} doesn't have an "env" defined`);
return undefined;
}

return [account, region];
Expand Down
9 changes: 8 additions & 1 deletion packages/@aws-cdk/core/test/test.context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ export = {
test.deepEqual(azs, 'abc');

test.done();
}
},

'Return default values if "env" is undefined to facilitate unit tests'(test: Test) {
const stack = new Stack();
test.deepEqual(new AvailabilityZoneProvider(stack).availabilityZones, [ 'dummy1a', 'dummy1b', 'dummy1c' ]);
test.deepEqual(new SSMParameterProvider(stack).getString('foo'), '');
test.done();
},
};

function firstKey(obj: any): string {
Expand Down

0 comments on commit 8823eac

Please sign in to comment.