Address feedback & handle interference with fakeAsync/async zone dele…

…gates

src/cdk/testing/testbed/BUILD.bazel

@@ -14,7 +14,6 @@ ts_library(
         "//src/cdk/testing",
         "@npm//@angular/core",
         "@npm//rxjs",
-        "@npm//zone.js",
     ],
 )
 

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.
-/// <reference types="zone.js" />
-
 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<TaskState>;
+/** Type that describes a potentially patched proxy zone instance. */
+type PatchedProxyZone = ProxyZone & {
+  [stateObservableSymbol]: undefined|Observable<TaskState>;
+};
 
+/**
+ * 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<TaskState>;
+  private _stateSubject: BehaviorSubject<TaskState> = new BehaviorSubject<TaskState>(
+      this._lastState ? this._getTaskStateFromInternalZoneState(this._lastState) : {stable: true});
 
-  constructor(public lastState: HasTaskState|null) {
-    this._stateSubject = new BehaviorSubject<TaskState>(lastState ?
-        this._getTaskStateFromInternalZoneState(lastState) : {stable: true});
-    this.state = this._stateSubject.asObservable();
-  }
+  /** Public observable that emits whenever the task state changes. */
+  readonly state: Observable<TaskState> = 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<TaskState> {
+    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;
+  }
 }

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.
-/// <reference types="zone.js" />
 
 /** A `HarnessEnvironment` implementation for Angular's Testbed. */
 export class TestbedHarnessEnvironment extends HarnessEnvironment<Element> {
@@ -29,7 +25,7 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment<Element> {
 
   protected constructor(rawRootElement: Element, private _fixture: ComponentFixture<unknown>) {
     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<Element> {
 
     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<Element> {
     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<TaskState> {
-    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;
-  }
 }

src/cdk/testing/testbed/zone-types.d.ts

@@ -6,17 +6,31 @@
  * 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.
-/// <reference types="zone.js" />
+/*
+ * 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;
 }
 
 export interface ProxyZone {
   lastTaskState: HasTaskState|null;
   setDelegate(spec: ZoneSpec): void;
   getDelegate(): ZoneSpec;
+  onHasTask(delegate: ZoneDelegate, current: Zone, target: Zone, hasTaskState: HasTaskState): void;
 }

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<void> {
     return (await this.input()).sendKeys({alt: true}, 'j');
   }
+
+  async getTaskStateResult(): Promise<string> {
+    await (await this.taskStateTestTrigger()).click();
+    return (await this.taskStateTestResult()).text();
+  }
 }

src/cdk/testing/tests/test-main-component.html

@@ -26,3 +26,10 @@ <h1 style="height: 100px; width: 200px;">Main Component</h1>
   <test-sub title="other 1"></test-sub>
   <test-sub title="other 2"></test-sub>
 </div>
+<div class="task-state-tests">
+  <button (click)="runTaskOutsideZone()" id="task-state-test-trigger">
+    Run task outside zone
+  </button>
+  <span id="task-state-result" #taskStateResult></span>
+</div>
+

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<HTMLElement>;
+  @ViewChild('taskStateResult', {static: false}) taskStateResultElement: ElementRef<HTMLElement>;
 
   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));
+  }
 }

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', () => {