fix(core): ComponentFixture stability should match ApplicationRef (angular#54949)

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.
Also, make sure your tests wait for the stability promise. We found many
examples of tests that did not, meaning the expectations did not execute
within the test body.

In addition, `ComponentFixture.isStable` would synchronously switch to
true in some scenarios but will now always be asynchronous.

PR Close angular#54949

Author: atscott

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)

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');
   });
 

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.');

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