From 22f5294d689d89ffca3c6ffb74531d42a14364de Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 16 Oct 2019 14:49:24 +0200 Subject: [PATCH] Address feedback & handle interference with fakeAsync/async zone delegates --- src/cdk/testing/testbed/BUILD.bazel | 1 - .../testbed/task-state-zone-interceptor.ts | 96 +++++++++++++++---- .../testbed/testbed-harness-environment.ts | 67 +++---------- src/cdk/testing/testbed/zone-types.d.ts | 24 ++++- .../tests/harnesses/main-component-harness.ts | 8 ++ .../testing/tests/test-main-component.html | 7 ++ src/cdk/testing/tests/test-main-component.ts | 10 +- src/cdk/testing/tests/testbed.spec.ts | 17 +++- 8 files changed, 147 insertions(+), 83 deletions(-) diff --git a/src/cdk/testing/testbed/BUILD.bazel b/src/cdk/testing/testbed/BUILD.bazel index 33bb1288548a..9e31ea8657e2 100644 --- a/src/cdk/testing/testbed/BUILD.bazel +++ b/src/cdk/testing/testbed/BUILD.bazel @@ -14,7 +14,6 @@ ts_library( "//src/cdk/testing", "@npm//@angular/core", "@npm//rxjs", - "@npm//zone.js", ], ) diff --git a/src/cdk/testing/testbed/task-state-zone-interceptor.ts b/src/cdk/testing/testbed/task-state-zone-interceptor.ts index 6f11cf9f58cc..41fea210a6eb 100644 --- a/src/cdk/testing/testbed/task-state-zone-interceptor.ts +++ b/src/cdk/testing/testbed/task-state-zone-interceptor.ts @@ -6,38 +6,39 @@ * found in the LICENSE file at https://angular.io/license */ -// Ensures that TypeScript knows that the global "zone.js" types -// are used in this source file. -/// - import {BehaviorSubject, Observable} from 'rxjs'; +import {ProxyZone, ProxyZoneStatic, HasTaskState, Zone, ZoneDelegate} from './zone-types'; +/** Current state of the intercepted zone. */ export interface TaskState { + /** Whether the zone is stable (i.e. no microtasks and macrotasks). */ stable: boolean; } -export class TaskStateZoneInterceptor implements ZoneSpec { - /** Name of the custom zone. */ - readonly name = 'cdk-testing-testbed-task-state-zone'; +/** Unique symbol that is used to patch a property to a proxy zone. */ +const stateObservableSymbol = Symbol('ProxyZone_PATCHED#stateObservable'); - /** Public observable that emits whenever the task state changes. */ - readonly state: Observable; +/** Type that describes a potentially patched proxy zone instance. */ +type PatchedProxyZone = ProxyZone & { + [stateObservableSymbol]: undefined|Observable; +}; +/** + * Interceptor that can be set up in a `ProxyZone` instance. The interceptor + * will keep track of the task state and emit whenever the state changes. + */ +export class TaskStateZoneInterceptor { /** Subject that can be used to emit a new state change. */ - private _stateSubject: BehaviorSubject; + private _stateSubject: BehaviorSubject = new BehaviorSubject( + this._lastState ? this._getTaskStateFromInternalZoneState(this._lastState) : {stable: true}); - constructor(public lastState: HasTaskState|null) { - this._stateSubject = new BehaviorSubject(lastState ? - this._getTaskStateFromInternalZoneState(lastState) : {stable: true}); - this.state = this._stateSubject.asObservable(); - } + /** Public observable that emits whenever the task state changes. */ + readonly state: Observable = this._stateSubject.asObservable(); - /** - * Implemented as part of "ZoneSpec". This will emit whenever the internal - * ZoneJS task state changes. - */ + constructor(private _lastState: HasTaskState|null) {} + + /** This will be called whenever the task state changes in the intercepted zone. */ onHasTask(delegate: ZoneDelegate, current: Zone, target: Zone, hasTaskState: HasTaskState) { - delegate.hasTask(target, hasTaskState); if (current === target) { this._stateSubject.next(this._getTaskStateFromInternalZoneState(hasTaskState)); } @@ -47,4 +48,59 @@ export class TaskStateZoneInterceptor implements ZoneSpec { private _getTaskStateFromInternalZoneState(state: HasTaskState): TaskState { return {stable: !state.macroTask && !state.microTask}; } + + /** + * Sets up the custom task state Zone interceptor in the `ProxyZone`. Throws if + * no `ProxyZone` could be found. + * @returns an observable that emits whenever the task state changes. + */ + static setup(): Observable { + if (Zone === undefined) { + throw Error('Could not find ZoneJS. For test harnesses running in TestBed, ' + + 'ZoneJS needs to be installed.'); + } + + // tslint:disable-next-line:variable-name + const ProxyZoneSpec = (Zone as any)['ProxyZoneSpec'] as ProxyZoneStatic|undefined; + + // If there is no "ProxyZoneSpec" installed, we throw an error and recommend + // setting up the proxy zone by pulling in the testing bundle. + if (!ProxyZoneSpec) { + throw Error( + 'ProxyZoneSpec is needed for the test harnesses but could not be found. ' + + 'Please make sure that your environment includes zone.js/dist/zone-testing.js'); + } + + // Ensure that there is a proxy zone instance set up, and get + // a reference to the instance if present. + const zoneSpec = ProxyZoneSpec.assertPresent() as PatchedProxyZone; + + // If there already is a delegate registered in the proxy zone, and it + // is type of the custom task state interceptor, we just use that state + // observable. This allows us to only intercept Zone once per test + // (similar to how `fakeAsync` or `async` work). + if (zoneSpec[stateObservableSymbol]) { + return zoneSpec[stateObservableSymbol]!; + } + + // Since we intercept on environment creation and the fixture has been + // created before, we might have missed tasks scheduled before. Fortunately + // the proxy zone keeps track of the previous task state, so we can just pass + // this as initial state to the task zone interceptor. + const interceptor = new TaskStateZoneInterceptor(zoneSpec.lastTaskState); + const zoneSpecOnHasTask = zoneSpec.onHasTask; + + // We setup the task state interceptor in the `ProxyZone`. Note that we cannot register + // the interceptor as a new proxy zone delegate because it would mean that other zone + // delegates (e.g. `FakeAsyncTestZone` or `AsyncTestZone`) can accidentally overwrite/disable + // our interceptor. Since we just intend to monitor the task state of the proxy zone, it is + // sufficient to just patch the proxy zone. This also avoids that we interfere with the task + // queue scheduling logic. + zoneSpec.onHasTask = function() { + zoneSpecOnHasTask.apply(zoneSpec, arguments); + interceptor.onHasTask.apply(interceptor, arguments); + }; + + return zoneSpec[stateObservableSymbol] = interceptor.state; + } } diff --git a/src/cdk/testing/testbed/testbed-harness-environment.ts b/src/cdk/testing/testbed/testbed-harness-environment.ts index ed3b8906ee6a..c0d0677b8881 100644 --- a/src/cdk/testing/testbed/testbed-harness-environment.ts +++ b/src/cdk/testing/testbed/testbed-harness-environment.ts @@ -7,18 +7,14 @@ */ import {HarnessEnvironment} from '@angular/cdk/testing'; -import {ComponentFixture} from '@angular/core/testing'; +import {ComponentFixture, flush} from '@angular/core/testing'; import {Observable} from 'rxjs'; import {takeWhile} from 'rxjs/operators'; import {ComponentHarness, ComponentHarnessConstructor, HarnessLoader} from '../component-harness'; import {TestElement} from '../test-element'; import {TaskState, TaskStateZoneInterceptor} from './task-state-zone-interceptor'; import {UnitTestElement} from './unit-test-element'; -import {ProxyZoneType} from './zone-types'; -// Ensures that TypeScript knows that the global "zone.js" types -// are used in this source file. -/// /** A `HarnessEnvironment` implementation for Angular's Testbed. */ export class TestbedHarnessEnvironment extends HarnessEnvironment { @@ -29,7 +25,7 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment { protected constructor(rawRootElement: Element, private _fixture: ComponentFixture) { super(rawRootElement); - this._taskState = this._setupZoneTaskStateInterceptor(); + this._taskState = TaskStateZoneInterceptor.setup(); _fixture.componentRef.onDestroy(() => this._destroyed = true); } @@ -66,6 +62,16 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment { this._fixture.detectChanges(); + // If we run in the fake async zone, we run "flush" to run any scheduled tasks. This + // ensures that the harnesses behave inside of the FakeAsyncTestZone similar to the + // "AsyncTestZone" and the root zone (i.e. neither fakeAsync or async). Note that we + // cannot just rely on the task state observable to become stable because the state will + // never change. This is because the task queue will be only drained if the fake async + // zone is being flushed. + if (Zone!.current.get('FakeAsyncTestZoneSpec')) { + flush(); + } + // Wait until the task queue has been drained and the zone is stable. Note that // we cannot rely on "fixture.whenStable" since it does not catch tasks scheduled // outside of the Angular zone. For test harnesses, we want to ensure that the @@ -89,53 +95,4 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment { await this.forceStabilize(); return Array.from(this.rawRootElement.querySelectorAll(selector)); } - - /** - * Sets up the custom task state ZoneJS interceptor. - * @returns an observable that emits whenever the task state changes. - */ - private _setupZoneTaskStateInterceptor(): Observable { - if (Zone === undefined) { - throw Error('Could not find ZoneJS. For test harnesses running in TestBed, ' + - 'ZoneJS needs to be installed.'); - } - - // tslint:disable-next-line:variable-name - const ProxyZoneSpec = (Zone as any)['ProxyZoneSpec'] as ProxyZoneType|undefined; - - // If there is no "ProxyZoneSpec" installed, we throw an error and recommend - // setting up the proxy zone by pulling in the testing bundle. - if (!ProxyZoneSpec) { - throw Error( - 'ProxyZoneSpec is needed for the test harnesses but could not be found. ' + - 'Please make sure that your environment includes zone.js/dist/zone-testing.js'); - } - - // Ensure that there is a proxy zone instance set up. - ProxyZoneSpec.assertPresent(); - - // Get the instance of the proxy zone spec. - const zoneSpec = ProxyZoneSpec.get(); - const currentDelegate = zoneSpec.getDelegate(); - - // If there already is a delegate registered in the proxy zone, and it - // is type of the custom task state interceptor, we just use that state - // observable. This allows us to only intercept Zone once per test - // (similar to how `fakeAsync` or `async` work). - if (currentDelegate && currentDelegate instanceof TaskStateZoneInterceptor) { - return currentDelegate.state; - } - - // Since we intercept on environment creation and the fixture has been - // created before, we might have missed tasks scheduled before. Fortunately - // the proxy zone keeps track of the previous task state, so we can just pass - // this as initial state to the task zone interceptor. - const newZone = new TaskStateZoneInterceptor(zoneSpec.lastTaskState); - - // Set the new delegate. This means that the "ProxyZone" will delegate - // all Zone information to the custom task zone interceptor. - zoneSpec.setDelegate(newZone); - - return newZone.state; - } } diff --git a/src/cdk/testing/testbed/zone-types.d.ts b/src/cdk/testing/testbed/zone-types.d.ts index 0fb4d40ef917..00427092e33b 100644 --- a/src/cdk/testing/testbed/zone-types.d.ts +++ b/src/cdk/testing/testbed/zone-types.d.ts @@ -6,12 +6,25 @@ * found in the LICENSE file at https://angular.io/license */ -// Ensures that TypeScript knows that the global "zone.js" types -// are used in this source file. -/// +/* + * Type definitions for "zone.js". We cannot reference the official types + * using a triple-slash types directive because the types would bring in + * the NodeJS types into the compilation unit. This would cause unexpected + * type checking failures. We just create minimal type definitions for Zone + * here and use these for our interceptor logic. + */ + +declare global { + // tslint:disable-next-line:variable-name + const Zone: {current: any}|undefined; +} + +export type Zone = Object; +export type ZoneDelegate = Object; +export type HasTaskState = {microTask: boolean, macroTask: boolean}; -export interface ProxyZoneType { - assertPresent: () => void; +export interface ProxyZoneStatic { + assertPresent: () => ProxyZone; get(): ProxyZone; } @@ -19,4 +32,5 @@ export interface ProxyZone { lastTaskState: HasTaskState|null; setDelegate(spec: ZoneSpec): void; getDelegate(): ZoneSpec; + onHasTask(delegate: ZoneDelegate, current: Zone, target: Zone, hasTaskState: HasTaskState): void; } diff --git a/src/cdk/testing/tests/harnesses/main-component-harness.ts b/src/cdk/testing/tests/harnesses/main-component-harness.ts index db222a45311d..b6ade34b71ec 100644 --- a/src/cdk/testing/tests/harnesses/main-component-harness.ts +++ b/src/cdk/testing/tests/harnesses/main-component-harness.ts @@ -42,6 +42,9 @@ export class MainComponentHarness extends ComponentHarness { readonly optionalSubComponent = this.locatorForOptional(SubComponentHarness); readonly errorSubComponent = this.locatorFor(WrongComponentHarness); + readonly taskStateTestTrigger = this.locatorFor('#task-state-test-trigger'); + readonly taskStateTestResult = this.locatorFor('#task-state-result'); + readonly fourItemLists = this.locatorForAll(SubComponentHarness.with({itemCount: 4})); readonly toolsLists = this.locatorForAll(SubComponentHarness.with({title: 'List of test tools'})); readonly fourItemToolsLists = @@ -96,4 +99,9 @@ export class MainComponentHarness extends ComponentHarness { async sendAltJ(): Promise { return (await this.input()).sendKeys({alt: true}, 'j'); } + + async getTaskStateResult(): Promise { + await (await this.taskStateTestTrigger()).click(); + return (await this.taskStateTestResult()).text(); + } } diff --git a/src/cdk/testing/tests/test-main-component.html b/src/cdk/testing/tests/test-main-component.html index 2f35635a077b..94f5241011ac 100644 --- a/src/cdk/testing/tests/test-main-component.html +++ b/src/cdk/testing/tests/test-main-component.html @@ -26,3 +26,10 @@

