From d7ab5d44a586856e488934fdf3847abc89c3525c Mon Sep 17 00:00:00 2001 From: Julie Ralph Date: Wed, 14 Oct 2015 09:41:15 -0700 Subject: [PATCH] fix(testing): let DOM adapter dictate XHR implementation for tests The test injector now uses an XHR implementation based on DOM.getXHR, which allows the current DOM adapter to dictate which XHR impl should be used. To prevent the changes to DOM adapter from introducing undesired new dependencies into the benchmarks, separate the async facade into a promise facade which is reexported by facade/async. See #4539 --- .../angular2/src/core/compiler/xhr_impl.ts | 4 +- .../src/core/dom/abstract_html_adapter.dart | 4 ++ modules/angular2/src/core/dom/dom_adapter.ts | 4 +- .../src/core/dom/generic_browser_adapter.ts | 6 +- .../angular2/src/core/dom/parse5_adapter.ts | 4 ++ modules/angular2/src/core/facade/async.dart | 51 +---------------- modules/angular2/src/core/facade/async.ts | 57 ++----------------- modules/angular2/src/core/facade/promise.dart | 53 +++++++++++++++++ modules/angular2/src/core/facade/promise.ts | 54 ++++++++++++++++++ modules/angular2/src/testing/test_injector.ts | 2 +- .../test/testing/testing_public_spec.ts | 5 ++ modules/angular2_material/test/button_spec.ts | 8 --- 12 files changed, 136 insertions(+), 116 deletions(-) create mode 100644 modules/angular2/src/core/facade/promise.dart create mode 100644 modules/angular2/src/core/facade/promise.ts diff --git a/modules/angular2/src/core/compiler/xhr_impl.ts b/modules/angular2/src/core/compiler/xhr_impl.ts index ecb1384eea..0462392115 100644 --- a/modules/angular2/src/core/compiler/xhr_impl.ts +++ b/modules/angular2/src/core/compiler/xhr_impl.ts @@ -1,9 +1,7 @@ -import {Injectable} from 'angular2/src/core/di'; -import {Promise, PromiseWrapper, PromiseCompleter} from 'angular2/src/core/facade/async'; +import {Promise, PromiseWrapper, PromiseCompleter} from 'angular2/src/core/facade/promise'; import {isPresent} from 'angular2/src/core/facade/lang'; import {XHR} from './xhr'; -@Injectable() export class XHRImpl extends XHR { get(url: string): Promise { var completer: PromiseCompleter < string >= PromiseWrapper.completer(); diff --git a/modules/angular2/src/core/dom/abstract_html_adapter.dart b/modules/angular2/src/core/dom/abstract_html_adapter.dart index ed42a32980..e38156cc72 100644 --- a/modules/angular2/src/core/dom/abstract_html_adapter.dart +++ b/modules/angular2/src/core/dom/abstract_html_adapter.dart @@ -5,6 +5,7 @@ import 'package:html/dom.dart'; import 'dom_adapter.dart'; import 'emulated_css.dart'; +import '../compiler/xhr.dart'; const _attrToPropMap = const { 'innerHtml': 'innerHTML', @@ -66,6 +67,9 @@ abstract class AbstractHtml5LibAdapter implements DomAdapter { throw 'not implemented'; } + @override + Type getXHR() => XHR; + Element parse(String templateHtml) => parser.parse(templateHtml).firstChild; query(selector) { throw 'not implemented'; diff --git a/modules/angular2/src/core/dom/dom_adapter.ts b/modules/angular2/src/core/dom/dom_adapter.ts index 93cb5d799d..f1b016082c 100644 --- a/modules/angular2/src/core/dom/dom_adapter.ts +++ b/modules/angular2/src/core/dom/dom_adapter.ts @@ -1,4 +1,4 @@ -import {isBlank} from 'angular2/src/core/facade/lang'; +import {isBlank, Type} from 'angular2/src/core/facade/lang'; export var DOM: DomAdapter; @@ -23,6 +23,8 @@ export abstract class DomAdapter { abstract logGroup(error); abstract logGroupEnd(); + abstract getXHR(): Type; + /** * Maps attribute names to their corresponding property names for cases * where attribute name doesn't match property name. diff --git a/modules/angular2/src/core/dom/generic_browser_adapter.ts b/modules/angular2/src/core/dom/generic_browser_adapter.ts index b2c5462806..30e0561be3 100644 --- a/modules/angular2/src/core/dom/generic_browser_adapter.ts +++ b/modules/angular2/src/core/dom/generic_browser_adapter.ts @@ -1,6 +1,8 @@ import {ListWrapper, StringMapWrapper} from 'angular2/src/core/facade/collection'; -import {isPresent, isFunction, StringWrapper} from 'angular2/src/core/facade/lang'; +import {isPresent, isFunction, StringWrapper, Type} from 'angular2/src/core/facade/lang'; import {DomAdapter} from './dom_adapter'; +import {XHRImpl} from 'angular2/src/core/compiler/xhr_impl'; + /** * Provides DOM operations in any browser environment. @@ -39,6 +41,8 @@ export abstract class GenericBrowserDomAdapter extends DomAdapter { this._transitionEnd = null; } } + + getXHR(): Type { return XHRImpl; } getDistributedNodes(el: HTMLElement): Node[] { return (el).getDistributedNodes(); } resolveAndSetHref(el: HTMLAnchorElement, baseUrl: string, href: string) { el.href = href == null ? baseUrl : baseUrl + '/../' + href; diff --git a/modules/angular2/src/core/dom/parse5_adapter.ts b/modules/angular2/src/core/dom/parse5_adapter.ts index 0f760e1834..a9b299b30b 100644 --- a/modules/angular2/src/core/dom/parse5_adapter.ts +++ b/modules/angular2/src/core/dom/parse5_adapter.ts @@ -11,11 +11,13 @@ import { isPresent, isBlank, global, + Type, setValueOnPath, DateWrapper } from 'angular2/src/core/facade/lang'; import {BaseException, WrappedException} from 'angular2/src/core/facade/exceptions'; import {SelectorMatcher, CssSelector} from 'angular2/src/core/compiler/selector'; +import {XHR} from 'angular2/src/core/compiler/xhr'; var _attrToPropMap: {[key: string]: string} = { 'class': 'className', @@ -61,6 +63,8 @@ export class Parse5DomAdapter extends DomAdapter { logGroupEnd() {} + getXHR(): Type { return XHR; } + get attrToPropMap() { return _attrToPropMap; } query(selector) { throw _notImplemented('query'); } diff --git a/modules/angular2/src/core/facade/async.dart b/modules/angular2/src/core/facade/async.dart index eee67827ec..09ca62ffb0 100644 --- a/modules/angular2/src/core/facade/async.dart +++ b/modules/angular2/src/core/facade/async.dart @@ -1,37 +1,9 @@ library angular2.core.facade.async; import 'dart:async'; -export 'dart:async' show Future, Stream, StreamController, StreamSubscription; +export 'dart:async' show Stream, StreamController, StreamSubscription; -class PromiseWrapper { - static Future resolve(obj) => new Future.value(obj); - - static Future reject(obj, stackTrace) => new Future.error(obj, - stackTrace != null ? stackTrace : obj is Error ? obj.stackTrace : null); - - static Future all(List promises) { - return Future - .wait(promises.map((p) => p is Future ? p : new Future.value(p))); - } - - static Future then(Future promise, success(value), [Function onError]) { - if (success == null) return promise.catchError(onError); - return promise.then(success, onError: onError); - } - - static Future wrap(Function fn) { - return new Future(fn); - } - - // Note: We can't rename this method to `catch`, as this is not a valid - // method name in Dart. - static Future catchError(Future promise, Function onError) { - return promise.catchError(onError); - } - - static PromiseCompleter completer() => - new PromiseCompleter(new Completer()); -} +export 'promise.dart'; class TimerWrapper { static Timer setTimeout(fn(), int millis) => @@ -105,22 +77,3 @@ class EventEmitter extends Stream { _controller.close(); } } - -class PromiseCompleter { - final Completer c; - - PromiseCompleter(this.c); - - Future get promise => c.future; - - void resolve(v) { - c.complete(v); - } - - void reject(error, stack) { - if (stack == null && error is Error) { - stack = error.stackTrace; - } - c.completeError(error, stack); - } -} diff --git a/modules/angular2/src/core/facade/async.ts b/modules/angular2/src/core/facade/async.ts index 3c9dca2b68..5a7600d74c 100644 --- a/modules/angular2/src/core/facade/async.ts +++ b/modules/angular2/src/core/facade/async.ts @@ -1,60 +1,11 @@ import {global, isPresent} from 'angular2/src/core/facade/lang'; +// We make sure promises are in a separate file so that we can use promises +// without depending on rxjs. +import {PromiseWrapper, Promise, PromiseCompleter} from 'angular2/src/core/facade/promise'; +export {PromiseWrapper, Promise, PromiseCompleter} from 'angular2/src/core/facade/promise'; // TODO(jeffbcross): use ES6 import once typings are available var Subject = require('@reactivex/rxjs/dist/cjs/Subject'); -export {Promise}; - -export interface PromiseCompleter { - promise: Promise; - resolve: (value?: R | PromiseLike) => void; - reject: (error?: any, stackTrace?: string) => void; -} - -export class PromiseWrapper { - static resolve(obj: T): Promise { return Promise.resolve(obj); } - - static reject(obj: any, _): Promise { return Promise.reject(obj); } - - // Note: We can't rename this method into `catch`, as this is not a valid - // method name in Dart. - static catchError(promise: Promise, - onError: (error: any) => T | PromiseLike): Promise { - return promise.catch(onError); - } - - static all(promises: any[]): Promise { - if (promises.length == 0) return Promise.resolve([]); - return Promise.all(promises); - } - - static then(promise: Promise, success: (value: T) => U | PromiseLike, - rejection?: (error: any, stack?: any) => U | PromiseLike): Promise { - return promise.then(success, rejection); - } - - static wrap(computation: () => T): Promise { - return new Promise((res, rej) => { - try { - res(computation()); - } catch (e) { - rej(e); - } - }); - } - - static completer(): PromiseCompleter { - var resolve; - var reject; - - var p = new Promise(function(res, rej) { - resolve = res; - reject = rej; - }); - - return {promise: p, resolve: resolve, reject: reject}; - } -} - export namespace NodeJS { export interface Timer {} } diff --git a/modules/angular2/src/core/facade/promise.dart b/modules/angular2/src/core/facade/promise.dart new file mode 100644 index 0000000000..53920b2e17 --- /dev/null +++ b/modules/angular2/src/core/facade/promise.dart @@ -0,0 +1,53 @@ +library angular2.core.facade.promise; + +import 'dart:async'; +export 'dart:async' show Future; + +class PromiseWrapper { + static Future resolve(obj) => new Future.value(obj); + + static Future reject(obj, stackTrace) => new Future.error(obj, + stackTrace != null ? stackTrace : obj is Error ? obj.stackTrace : null); + + static Future all(List promises) { + return Future + .wait(promises.map((p) => p is Future ? p : new Future.value(p))); + } + + static Future then(Future promise, success(value), [Function onError]) { + if (success == null) return promise.catchError(onError); + return promise.then(success, onError: onError); + } + + static Future wrap(Function fn) { + return new Future(fn); + } + + // Note: We can't rename this method to `catch`, as this is not a valid + // method name in Dart. + static Future catchError(Future promise, Function onError) { + return promise.catchError(onError); + } + + static PromiseCompleter completer() => + new PromiseCompleter(new Completer()); +} + +class PromiseCompleter { + final Completer c; + + PromiseCompleter(this.c); + + Future get promise => c.future; + + void resolve(v) { + c.complete(v); + } + + void reject(error, stack) { + if (stack == null && error is Error) { + stack = error.stackTrace; + } + c.completeError(error, stack); + } +} diff --git a/modules/angular2/src/core/facade/promise.ts b/modules/angular2/src/core/facade/promise.ts new file mode 100644 index 0000000000..1fcd901b8d --- /dev/null +++ b/modules/angular2/src/core/facade/promise.ts @@ -0,0 +1,54 @@ +// Promises are put into their own facade file so that they can be used without +// introducing a dependency on rxjs. They are re-exported through facade/async. +export {Promise}; + +export interface PromiseCompleter { + promise: Promise; + resolve: (value?: R | PromiseLike) => void; + reject: (error?: any, stackTrace?: string) => void; +} + +export class PromiseWrapper { + static resolve(obj: T): Promise { return Promise.resolve(obj); } + + static reject(obj: any, _): Promise { return Promise.reject(obj); } + + // Note: We can't rename this method into `catch`, as this is not a valid + // method name in Dart. + static catchError(promise: Promise, + onError: (error: any) => T | PromiseLike): Promise { + return promise.catch(onError); + } + + static all(promises: any[]): Promise { + if (promises.length == 0) return Promise.resolve([]); + return Promise.all(promises); + } + + static then(promise: Promise, success: (value: T) => U | PromiseLike, + rejection?: (error: any, stack?: any) => U | PromiseLike): Promise { + return promise.then(success, rejection); + } + + static wrap(computation: () => T): Promise { + return new Promise((res, rej) => { + try { + res(computation()); + } catch (e) { + rej(e); + } + }); + } + + static completer(): PromiseCompleter { + var resolve; + var reject; + + var p = new Promise(function(res, rej) { + resolve = res; + reject = rej; + }); + + return {promise: p, resolve: resolve, reject: reject}; + } +} diff --git a/modules/angular2/src/testing/test_injector.ts b/modules/angular2/src/testing/test_injector.ts index 3e2e30c18e..ce7494634a 100644 --- a/modules/angular2/src/testing/test_injector.ts +++ b/modules/angular2/src/testing/test_injector.ts @@ -115,7 +115,7 @@ function _getAppBindings() { PipeResolver, provide(ExceptionHandler, {useValue: new ExceptionHandler(DOM)}), provide(LocationStrategy, {useClass: MockLocationStrategy}), - XHR, + provide(XHR, {useClass: DOM.getXHR()}), TestComponentBuilder, provide(NgZone, {useClass: MockNgZone}), provide(AnimationBuilder, {useClass: MockAnimationBuilder}), diff --git a/modules/angular2/test/testing/testing_public_spec.ts b/modules/angular2/test/testing/testing_public_spec.ts index 5b356d0cab..4feb459bd8 100644 --- a/modules/angular2/test/testing/testing_public_spec.ts +++ b/modules/angular2/test/testing/testing_public_spec.ts @@ -16,6 +16,8 @@ import { import {Injectable, NgIf, bind} from 'angular2/core'; import {Directive, Component, View, ViewMetadata} from 'angular2/angular2'; +import {XHR} from 'angular2/src/core/compiler/xhr'; +import {XHRImpl} from 'angular2/src/core/compiler/xhr_impl'; // Services, and components for the tests. @@ -118,6 +120,9 @@ export function main() { }, 0); }); + it('provides a real XHR instance', + inject([XHR], (xhr) => { expect(xhr).toBeAnInstanceOf(XHRImpl); })); + describe('setting up Providers', () => { beforeEachProviders(() => [bind(FancyService).toValue(new FancyService())]); diff --git a/modules/angular2_material/test/button_spec.ts b/modules/angular2_material/test/button_spec.ts index a86a3eeb95..7f2027d895 100644 --- a/modules/angular2_material/test/button_spec.ts +++ b/modules/angular2_material/test/button_spec.ts @@ -20,9 +20,6 @@ import {MdButton, MdAnchor} from 'angular2_material/src/components/button/button import {TestUrlResolver} from './test_url_resolver'; -import {XHR} from 'angular2/src/core/compiler/xhr'; -import {XHRImpl} from 'angular2/src/core/compiler/xhr_impl'; - export function main() { describe('MdButton', () => { @@ -33,11 +30,6 @@ export function main() { // with both JS and Dart output. bind(UrlResolver) .toValue(new TestUrlResolver()), - - // Need to use the real XHR implementation (instead of the mock) so we can actually request - // the template files, since Angular 2 doesn't have anything like $templateCache. This should - // eventually be replaced with a preprocessor that inlines templates. - provide(XHR, {useClass: XHRImpl}) ]); beforeEach(inject([TestComponentBuilder], (tcb) => { builder = tcb; }));