From ddd5a235c39477970b8db8c37a1a63a82a389357 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Mon, 8 Jun 2015 18:07:06 -0700 Subject: [PATCH] test(change detect): Port change detect tests for mode More the change detect tests that exercise various detection modes to use the Dart pre-generated change detectors in addition to the `dynamic` and `JIT` change detectors. See #502 --- .../change_detector_codegen.dart | 2 +- .../change_detection/change_detector_spec.ts | 212 ++++++-------- .../change_detection/simple_watch_config.ts | 259 ++++++++++-------- 3 files changed, 235 insertions(+), 238 deletions(-) diff --git a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart index cd9621d069..4b1cfbec17 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -208,7 +208,7 @@ class _CodegenState { var detectorFieldNames = _genGetDetectorFieldNames(); for (var i = 0; i < detectorFieldNames.length; ++i) { buf.writeln('${detectorFieldNames[i]} = directives.getDetectorFor(' - '$_DIRECTIVES_ACCESSOR[$i].directiveIndex)'); + '$_DIRECTIVES_ACCESSOR[$i].directiveIndex);'); } return '$buf'; } diff --git a/modules/angular2/test/change_detection/change_detector_spec.ts b/modules/angular2/test/change_detection/change_detector_spec.ts index 43da9f33de..2069417227 100644 --- a/modules/angular2/test/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/change_detection/change_detector_spec.ts @@ -401,7 +401,95 @@ export function main() { }); }); - // TODO(kegluneq): Insert describe('mode', ...) testcases. + describe('mode', () => { + var _createWithoutHydrate = function(expression: string) { + var registry = null; + return _getProtoChangeDetector(getDefinition(expression).cdDef, registry).instantiate(new TestDispatcher()); + }; + + it('should set the mode to CHECK_ALWAYS when the default change detection is used', + () => { + var cd = _createWithoutHydrate('emptyUsingDefaultStrategy'); + expect(cd.mode).toEqual(null); + + cd.hydrate(_DEFAULT_CONTEXT, null, null); + expect(cd.mode).toEqual(CHECK_ALWAYS); + }); + + it('should set the mode to CHECK_ONCE when the push change detection is used', () => { + var cd = _createWithoutHydrate('emptyUsingOnPushStrategy'); + cd.hydrate(_DEFAULT_CONTEXT, null, null); + + expect(cd.mode).toEqual(CHECK_ONCE); + }); + + it('should not check a detached change detector', () => { + var val = _createChangeDetector('a', new TestData('value')); + + val.changeDetector.hydrate(_DEFAULT_CONTEXT, null, null); + val.changeDetector.mode = DETACHED; + val.changeDetector.detectChanges(); + + expect(val.dispatcher.log).toEqual([]); + }); + + it('should not check a checked change detector', () => { + var val = _createChangeDetector('a', new TestData('value')); + + val.changeDetector.hydrate(_DEFAULT_CONTEXT, null, null); + val.changeDetector.mode = CHECKED; + val.changeDetector.detectChanges(); + + expect(val.dispatcher.log).toEqual([]); + }); + + it('should change CHECK_ONCE to CHECKED', () => { + var cd = _createChangeDetector('10').changeDetector; + cd.hydrate(_DEFAULT_CONTEXT, null, null); + cd.mode = CHECK_ONCE; + + cd.detectChanges(); + + expect(cd.mode).toEqual(CHECKED); + }); + + it('should not change the CHECK_ALWAYS', () => { + var cd = _createChangeDetector('10').changeDetector; + cd.hydrate(_DEFAULT_CONTEXT, null, null); + cd.mode = CHECK_ALWAYS; + + cd.detectChanges(); + + expect(cd.mode).toEqual(CHECK_ALWAYS); + }); + + describe('marking ON_PUSH detectors as CHECK_ONCE after an update', () => { + var checkedDetector; + var directives; + + beforeEach(() => { + checkedDetector = _createWithoutHydrate('emptyUsingOnPushStrategy'); + checkedDetector.hydrate(_DEFAULT_CONTEXT, null, null); + checkedDetector.mode = CHECKED; + + var targetDirective = new TestData(null); + directives = new FakeDirectives([targetDirective], [checkedDetector]); + }); + + it('should set the mode to CHECK_ONCE when a binding is updated', () => { + var cd = _createWithoutHydrate('onPushRecordsUsingDefaultStrategy'); + cd.hydrate(_DEFAULT_CONTEXT, null, directives); + + expect(checkedDetector.mode).toEqual(CHECKED); + + // evaluate the record, update the targetDirective, and mark its detector as + // CHECK_ONCE + cd.detectChanges(); + + expect(checkedDetector.mode).toEqual(CHECK_ONCE); + }); + }); + }); describe('markPathToRootAsCheckOnce', () => { function changeDetector(mode, parent) { @@ -595,21 +683,6 @@ export function main() { function dirs(directives: List) { return new FakeDirectives(directives, []); } - function createChangeDetector(propName: string, exp: string, context = _DEFAULT_CONTEXT, - registry = null) { - var dispatcher = new TestDispatcher(); - - var locals = null; - var variableBindings = []; - - var records = [BindingRecord.createForElement(ast(exp), 0, propName)]; - var pcd = createProtoChangeDetector(records, variableBindings, [], registry); - var cd = pcd.instantiate(dispatcher); - cd.hydrate(context, locals, null); - - return {"changeDetector": cd, "dispatcher": dispatcher}; - } - describe(`${name} change detection`, () => { var dispatcher; @@ -879,113 +952,6 @@ export function main() { }); }); }); - - describe("mode", () => { - it("should set the mode to CHECK_ALWAYS when the default change detection is used", - () => { - var proto = createProtoChangeDetector([], [], [], null, DEFAULT); - var cd = proto.instantiate(null); - - expect(cd.mode).toEqual(null); - - cd.hydrate(_DEFAULT_CONTEXT, null, null); - - expect(cd.mode).toEqual(CHECK_ALWAYS); - }); - - it("should set the mode to CHECK_ONCE when the push change detection is used", () => { - var proto = createProtoChangeDetector([], [], [], null, ON_PUSH); - var cd = proto.instantiate(null); - cd.hydrate(_DEFAULT_CONTEXT, null, null); - - expect(cd.mode).toEqual(CHECK_ONCE); - }); - - it("should not check a detached change detector", () => { - var c = createChangeDetector('name', 'a', new TestData("value")); - var cd = c["changeDetector"]; - var dispatcher = c["dispatcher"]; - - cd.hydrate(_DEFAULT_CONTEXT, null, null); - cd.mode = DETACHED; - cd.detectChanges(); - - expect(dispatcher.log).toEqual([]); - }); - - it("should not check a checked change detector", () => { - var c = createChangeDetector('name', 'a', new TestData("value")); - var cd = c["changeDetector"]; - var dispatcher = c["dispatcher"]; - - cd.hydrate(_DEFAULT_CONTEXT, null, null); - cd.mode = CHECKED; - cd.detectChanges(); - - expect(dispatcher.log).toEqual([]); - }); - - it("should change CHECK_ONCE to CHECKED", () => { - var cd = createProtoChangeDetector([]).instantiate(null); - cd.hydrate(_DEFAULT_CONTEXT, null, null); - cd.mode = CHECK_ONCE; - - cd.detectChanges(); - - expect(cd.mode).toEqual(CHECKED); - }); - - it("should not change the CHECK_ALWAYS", () => { - var cd = createProtoChangeDetector([]).instantiate(null); - cd.hydrate(_DEFAULT_CONTEXT, null, null); - cd.mode = CHECK_ALWAYS; - - cd.detectChanges(); - - expect(cd.mode).toEqual(CHECK_ALWAYS); - }); - - describe("marking ON_PUSH detectors as CHECK_ONCE after an update", () => { - var checkedDetector; - var dirRecordWithOnPush; - var updateDirWithOnPushRecord; - var directives; - - beforeEach(() => { - var proto = createProtoChangeDetector([], [], [], null, ON_PUSH); - checkedDetector = proto.instantiate(null); - checkedDetector.hydrate(_DEFAULT_CONTEXT, null, null); - checkedDetector.mode = CHECKED; - - // this directive is a component with ON_PUSH change detection - dirRecordWithOnPush = new DirectiveRecord( - {directiveIndex: new DirectiveIndex(0, 0), changeDetection: ON_PUSH}); - - // a record updating a component - updateDirWithOnPushRecord = BindingRecord.createForDirective( - ast("42"), "a", (o, v) => o.a = v, dirRecordWithOnPush); - - var targetDirective = new TestData(null); - directives = new FakeDirectives([targetDirective], [checkedDetector]); - }); - - it("should set the mode to CHECK_ONCE when a binding is updated", () => { - var proto = createProtoChangeDetector([updateDirWithOnPushRecord], [], - [dirRecordWithOnPush]); - - var cd = proto.instantiate(null); - cd.hydrate(_DEFAULT_CONTEXT, null, directives); - - expect(checkedDetector.mode).toEqual(CHECKED); - - // evaluate the record, update the targetDirective, and mark its detector as - // CHECK_ONCE - cd.detectChanges(); - - expect(checkedDetector.mode).toEqual(CHECK_ONCE); - }); - }); - }); }); }); } diff --git a/modules/angular2/test/change_detection/simple_watch_config.ts b/modules/angular2/test/change_detection/simple_watch_config.ts index e970c33d5a..c60cb84f73 100644 --- a/modules/angular2/test/change_detection/simple_watch_config.ts +++ b/modules/angular2/test/change_detection/simple_watch_config.ts @@ -1,8 +1,12 @@ -import {ListWrapper, MapWrapper} from 'angular2/src/facade/collection'; +import {ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import {isBlank, isPresent} from 'angular2/src/facade/lang'; import { + DEFAULT, + ON_PUSH, BindingRecord, ChangeDetectorDefinition, + DirectiveIndex, + DirectiveRecord, Lexer, Locals, Parser @@ -34,26 +38,31 @@ export var PROP_NAME = 'propName'; * In this case, we expect `id` and `expression` to be the same string. */ export function getDefinition(id: string): TestDefinition { - var expression = null; - var locals = null; - if (MapWrapper.contains(_availableDefinitionsWithLocals, id)) { - var val = MapWrapper.get(_availableDefinitionsWithLocals, id); - expression = val.expression; - locals = val.locals; + var testDef = null; + if (StringMapWrapper.contains(_ExpressionWithLocals.availableDefinitions, id)) { + let val = StringMapWrapper.get(_ExpressionWithLocals.availableDefinitions, id); + let cdDef = val.createChangeDetectorDefinition(); + cdDef.id = id; + testDef = new TestDefinition(id, cdDef, val.locals); + } else if (StringMapWrapper.contains(_ExpressionWithMode.availableDefinitions, id)) { + let val = StringMapWrapper.get(_ExpressionWithMode.availableDefinitions, id); + let cdDef = val.createChangeDetectorDefinition(); + cdDef.id = id; + testDef = new TestDefinition(id, cdDef, null); } else if (ListWrapper.indexOf(_availableDefinitions, id) >= 0) { - expression = id; + var strategy = null; + var variableBindings = []; + var bindingRecords = _createBindingRecords(id); + var directiveRecords = []; + let cdDef = new ChangeDetectorDefinition(id, strategy, variableBindings, bindingRecords, + directiveRecords); + testDef = new TestDefinition(id, cdDef, null); } - if (isBlank(expression)) { + if (isBlank(testDef)) { throw `No ChangeDetectorDefinition for ${id} available. Please modify this file if necessary.`; } - var strategy = null; - var variableBindings = isPresent(locals) ? _convertLocalsToVariableBindings(locals) : []; - var bindingRecords = _createBindingRecords(expression); - var directiveRecords = []; - var cdDef = new ChangeDetectorDefinition(id, strategy, variableBindings, bindingRecords, - directiveRecords); - return new TestDefinition(id, cdDef, locals); + return testDef; } export class TestDefinition { @@ -68,106 +77,128 @@ export function getAllDefinitions(): List { return ListWrapper.map(_availableDefinitions, (id) => getDefinition(id)); } +class _ExpressionWithLocals { + constructor(private _expression: string, public locals: Locals) {} + + createChangeDetectorDefinition(): ChangeDetectorDefinition { + var strategy = null; + var variableBindings = _convertLocalsToVariableBindings(this.locals); + var bindingRecords = _createBindingRecords(this._expression); + var directiveRecords = []; + return new ChangeDetectorDefinition('(empty id)', strategy, variableBindings, bindingRecords, + directiveRecords); + } + + /** + * Map from test id to _ExpressionWithLocals. + * Tests in this map define an expression and local values which those expressions refer to. + */ + static availableDefinitions: StringMap = { + 'valueFromLocals': new _ExpressionWithLocals( + 'key', new Locals(null, MapWrapper.createFromPairs([['key', 'value']]))), + 'functionFromLocals': new _ExpressionWithLocals( + 'key()', new Locals(null, MapWrapper.createFromPairs([['key', () => 'value']]))), + 'nestedLocals': new _ExpressionWithLocals( + 'key', new Locals(new Locals(null, MapWrapper.createFromPairs([['key', 'value']])), + MapWrapper.create())), + 'fallbackLocals': new _ExpressionWithLocals( + 'name', new Locals(null, MapWrapper.createFromPairs([['key', 'value']]))), + 'contextNestedPropertyWithLocals': new _ExpressionWithLocals( + 'address.city', new Locals(null, MapWrapper.createFromPairs([['city', 'MTV']]))), + 'localPropertyWithSimilarContext': new _ExpressionWithLocals( + 'city', new Locals(null, MapWrapper.createFromPairs([['city', 'MTV']]))) + }; +} + +class _ExpressionWithMode { + constructor(private _strategy: string, private _withRecords: boolean) {} + + createChangeDetectorDefinition(): ChangeDetectorDefinition { + var variableBindings = []; + var bindingRecords = null; + var directiveRecords = null; + if (this._withRecords) { + var dirRecordWithOnPush = + new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0), changeDetection: ON_PUSH}); + var updateDirWithOnPushRecord = + BindingRecord.createForDirective(_parser.parseBinding('42', 'location'), 'a', + (o, v) => (o).a = v, dirRecordWithOnPush); + bindingRecords = [updateDirWithOnPushRecord]; + directiveRecords = [dirRecordWithOnPush]; + } else { + bindingRecords = []; + directiveRecords = []; + } + return new ChangeDetectorDefinition('(empty id)', this._strategy, variableBindings, + bindingRecords, directiveRecords); + } + + /** + * Map from test id to _ExpressionWithMode. + * Definitions in this map define conditions which allow testing various change detector modes. + */ + static availableDefinitions: StringMap = { + 'emptyUsingDefaultStrategy': new _ExpressionWithMode(DEFAULT, false), + 'emptyUsingOnPushStrategy': new _ExpressionWithMode(ON_PUSH, false), + 'onPushRecordsUsingDefaultStrategy': new _ExpressionWithMode(DEFAULT, true) + }; +} + /** * The list of all test definitions this config supplies. * Items in this list that do not appear in other structures define tests with expressions * equivalent to their ids. */ -var _availableDefinitions = [ - '10', - '"str"', - '"a\n\nb"', - '10 + 2', - '10 - 2', - '10 * 2', - '10 / 2', - '11 % 2', - '1 == 1', - '1 != 1', - '1 == true', - '1 === 1', - '1 !== 1', - '1 === true', - '1 < 2', - '2 < 1', - '1 > 2', - '2 > 1', - '1 <= 2', - '2 <= 2', - '2 <= 1', - '2 >= 1', - '2 >= 2', - '1 >= 2', - 'true && true', - 'true && false', - 'true || false', - 'false || false', - '!true', - '!!true', - '1 < 2 ? 1 : 2', - '1 > 2 ? 1 : 2', - '["foo", "bar"][0]', - '{"foo": "bar"}["foo"]', - 'name', - '[1, 2]', - '[1, a]', - '{z: 1}', - '{z: a}', - 'name | pipe', - 'value', - 'a', - 'address.city', - 'address?.city', - 'address?.toString()', - 'sayHi("Jim")', - 'a()(99)', - 'a.sayHi("Jim")', - 'valueFromLocals', - 'functionFromLocals', - 'nestedLocals', - 'fallbackLocals', - 'contextNestedPropertyWithLocals', - 'localPropertyWithSimilarContext' -]; - -class _ExpressionWithLocals { - constructor(public expression: string, public locals: Locals) {} -} - -/** - * Map from test id to _ExpressionWithLocals. - * Tests in this map define an expression and local values which those expressions refer to. - */ -var _availableDefinitionsWithLocals = MapWrapper.createFromPairs([ - [ - 'valueFromLocals', - new _ExpressionWithLocals('key', - new Locals(null, MapWrapper.createFromPairs([['key', 'value']]))) - ], - [ - 'functionFromLocals', - new _ExpressionWithLocals( - 'key()', new Locals(null, MapWrapper.createFromPairs([['key', () => 'value']]))) - ], - [ - 'nestedLocals', - new _ExpressionWithLocals( - 'key', new Locals(new Locals(null, MapWrapper.createFromPairs([['key', 'value']])), - MapWrapper.create())) - ], - [ - 'fallbackLocals', - new _ExpressionWithLocals('name', - new Locals(null, MapWrapper.createFromPairs([['key', 'value']]))) - ], - [ - 'contextNestedPropertyWithLocals', - new _ExpressionWithLocals('address.city', - new Locals(null, MapWrapper.createFromPairs([['city', 'MTV']]))) - ], - [ - 'localPropertyWithSimilarContext', - new _ExpressionWithLocals('city', - new Locals(null, MapWrapper.createFromPairs([['city', 'MTV']]))) - ] -]); +var _availableDefinitions = ListWrapper.concat( + [ + '10', + '"str"', + '"a\n\nb"', + '10 + 2', + '10 - 2', + '10 * 2', + '10 / 2', + '11 % 2', + '1 == 1', + '1 != 1', + '1 == true', + '1 === 1', + '1 !== 1', + '1 === true', + '1 < 2', + '2 < 1', + '1 > 2', + '2 > 1', + '1 <= 2', + '2 <= 2', + '2 <= 1', + '2 >= 1', + '2 >= 2', + '1 >= 2', + 'true && true', + 'true && false', + 'true || false', + 'false || false', + '!true', + '!!true', + '1 < 2 ? 1 : 2', + '1 > 2 ? 1 : 2', + '["foo", "bar"][0]', + '{"foo": "bar"}["foo"]', + 'name', + '[1, 2]', + '[1, a]', + '{z: 1}', + '{z: a}', + 'name | pipe', + 'value', + 'a', + 'address.city', + 'address?.city', + 'address?.toString()', + 'sayHi("Jim")', + 'a()(99)', + 'a.sayHi("Jim")' + ], + ListWrapper.concat(StringMapWrapper.keys(_ExpressionWithLocals.availableDefinitions), + StringMapWrapper.keys(_ExpressionWithMode.availableDefinitions)));