Main Component

+
+ + +
+ diff --git a/src/cdk/testing/tests/test-main-component.ts b/src/cdk/testing/tests/test-main-component.ts index 371f1bc56c02..bb3ac48613d1 100644 --- a/src/cdk/testing/tests/test-main-component.ts +++ b/src/cdk/testing/tests/test-main-component.ts @@ -12,6 +12,7 @@ import { ChangeDetectorRef, Component, ElementRef, + NgZone, OnDestroy, ViewChild, ViewEncapsulation @@ -44,6 +45,7 @@ export class TestMainComponent implements OnDestroy { relativeY = 0; @ViewChild('clickTestElement', {static: false}) clickTestElement: ElementRef; + @ViewChild('taskStateResult', {static: false}) taskStateResultElement: ElementRef; private _fakeOverlayElement: HTMLElement; @@ -55,7 +57,7 @@ export class TestMainComponent implements OnDestroy { this._isHovering = false; } - constructor(private _cdr: ChangeDetectorRef) { + constructor(private _cdr: ChangeDetectorRef, private _zone: NgZone) { this.username = 'Yi'; this.counter = 0; this.asyncCounter = 0; @@ -99,4 +101,10 @@ export class TestMainComponent implements OnDestroy { this.relativeX = Math.round(event.clientX - left); this.relativeY = Math.round(event.clientY - top); } + + runTaskOutsideZone() { + this._zone.runOutsideAngular(() => setTimeout(() => { + this.taskStateResultElement.nativeElement.textContent = 'result'; + }, 100)); + } } diff --git a/src/cdk/testing/tests/testbed.spec.ts b/src/cdk/testing/tests/testbed.spec.ts index 41cb05855aed..fc30ef818eb4 100644 --- a/src/cdk/testing/tests/testbed.spec.ts +++ b/src/cdk/testing/tests/testbed.spec.ts @@ -1,6 +1,6 @@ import {HarnessLoader} from '@angular/cdk/testing'; import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed'; -import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {async, ComponentFixture, fakeAsync, TestBed} from '@angular/core/testing'; import {FakeOverlayHarness} from './harnesses/fake-overlay-harness'; import {MainComponentHarness} from './harnesses/main-component-harness'; import {SubComponentHarness} from './harnesses/sub-component-harness'; @@ -252,6 +252,21 @@ describe('TestbedHarnessEnvironment', () => { const subcomps = await harness.directAncestorSelectorSubcomponent(); expect(subcomps.length).toBe(2); }); + + it('should respect tasks outside of NgZone when stabilizing within native async/await', + async () => { + expect(await harness.getTaskStateResult()).toBe('result'); + }); + + it('should respect tasks outside of NgZone when stabilizing within async test zone', + async (() => { + harness.getTaskStateResult().then(res => expect(res).toBe('result')); + })); + + it('should respect tasks outside of NgZone when stabilizing within fakeAsync test zone', + fakeAsync(async () => { + expect(await harness.getTaskStateResult()).toBe('result'); + })); }); describe('TestElement', () => {