From e748adda2e7a1f6e302628d0d76b5c3d1e3fc196 Mon Sep 17 00:00:00 2001 From: Julie Ralph Date: Tue, 8 Dec 2015 19:03:21 -0800 Subject: [PATCH] refactor(testing): move common testing logic into test_injector Before, all test framework wrappers (internal for dart and js/ts, angular2_test for dart and testing for js/ts) had similar logic to keep track of current global test injector and test provider list. This change wraps that logic into one class managed by the test injector. Closes #5920 --- modules/angular2/src/testing/test_injector.ts | 56 ++++++++++++++++++- modules/angular2/src/testing/testing.ts | 30 ++++------ .../src/testing/testing_internal.dart | 31 +++++----- .../angular2/src/testing/testing_internal.ts | 36 +++++------- ...client_server_message_bus.server.spec.dart | 1 - ...client_server_message_bus.server.spec.dart | 1 - .../web_socket_message_bus_spec.dart | 1 - .../web_workers/shared/message_bus_spec.ts | 1 - .../shared/service_message_broker_spec.ts | 1 - .../worker/event_dispatcher_spec.ts | 1 - .../worker/renderer_integration_spec.ts | 6 +- .../test/web_workers/worker/xhr_impl_spec.ts | 1 - .../lib/angular2_testing.dart | 48 +++++++--------- 13 files changed, 120 insertions(+), 94 deletions(-) diff --git a/modules/angular2/src/testing/test_injector.ts b/modules/angular2/src/testing/test_injector.ts index 637d5d8d3e..05fddf45f8 100644 --- a/modules/angular2/src/testing/test_injector.ts +++ b/modules/angular2/src/testing/test_injector.ts @@ -23,7 +23,7 @@ import { defaultKeyValueDiffers, ChangeDetectorGenConfig } from 'angular2/src/core/change_detection/change_detection'; -import {ExceptionHandler} from 'angular2/src/facade/exceptions'; +import {BaseException, ExceptionHandler} from 'angular2/src/facade/exceptions'; import {PipeResolver} from 'angular2/src/core/linker/pipe_resolver'; import {XHR} from 'angular2/src/compiler/xhr'; @@ -131,11 +131,62 @@ function _runtimeCompilerBindings() { ]; } +export class TestInjector { + private _instantiated: boolean = false; + + private _injector: Injector = null; + + private _providers: Array = []; + + reset() { + this._injector = null; + this._providers = []; + this._instantiated = false; + } + + addProviders(providers: Array) { + if (this._instantiated) { + throw new BaseException('Cannot add providers after test injector is instantiated'); + } + this._providers = ListWrapper.concat(this._providers, providers); + } + + createInjector() { + var rootInjector = Injector.resolveAndCreate(_getRootProviders()); + this._injector = rootInjector.resolveAndCreateChild(ListWrapper.concat( + ListWrapper.concat(_getAppBindings(), _runtimeCompilerBindings()), this._providers)); + this._instantiated = true; + return this._injector; + } + + execute(fn: FunctionWithParamTokens): any { + if (!this._instantiated) { + this.createInjector(); + } + return fn.execute(this._injector); + } +} + +var _testInjector: TestInjector = null; + +export function getTestInjector() { + if (_testInjector == null) { + _testInjector = new TestInjector(); + } + return _testInjector; +} + +/** + * @deprecated Use TestInjector#createInjector() instead. + */ export function createTestInjector(providers: Array): Injector { var rootInjector = Injector.resolveAndCreate(_getRootProviders()); return rootInjector.resolveAndCreateChild(ListWrapper.concat(_getAppBindings(), providers)); } +/** + * @deprecated Use TestInjector#createInjector() instead. + */ export function createTestInjectorWithRuntimeCompiler( providers: Array): Injector { return createTestInjector(ListWrapper.concat(_runtimeCompilerBindings(), providers)); @@ -159,7 +210,8 @@ export function createTestInjectorWithRuntimeCompiler( * ``` * * Notes: - * - inject is currently a function because of some Traceur limitation the syntax should eventually + * - inject is currently a function because of some Traceur limitation the syntax should + * eventually * becomes `it('...', @Inject (object: AClass, async: AsyncTestCompleter) => { ... });` * * @param {Array} tokens diff --git a/modules/angular2/src/testing/testing.ts b/modules/angular2/src/testing/testing.ts index e29d46e8e0..73176eea21 100644 --- a/modules/angular2/src/testing/testing.ts +++ b/modules/angular2/src/testing/testing.ts @@ -7,10 +7,11 @@ import {ListWrapper} from 'angular2/src/facade/collection'; import {bind} from 'angular2/core'; import { - createTestInjectorWithRuntimeCompiler, FunctionWithParamTokens, inject, - injectAsync + injectAsync, + TestInjector, + getTestInjector } from './test_injector'; export {inject, injectAsync} from './test_injector'; @@ -92,14 +93,10 @@ var jsmIt = _global.it; var jsmIIt = _global.fit; var jsmXIt = _global.xit; -var testProviders; -var injector; +var testInjector: TestInjector = getTestInjector(); // Reset the test providers before each test. -jsmBeforeEach(() => { - testProviders = []; - injector = null; -}); +jsmBeforeEach(() => { testInjector.reset(); }); /** * Allows overriding default providers of the test injector, @@ -115,8 +112,9 @@ export function beforeEachProviders(fn): void { jsmBeforeEach(() => { var providers = fn(); if (!providers) return; - testProviders = [...testProviders, ...providers]; - if (injector !== null) { + try { + testInjector.addProviders(providers); + } catch (e) { throw new Error('beforeEachProviders was called after the injector had ' + 'been used in a beforeEach or it block. This invalidates the ' + 'test injector'); @@ -188,17 +186,13 @@ function _it(jsmFn: Function, name: string, testFn: FunctionWithParamTokens | An if (testFn instanceof FunctionWithParamTokens) { jsmFn(name, (done) => { - if (!injector) { - injector = createTestInjectorWithRuntimeCompiler(testProviders); - } - var finishCallback = () => { // Wait one more event loop to make sure we catch unreturned promises and // promise rejections. setTimeout(done, 0); }; var returnedTestValue = - runInTestZone(() => testFn.execute(injector), finishCallback, done.fail); + runInTestZone(() => testInjector.execute(testFn), finishCallback, done.fail); if (testFn.isAsync) { if (_isPromiseLike(returnedTestValue)) { @@ -243,11 +237,9 @@ export function beforeEach(fn: FunctionWithParamTokens | AnyTestFn): void { // promise rejections. setTimeout(done, 0); }; - if (!injector) { - injector = createTestInjectorWithRuntimeCompiler(testProviders); - } - var returnedTestValue = runInTestZone(() => fn.execute(injector), finishCallback, done.fail); + var returnedTestValue = + runInTestZone(() => testInjector.execute(fn), finishCallback, done.fail); if (fn.isAsync) { if (_isPromiseLike(returnedTestValue)) { (>returnedTestValue).then(null, (err) => { done.fail(err); }); diff --git a/modules/angular2/src/testing/testing_internal.dart b/modules/angular2/src/testing/testing_internal.dart index 0bc0f6d5a2..821300b742 100644 --- a/modules/angular2/src/testing/testing_internal.dart +++ b/modules/angular2/src/testing/testing_internal.dart @@ -21,20 +21,23 @@ import 'package:angular2/src/core/reflection/reflection.dart'; import 'package:angular2/src/core/reflection/reflection_capabilities.dart'; import 'package:angular2/src/core/di/provider.dart' show bind; -import 'package:angular2/src/core/di/injector.dart' show Injector; import 'package:angular2/src/facade/collection.dart' show StringMapWrapper; import 'test_injector.dart'; export 'test_injector.dart' show inject; -List _testBindings = []; -Injector _injector; +TestInjector _testInjector = getTestInjector(); bool _isCurrentTestAsync; +Future _currentTestFuture; bool _inIt = false; class AsyncTestCompleter { final _completer = new Completer(); + AsyncTestCompleter() { + _currentTestFuture = this.future; + } + void done() { _completer.complete(); } @@ -50,10 +53,11 @@ void testSetup() { // - Priority 1: create the test injector to be used in beforeEach() and it() gns.beforeEach(() { - _testBindings.clear(); + _testInjector.reset(); + _currentTestFuture = null; }, priority: 3); - var completerBinding = bind(AsyncTestCompleter).toFactory(() { + var completerProvider = bind(AsyncTestCompleter).toFactory(() { // Mark the test as async when an AsyncTestCompleter is injected in an it(), if (!_inIt) throw 'AsyncTestCompleter can only be injected in an "it()"'; _isCurrentTestAsync = true; @@ -62,15 +66,14 @@ void testSetup() { gns.beforeEach(() { _isCurrentTestAsync = false; - _testBindings.add(completerBinding); - _injector = createTestInjectorWithRuntimeCompiler(_testBindings); + _testInjector.addProviders([completerProvider]); }, priority: 1); } /** - * Allows overriding default bindings defined in test_injector.js. + * Allows overriding default providers defined in test_injector.js. * - * The given function must return a list of DI bindings. + * The given function must return a list of DI providers. * * Example: * @@ -81,8 +84,8 @@ void testSetup() { */ void beforeEachProviders(Function fn) { gns.beforeEach(() { - var bindings = fn(); - if (bindings != null) _testBindings.addAll(bindings); + var providers = fn(); + if (providers != null) _testInjector.addProviders(providers); }, priority: 2); } @@ -95,7 +98,7 @@ void beforeEach(fn) { if (fn is! FunctionWithParamTokens) fn = new FunctionWithParamTokens([], fn, false); gns.beforeEach(() { - fn.execute(_injector); + _testInjector.execute(fn); }); } @@ -104,9 +107,9 @@ void _it(gnsFn, name, fn) { new FunctionWithParamTokens([], fn, false); gnsFn(name, () { _inIt = true; - fn.execute(_injector); + _testInjector.execute(fn); _inIt = false; - if (_isCurrentTestAsync) return _injector.get(AsyncTestCompleter).future; + if (_isCurrentTestAsync) return _currentTestFuture; }); } diff --git a/modules/angular2/src/testing/testing_internal.ts b/modules/angular2/src/testing/testing_internal.ts index 86ac1e380e..72ca9ebb10 100644 --- a/modules/angular2/src/testing/testing_internal.ts +++ b/modules/angular2/src/testing/testing_internal.ts @@ -5,11 +5,7 @@ import {NgZoneZone} from 'angular2/src/core/zone/ng_zone'; import {provide} from 'angular2/core'; -import { - createTestInjectorWithRuntimeCompiler, - FunctionWithParamTokens, - inject -} from './test_injector'; +import {TestInjector, getTestInjector, FunctionWithParamTokens, inject} from './test_injector'; import {browserDetection} from './utils'; export {inject} from './test_injector'; @@ -48,7 +44,7 @@ var inIt = false; jasmine.DEFAULT_TIMEOUT_INTERVAL = 500; var globalTimeOut = browserDetection.isSlow ? 3000 : jasmine.DEFAULT_TIMEOUT_INTERVAL; -var testProviders; +var testInjector = getTestInjector(); /** * Mechanism to run `beforeEach()` functions of Angular tests. @@ -62,16 +58,17 @@ class BeforeEachRunner { beforeEach(fn: FunctionWithParamTokens | SyncTestFn): void { this._fns.push(fn); } - run(injector): void { - if (this._parent) this._parent.run(injector); + run(): void { + if (this._parent) this._parent.run(); this._fns.forEach((fn) => { - return isFunction(fn) ? (fn)() : (fn).execute(injector); + return isFunction(fn) ? (fn)() : + (testInjector.execute(fn)); }); } } // Reset the test providers before each test -jsmBeforeEach(() => { testProviders = []; }); +jsmBeforeEach(() => { testInjector.reset(); }); function _describe(jsmFn, ...args) { var parentRunner = runnerStack.length === 0 ? null : runnerStack[runnerStack.length - 1]; @@ -120,7 +117,7 @@ export function beforeEachProviders(fn): void { jsmBeforeEach(() => { var providers = fn(); if (!providers) return; - testProviders = [...testProviders, ...providers]; + testInjector.addProviders(providers); }); } @@ -150,18 +147,17 @@ function _it(jsmFn: Function, name: string, testFn: FunctionWithParamTokens | An } }); - var injector = createTestInjectorWithRuntimeCompiler([...testProviders, completerProvider]); - runner.run(injector); + testInjector.addProviders([completerProvider]); + runner.run(); inIt = true; - testFn.execute(injector); + testInjector.execute(testFn); inIt = false; }, timeOut); } else { jsmFn(name, () => { - var injector = createTestInjectorWithRuntimeCompiler(testProviders); - runner.run(injector); - testFn.execute(injector); + runner.run(); + testInjector.execute(testFn); }, timeOut); } @@ -170,14 +166,12 @@ function _it(jsmFn: Function, name: string, testFn: FunctionWithParamTokens | An if ((testFn).length === 0) { jsmFn(name, () => { - var injector = createTestInjectorWithRuntimeCompiler(testProviders); - runner.run(injector); + runner.run(); (testFn)(); }, timeOut); } else { jsmFn(name, (done) => { - var injector = createTestInjectorWithRuntimeCompiler(testProviders); - runner.run(injector); + runner.run(); (testFn)(done); }, timeOut); } diff --git a/modules/angular2/test/web_workers/debug_tools/multi_client_server_message_bus.server.spec.dart b/modules/angular2/test/web_workers/debug_tools/multi_client_server_message_bus.server.spec.dart index 43494c9e6a..857b948b0b 100644 --- a/modules/angular2/test/web_workers/debug_tools/multi_client_server_message_bus.server.spec.dart +++ b/modules/angular2/test/web_workers/debug_tools/multi_client_server_message_bus.server.spec.dart @@ -11,7 +11,6 @@ import "package:angular2/testing_internal.dart" iit, expect, beforeEach, - createTestInjector, beforeEachProviders, SpyObject, proxy; diff --git a/modules/angular2/test/web_workers/debug_tools/single_client_server_message_bus.server.spec.dart b/modules/angular2/test/web_workers/debug_tools/single_client_server_message_bus.server.spec.dart index 0cf86f781b..50ca5f15ae 100644 --- a/modules/angular2/test/web_workers/debug_tools/single_client_server_message_bus.server.spec.dart +++ b/modules/angular2/test/web_workers/debug_tools/single_client_server_message_bus.server.spec.dart @@ -10,7 +10,6 @@ import "package:angular2/testing_internal.dart" it, expect, beforeEach, - createTestInjector, beforeEachProviders, SpyObject, proxy; diff --git a/modules/angular2/test/web_workers/debug_tools/web_socket_message_bus_spec.dart b/modules/angular2/test/web_workers/debug_tools/web_socket_message_bus_spec.dart index 6823eb008a..9b87a3e281 100644 --- a/modules/angular2/test/web_workers/debug_tools/web_socket_message_bus_spec.dart +++ b/modules/angular2/test/web_workers/debug_tools/web_socket_message_bus_spec.dart @@ -8,7 +8,6 @@ import "package:angular2/testing_internal.dart" it, expect, beforeEach, - createTestInjector, beforeEachProviders, SpyObject, proxy; diff --git a/modules/angular2/test/web_workers/shared/message_bus_spec.ts b/modules/angular2/test/web_workers/shared/message_bus_spec.ts index e4fcc2ade0..1e3073c94e 100644 --- a/modules/angular2/test/web_workers/shared/message_bus_spec.ts +++ b/modules/angular2/test/web_workers/shared/message_bus_spec.ts @@ -5,7 +5,6 @@ import { it, expect, beforeEach, - createTestInjectorWithRuntimeCompiler, beforeEachProviders, SpyObject, proxy diff --git a/modules/angular2/test/web_workers/shared/service_message_broker_spec.ts b/modules/angular2/test/web_workers/shared/service_message_broker_spec.ts index 9b0b148221..d30be8bc59 100644 --- a/modules/angular2/test/web_workers/shared/service_message_broker_spec.ts +++ b/modules/angular2/test/web_workers/shared/service_message_broker_spec.ts @@ -5,7 +5,6 @@ import { it, expect, beforeEach, - createTestInjectorWithRuntimeCompiler, beforeEachProviders, SpyObject, proxy diff --git a/modules/angular2/test/web_workers/worker/event_dispatcher_spec.ts b/modules/angular2/test/web_workers/worker/event_dispatcher_spec.ts index 5efa1ea811..a97d1b0096 100644 --- a/modules/angular2/test/web_workers/worker/event_dispatcher_spec.ts +++ b/modules/angular2/test/web_workers/worker/event_dispatcher_spec.ts @@ -5,7 +5,6 @@ import { it, expect, beforeEach, - createTestInjectorWithRuntimeCompiler, beforeEachProviders, SpyObject, proxy diff --git a/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts b/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts index dbeca90582..8f56648945 100644 --- a/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts +++ b/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts @@ -7,8 +7,8 @@ import { iit, expect, beforeEach, - createTestInjectorWithRuntimeCompiler, beforeEachProviders, + TestInjector, TestComponentBuilder } from "angular2/testing_internal"; import {DOM} from 'angular2/src/platform/dom/dom_adapter'; @@ -102,12 +102,14 @@ export function main() { beforeEachProviders(() => { var uiRenderProtoViewStore = new RenderProtoViewRefStore(false); uiRenderViewStore = new RenderViewWithFragmentsStore(false); - uiInjector = createTestInjectorWithRuntimeCompiler([ + var testInjector = new TestInjector(); + testInjector.addProviders([ provide(RenderProtoViewRefStore, {useValue: uiRenderProtoViewStore}), provide(RenderViewWithFragmentsStore, {useValue: uiRenderViewStore}), provide(DomRenderer, {useClass: DomRenderer_}), provide(Renderer, {useExisting: DomRenderer}) ]); + uiInjector = testInjector.createInjector(); var uiSerializer = uiInjector.get(Serializer); var domRenderer = uiInjector.get(DomRenderer); var workerRenderProtoViewStore = new RenderProtoViewRefStore(true); diff --git a/modules/angular2/test/web_workers/worker/xhr_impl_spec.ts b/modules/angular2/test/web_workers/worker/xhr_impl_spec.ts index 83fe0f9828..dfb68279af 100644 --- a/modules/angular2/test/web_workers/worker/xhr_impl_spec.ts +++ b/modules/angular2/test/web_workers/worker/xhr_impl_spec.ts @@ -5,7 +5,6 @@ import { it, expect, beforeEach, - createTestInjectorWithRuntimeCompiler, beforeEachProviders } from 'angular2/testing_internal'; import {SpyMessageBroker} from './spies'; diff --git a/modules_dart/angular2_testing/lib/angular2_testing.dart b/modules_dart/angular2_testing/lib/angular2_testing.dart index 60d1302dd3..da737b3863 100644 --- a/modules_dart/angular2_testing/lib/angular2_testing.dart +++ b/modules_dart/angular2_testing/lib/angular2_testing.dart @@ -1,11 +1,8 @@ library angular2_testing.angular2_testing; import 'package:test/test.dart'; -import 'package:test/src/backend/invoker.dart'; -import 'package:test/src/backend/live_test.dart'; import 'package:angular2/angular2.dart'; -import 'package:angular2/src/core/di/injector.dart' show Injector; import 'package:angular2/src/core/di/metadata.dart' show InjectMetadata; import 'package:angular2/src/core/di/exceptions.dart' show NoAnnotationError; import 'package:angular2/platform/browser_static.dart' show BrowserDomAdapter; @@ -31,6 +28,13 @@ void initAngularTests() { reflector.reflectionCapabilities = new ReflectionCapabilities(); } +void _addTestInjectorTearDown() { + // Multiple resets are harmless. + tearDown(() { + _testInjector.reset(); + }); +} + /// Allows overriding default bindings defined in test_injector.dart. /// /// The given function must return a list of DI providers. @@ -45,13 +49,17 @@ void initAngularTests() { /// ``` void setUpProviders(Iterable providerFactory()) { setUp(() { - if (_currentInjector != null) { + try { + _testInjector.addProviders(providerFactory()); + } catch(e) { throw 'setUpProviders was called after the injector had ' 'been used in a setUp or test block. This invalidates the ' 'test injector'; } - _currentTestProviders.addAll(providerFactory()); + }); + + _addTestInjectorTearDown(); } dynamic _runInjectableFunction(Function fn) { @@ -72,11 +80,8 @@ dynamic _runInjectableFunction(Function fn) { tokens.add(token); } - if (_currentInjector == null) { - _currentInjector = createTestInjectorWithRuntimeCompiler(_currentTestProviders); - } var injectFn = new FunctionWithParamTokens(tokens, fn, false); - return injectFn.execute(_currentInjector); + return _testInjector.execute(injectFn); } /// Use the test injector to get bindings and run a function. @@ -92,6 +97,8 @@ void ngSetUp(Function fn) { setUp(() async { await _runInjectableFunction(fn); }); + + _addTestInjectorTearDown(); } /// Add a test which can use the test injector. @@ -108,25 +115,8 @@ void ngTest(String description, Function fn, test(description, () async { await _runInjectableFunction(fn); }, testOn: testOn, timeout: timeout, skip: skip, onPlatform: onPlatform); + + _addTestInjectorTearDown(); } -final _providersExpando = - new Expando>('Providers for the current test'); -final _injectorExpando = - new Expando('Angular Injector for the current test'); - -List get _currentTestProviders { - if (_providersExpando[_currentTest] == null) { - return _providersExpando[_currentTest] = []; - } - return _providersExpando[_currentTest]; -} - -Injector get _currentInjector => _injectorExpando[_currentTest]; -void set _currentInjector(Injector newInjector) { - _injectorExpando[_currentTest] = newInjector; -} - -// TODO: warning, the Invoker.current.liveTest is not a settled API and is -// subject to change in future versions of package:test. -LiveTest get _currentTest => Invoker.current.liveTest; +final TestInjector _testInjector = getTestInjector();