From 5f40e5ba21b2311dbd904a396b7a4cb11a92c179 Mon Sep 17 00:00:00 2001 From: Dimitri Benin Date: Sat, 31 Dec 2016 11:50:03 +0100 Subject: [PATCH] fix(testing): async/fakeAsync/inject/withModule helpers should pass through context to callback functions (#13718) Make sure that context (`this`) that is passed to functions generated by test helpers is passed through to the callback functions. Enables usage of Jasmine's variable sharing system to prevent accidental memory leaks during test runs. --- modules/@angular/core/testing/async.ts | 19 ++++--- modules/@angular/core/testing/fake_async.ts | 3 +- modules/@angular/core/testing/test_bed.ts | 39 ++++++++------ .../test/testing_public_spec.ts | 54 +++++++++++++++---- .../public_api_guard/core/testing/index.d.ts | 2 +- 5 files changed, 80 insertions(+), 37 deletions(-) diff --git a/modules/@angular/core/testing/async.ts b/modules/@angular/core/testing/async.ts index 2f821d2201..b9c7de3fc0 100644 --- a/modules/@angular/core/testing/async.ts +++ b/modules/@angular/core/testing/async.ts @@ -31,14 +31,15 @@ export function async(fn: Function): (done: any) => any { // If we're running using the Jasmine test framework, adapt to call the 'done' // function when asynchronous activity is finished. if (_global.jasmine) { - return (done: any) => { + // Not using an arrow function to preserve context passed from call site + return function(done: any) { if (!done) { // if we run beforeEach in @angular/core/testing/testing_internal then we get no done // fake it here and assume sync. done = function() {}; done.fail = function(e: any) { throw e; }; } - runInTestZone(fn, done, (err: any) => { + runInTestZone(fn, this, done, (err: any) => { if (typeof err === 'string') { return done.fail(new Error(err)); } else { @@ -50,12 +51,16 @@ export function async(fn: Function): (done: any) => any { // Otherwise, return a promise which will resolve when asynchronous activity // is finished. This will be correctly consumed by the Mocha framework with // it('...', async(myFn)); or can be used in a custom framework. - return () => new Promise((finishCallback, failCallback) => { - runInTestZone(fn, finishCallback, failCallback); - }); + // Not using an arrow function to preserve context passed from call site + return function() { + return new Promise((finishCallback, failCallback) => { + runInTestZone(fn, this, finishCallback, failCallback); + }); + }; } -function runInTestZone(fn: Function, finishCallback: Function, failCallback: Function) { +function runInTestZone( + fn: Function, context: any, finishCallback: Function, failCallback: Function) { const currentZone = Zone.current; const AsyncTestZoneSpec = (Zone as any)['AsyncTestZoneSpec']; if (AsyncTestZoneSpec === undefined) { @@ -103,5 +108,5 @@ function runInTestZone(fn: Function, finishCallback: Function, failCallback: Fun 'test'); proxyZoneSpec.setDelegate(testZoneSpec); }); - return Zone.current.runGuarded(fn); + return Zone.current.runGuarded(fn, context); } diff --git a/modules/@angular/core/testing/fake_async.ts b/modules/@angular/core/testing/fake_async.ts index d72fb0c294..d87ae709c3 100644 --- a/modules/@angular/core/testing/fake_async.ts +++ b/modules/@angular/core/testing/fake_async.ts @@ -48,6 +48,7 @@ let _inFakeAsyncCall = false; * @experimental */ export function fakeAsync(fn: Function): (...args: any[]) => any { + // Not using an arrow function to preserve context passed from call site return function(...args: any[]) { const proxyZoneSpec = ProxyZoneSpec.assertPresent(); if (_inFakeAsyncCall) { @@ -67,7 +68,7 @@ export function fakeAsync(fn: Function): (...args: any[]) => any { const lastProxyZoneSpec = proxyZoneSpec.getDelegate(); proxyZoneSpec.setDelegate(_fakeAsyncTestZoneSpec); try { - res = fn(...args); + res = fn.apply(this, args); flushMicrotasks(); } finally { proxyZoneSpec.setDelegate(lastProxyZoneSpec); diff --git a/modules/@angular/core/testing/test_bed.ts b/modules/@angular/core/testing/test_bed.ts index 9e9e29c110..50f9afc98f 100644 --- a/modules/@angular/core/testing/test_bed.ts +++ b/modules/@angular/core/testing/test_bed.ts @@ -325,10 +325,10 @@ export class TestBed implements Injector { return result === UNDEFINED ? this._compiler.injector.get(token, notFoundValue) : result; } - execute(tokens: any[], fn: Function): any { + execute(tokens: any[], fn: Function, context?: any): any { this._initIfNeeded(); const params = tokens.map(t => this.get(t)); - return fn(...params); + return fn.apply(context, params); } overrideModule(ngModule: Type, override: MetadataOverride): void { @@ -413,17 +413,19 @@ export function getTestBed() { export function inject(tokens: any[], fn: Function): () => any { const testBed = getTestBed(); if (tokens.indexOf(AsyncTestCompleter) >= 0) { - return () => - // Return an async test method that returns a Promise if AsyncTestCompleter is one of - // the - // injected tokens. - testBed.compileComponents().then(() => { - const completer: AsyncTestCompleter = testBed.get(AsyncTestCompleter); - testBed.execute(tokens, fn); - return completer.promise; - }); + // Not using an arrow function to preserve context passed from call site + return function() { + // Return an async test method that returns a Promise if AsyncTestCompleter is one of + // the injected tokens. + return testBed.compileComponents().then(() => { + const completer: AsyncTestCompleter = testBed.get(AsyncTestCompleter); + testBed.execute(tokens, fn, this); + return completer.promise; + }); + }; } else { - return () => testBed.execute(tokens, fn); + // Not using an arrow function to preserve context passed from call site + return function() { return testBed.execute(tokens, fn, this); }; } } @@ -441,9 +443,11 @@ export class InjectSetupWrapper { } inject(tokens: any[], fn: Function): () => any { - return () => { - this._addModule(); - return inject(tokens, fn)(); + const self = this; + // Not using an arrow function to preserve context passed from call site + return function() { + self._addModule(); + return inject(tokens, fn).call(this); }; } } @@ -456,12 +460,13 @@ export function withModule(moduleDef: TestModuleMetadata, fn: Function): () => a export function withModule(moduleDef: TestModuleMetadata, fn: Function = null): (() => any)| InjectSetupWrapper { if (fn) { - return () => { + // Not using an arrow function to preserve context passed from call site + return function() { const testBed = getTestBed(); if (moduleDef) { testBed.configureTestingModule(moduleDef); } - return fn(); + return fn.apply(this); }; } return new InjectSetupWrapper(() => moduleDef); diff --git a/modules/@angular/platform-browser/test/testing_public_spec.ts b/modules/@angular/platform-browser/test/testing_public_spec.ts index b7b4d98bcf..a68cd66064 100644 --- a/modules/@angular/platform-browser/test/testing_public_spec.ts +++ b/modules/@angular/platform-browser/test/testing_public_spec.ts @@ -114,31 +114,63 @@ class CompWithUrlTemplate { export function main() { describe('public testing API', () => { - describe('using the async helper', () => { - let actuallyDone: boolean; + describe('using the async helper with context passing', () => { + beforeEach(function() { this.actuallyDone = false; }); - beforeEach(() => actuallyDone = false); + afterEach(function() { expect(this.actuallyDone).toEqual(true); }); - afterEach(() => expect(actuallyDone).toEqual(true)); + it('should run normal tests', function() { this.actuallyDone = true; }); - it('should run normal tests', () => actuallyDone = true); - - it('should run normal async tests', (done) => { + it('should run normal async tests', function(done) { setTimeout(() => { - actuallyDone = true; + this.actuallyDone = true; done(); }, 0); }); it('should run async tests with tasks', - async(() => setTimeout(() => actuallyDone = true, 0))); + async(function() { setTimeout(() => this.actuallyDone = true, 0); })); - it('should run async tests with promises', async(() => { + it('should run async tests with promises', async(function() { const p = new Promise((resolve, reject) => setTimeout(resolve, 10)); - p.then(() => actuallyDone = true); + p.then(() => this.actuallyDone = true); })); }); + describe('basic context passing to inject, fakeAsync and withModule helpers', () => { + const moduleConfig = { + providers: [FancyService], + }; + + beforeEach(function() { this.contextModified = false; }); + + afterEach(function() { expect(this.contextModified).toEqual(true); }); + + it('should pass context to inject helper', + inject([], function() { this.contextModified = true; })); + + it('should pass context to fakeAsync helper', + fakeAsync(function() { this.contextModified = true; })); + + it('should pass context to withModule helper - simple', + withModule(moduleConfig, function() { this.contextModified = true; })); + + it('should pass context to withModule helper - advanced', + withModule(moduleConfig).inject([FancyService], function(service: FancyService) { + expect(service.value).toBe('real value'); + this.contextModified = true; + })); + + it('should preserve context when async and inject helpers are combined', + async(inject([], function() { setTimeout(() => this.contextModified = true, 0); }))); + + it('should preserve context when fakeAsync and inject helpers are combined', + fakeAsync(inject([], function() { + setTimeout(() => this.contextModified = true, 0); + tick(1); + }))); + }); + describe('using the test injector with the inject helper', () => { describe('setting up Providers', () => { beforeEach(() => { diff --git a/tools/public_api_guard/core/testing/index.d.ts b/tools/public_api_guard/core/testing/index.d.ts index 1bcc8a6ce9..c993bb086a 100644 --- a/tools/public_api_guard/core/testing/index.d.ts +++ b/tools/public_api_guard/core/testing/index.d.ts @@ -67,7 +67,7 @@ export declare class TestBed implements Injector { }): void; configureTestingModule(moduleDef: TestModuleMetadata): void; createComponent(component: Type): ComponentFixture; - execute(tokens: any[], fn: Function): any; + execute(tokens: any[], fn: Function, context?: any): any; get(token: any, notFoundValue?: any): any; /** @experimental */ initTestEnvironment(ngModule: Type, platform: PlatformRef): void; overrideComponent(component: Type, override: MetadataOverride): void;