fix(core): ComponentFixture stability should match ApplicationRef

This change aligns the stability of `ComponentFixture` with that of `ApplicationRef`, preventing confusing differences between the two as more APIs start using the `PendingTasks` that may not be tracked by `NgZone`. BREAKING CHANGE: `ComponentFixture.whenStable` now matches the `ApplicationRef.isStable` observable. Prior to this change, stability of the fixture did not include everything that was considered in `ApplicationRef`. `whenStable` of the fixture will now include unfinished router navigations and unfinished `HttpClient` requests. This will cause tests that `await` the `whenStable` promise to time out when there are incomplete requests. To fix this, remove the `whenStable`, instead wait for another condition, or ensure `HttpTestingController` mocks responses for all requests. Try adding `HttpTestingController.verify()` before your `await fixture.whenStable` to identify the open requests. In addition, `ComponentFixture.isStable` would synchronously switch to true in some scenarios but will now always be asynchronous.
angular · Mar 20, 2024 · 8a0b725 · 8a0b725
1 parent 314112d
commit 8a0b725
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 87 deletions.
diff --git a/goldens/public-api/core/testing/index.md b/goldens/public-api/core/testing/index.md
@@ -44,12 +44,12 @@ export abstract class ComponentFixture<T> {
     abstract detectChanges(checkNoChanges?: boolean): void;
     elementRef: ElementRef;
     getDeferBlocks(): Promise<DeferBlockFixture[]>;
-    abstract isStable(): boolean;
+    isStable(): boolean;
     nativeElement: any;
     // (undocumented)
     ngZone: NgZone | null;
     whenRenderingDone(): Promise<any>;
-    abstract whenStable(): Promise<any>;
+    whenStable(): Promise<any>;
 }
 
 // @public (undocumented)

diff --git a/packages/core/test/component_fixture_spec.ts b/packages/core/test/component_fixture_spec.ts
@@ -133,7 +133,6 @@ describe('ComponentFixture', () => {
     const element = componentFixture.debugElement.children[0];
     dispatchEvent(element.nativeElement, 'click');
 
-    expect(componentFixture.isStable()).toBe(true);
     expect(componentFixture.nativeElement).toHaveText('11');
   });
 

diff --git a/packages/core/testing/src/component_fixture.ts b/packages/core/testing/src/component_fixture.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ApplicationRef, ChangeDetectorRef, ComponentRef, DebugElement, ElementRef, getDebugNode, inject, NgZone, RendererFactory2, ViewRef, ɵDeferBlockDetails as DeferBlockDetails, ɵdetectChangesInViewIfRequired, ɵEffectScheduler as EffectScheduler, ɵgetDeferBlocks as getDeferBlocks, ɵNoopNgZone as NoopNgZone, ɵPendingTasks as PendingTasks,} from '@angular/core';
+import {ApplicationRef, ChangeDetectorRef, ComponentRef, DebugElement, ElementRef, getDebugNode, inject, NgZone, RendererFactory2, ViewRef, ɵDeferBlockDetails as DeferBlockDetails, ɵdetectChangesInViewIfRequired, ɵEffectScheduler as EffectScheduler, ɵgetDeferBlocks as getDeferBlocks, ɵNoopNgZone as NoopNgZone, ɵPendingTasks as PendingTasks} from '@angular/core';
 import {Subject, Subscription} from 'rxjs';
 import {first} from 'rxjs/operators';
 
@@ -62,6 +62,7 @@ export abstract class ComponentFixture<T> {
   protected readonly _appRef = inject(ApplicationRef);
   /** @internal */
   protected readonly _testAppRef = this._appRef as unknown as TestAppRef;
+  private readonly pendingTasks = inject(PendingTasks);
 
   // TODO(atscott): Remove this from public API
   ngZone = this._noZoneOptionIsSet ? null : this._ngZone;
@@ -99,15 +100,22 @@ export abstract class ComponentFixture<T> {
    * Return whether the fixture is currently stable or has async tasks that have not been completed
    * yet.
    */
-  abstract isStable(): boolean;
+  isStable(): boolean {
+    return !this.pendingTasks.hasPendingTasks.value;
+  }
 
   /**
    * Get a promise that resolves when the fixture is stable.
    *
    * This can be used to resume testing after events have triggered asynchronous activity or
    * asynchronous change detection.
    */
-  abstract whenStable(): Promise<any>;
+  whenStable(): Promise<any> {
+    if (this.isStable()) {
+      return Promise.resolve(false);
+    }
+    return this._appRef.isStable.pipe(first(stable => stable)).toPromise();
+  }
 
   /**
    * Retrieves all defer block fixtures in the component fixture.
@@ -165,7 +173,6 @@ export abstract class ComponentFixture<T> {
 export class ScheduledComponentFixture<T> extends ComponentFixture<T> {
   private readonly disableDetectChangesError =
       inject(AllowDetectChangesAndAcknowledgeItCanHideApplicationBugs, {optional: true}) ?? false;
-  private readonly pendingTasks = inject(PendingTasks);
 
   initialize(): void {
     this._appRef.attachView(this.componentRef.hostView);
@@ -188,17 +195,6 @@ export class ScheduledComponentFixture<T> extends ComponentFixture<T> {
     this._effectRunner.flush();
   }
 
-  override isStable(): boolean {
-    return !this.pendingTasks.hasPendingTasks.value;
-  }
-
-  override whenStable(): Promise<boolean> {
-    if (this.isStable()) {
-      return Promise.resolve(false);
-    }
-    return this._appRef.isStable.pipe(first((stable) => stable)).toPromise().then(() => true);
-  }
-
   override autoDetectChanges(autoDetect?: boolean|undefined): void {
     throw new Error('Cannot call autoDetectChanges when using change detection scheduling.');
   }
@@ -216,9 +212,6 @@ interface TestAppRef {
 export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
   private _subscriptions = new Subscription();
   private _autoDetect = inject(ComponentFixtureAutoDetect, {optional: true}) ?? false;
-  private _isStable: boolean = true;
-  private _promise: Promise<boolean>|null = null;
-  private _resolve: ((result: boolean) => void)|null = null;
   private afterTickSubscription: Subscription|undefined = undefined;
   private beforeRenderSubscription: Subscription|undefined = undefined;
 
@@ -232,36 +225,6 @@ export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
     // Create subscriptions outside the NgZone so that the callbacks run outside
     // of NgZone.
     this._ngZone.runOutsideAngular(() => {
-      this._subscriptions.add(
-          this._ngZone.onUnstable.subscribe({
-            next: () => {
-              this._isStable = false;
-            },
-          }),
-      );
-      this._subscriptions.add(
-          this._ngZone.onStable.subscribe({
-            next: () => {
-              this._isStable = true;
-              // Check whether there is a pending whenStable() completer to resolve.
-              if (this._promise !== null) {
-                // If so check whether there are no pending macrotasks before resolving.
-                // Do this check in the next tick so that ngZone gets a chance to update the state
-                // of pending macrotasks.
-                queueMicrotask(() => {
-                  if (!this._ngZone.hasPendingMacrotasks) {
-                    if (this._promise !== null) {
-                      this._resolve!(true);
-                      this._resolve = null;
-                      this._promise = null;
-                    }
-                  }
-                });
-              }
-            },
-          }),
-      );
-
       this._subscriptions.add(
           this._ngZone.onError.subscribe({
             next: (error: any) => {
@@ -287,23 +250,6 @@ export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
     this._effectRunner.flush();
   }
 
-  override isStable(): boolean {
-    return this._isStable && !this._ngZone.hasPendingMacrotasks;
-  }
-
-  override whenStable(): Promise<boolean> {
-    if (this.isStable()) {
-      return Promise.resolve(false);
-    } else if (this._promise !== null) {
-      return this._promise;
-    } else {
-      this._promise = new Promise((res) => {
-        this._resolve = res;
-      });
-      return this._promise;
-    }
-  }
-
   override autoDetectChanges(autoDetect = true): void {
     if (this._noZoneOptionIsSet) {
       throw new Error('Cannot call autoDetectChanges when ComponentFixtureNoNgZone is set.');

diff --git a/packages/forms/test/template_integration_spec.ts b/packages/forms/test/template_integration_spec.ts
@@ -267,7 +267,7 @@ describe('template-driven forms integration tests', () => {
          });
        }));
 
-    it('should set status classes involving nested FormGroups', () => {
+    it('should set status classes involving nested FormGroups', async () => {
       const fixture = initTest(NgModelNestedForm);
       fixture.componentInstance.first = '';
       fixture.componentInstance.other = '';
@@ -277,29 +277,28 @@ describe('template-driven forms integration tests', () => {
       const modelGroup = fixture.debugElement.query(By.css('[ngModelGroup]')).nativeElement;
       const input = fixture.debugElement.query(By.css('input')).nativeElement;
 
-      fixture.whenStable().then(() => {
-        fixture.detectChanges();
-        expect(sortedClassList(modelGroup)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
+      await fixture.whenStable();
+      fixture.detectChanges();
+      expect(sortedClassList(modelGroup)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
 
-        expect(sortedClassList(form)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
+      expect(sortedClassList(form)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
 
-        const formEl = fixture.debugElement.query(By.css('form')).nativeElement;
-        dispatchEvent(formEl, 'submit');
-        fixture.detectChanges();
+      const formEl = fixture.debugElement.query(By.css('form')).nativeElement;
+      dispatchEvent(formEl, 'submit');
+      fixture.detectChanges();
 
-        expect(sortedClassList(modelGroup)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
-        expect(sortedClassList(form)).toEqual([
-          'ng-pristine', 'ng-submitted', 'ng-untouched', 'ng-valid'
-        ]);
-        expect(sortedClassList(input)).not.toContain('ng-submitted');
+      expect(sortedClassList(modelGroup)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
+      expect(sortedClassList(form)).toEqual([
+        'ng-pristine', 'ng-submitted', 'ng-untouched', 'ng-valid'
+      ]);
+      expect(sortedClassList(input)).not.toContain('ng-submitted');
 
-        dispatchEvent(formEl, 'reset');
-        fixture.detectChanges();
+      dispatchEvent(formEl, 'reset');
+      fixture.detectChanges();
 
-        expect(sortedClassList(modelGroup)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
-        expect(sortedClassList(form)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
-        expect(sortedClassList(input)).not.toContain('ng-submitted');
-      });
+      expect(sortedClassList(modelGroup)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
+      expect(sortedClassList(form)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
+      expect(sortedClassList(input)).not.toContain('ng-submitted');
     });
 
     it('should not create a template-driven form when ngNoForm is used', () => {