refactor(core): Private option to rethrow ApplicationRef.tick errors in tests (angular#57153)

This creates a private option that can be used internally while we
migrate this to the default and only behavior. ~200 tests in TGP have errors
that are being swallowed (console.log) and not causing the test to fail.
We can first explicitly opt those out, flip the default internally, then
"fix" them by adding expect...toThrow.

PR Close angular#57153

Author: atscott

packages/core/test/component_fixture_spec.ts

@@ -13,7 +13,6 @@ import {
   ErrorHandler,
   Injectable,
   Input,
-  NgZone,
   createComponent,
   provideExperimentalZonelessChangeDetection,
   signal,
@@ -352,14 +351,7 @@ describe('ComponentFixture', () => {
     expect(() => fixture.detectChanges()).toThrow();
   });
 
-  // note: this test only verifies existing behavior was not broken by a change to the zoneless fixture.
-  // We probably do want the whenStable promise to be rejected. The current zone-based fixture is bad
-  // and confusing for two reason:
-  //  1. with autoDetect, errors in the fixture _cannot be handled_ with whenStable because
-  //  they're just thrown inside the rxjs subcription (and then goes to setTimeout(() => throw e))
-  //  2. errors from other views attached to ApplicationRef just go to the ErrorHandler, which by default
-  //  only logs to console, allowing the test to pass
-  it('resolves whenStable promise when errors happen during appRef.tick', async () => {
+  describe('errors during ApplicationRef.tick', () => {
     @Component({
       template: '',
       standalone: true,
@@ -375,13 +367,33 @@ describe('ComponentFixture', () => {
     })
     class Blank {}
 
-    const fixture = TestBed.createComponent(Blank);
-    const throwingThing = createComponent(ThrowingThing, {
-      environmentInjector: TestBed.inject(EnvironmentInjector),
+    // note: this test only verifies existing behavior was not broken by a change to the zoneless fixture.
+    // We probably do want the whenStable promise to be rejected. The current zone-based fixture is bad
+    // and confusing for two reason:
+    //  1. with autoDetect, errors in the fixture _cannot be handled_ with whenStable because
+    //  they're just thrown inside the rxjs subcription (and then goes to setTimeout(() => throw e))
+    //  2. errors from other views attached to ApplicationRef just go to the ErrorHandler, which by default
+    //  only logs to console, allowing the test to pass
+    it('resolves whenStable promise when errors happen during appRef.tick', async () => {
+      const fixture = TestBed.createComponent(Blank);
+      const throwingThing = createComponent(ThrowingThing, {
+        environmentInjector: TestBed.inject(EnvironmentInjector),
+      });
+
+      TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
+      await expectAsync(fixture.whenStable()).toBeResolved();
     });
 
-    TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
-    await expectAsync(fixture.whenStable()).toBeResolved();
+    it('can opt-in to rethrowing application errors and rejecting whenStable promises', async () => {
+      TestBed.configureTestingModule({_rethrowApplicationTickErrors: true} as any);
+      const fixture = TestBed.createComponent(Blank);
+      const throwingThing = createComponent(ThrowingThing, {
+        environmentInjector: TestBed.inject(EnvironmentInjector),
+      });
+
+      TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
+      await expectAsync(fixture.whenStable()).toBeRejected();
+    });
   });
 
   describe('defer', () => {

packages/core/testing/src/application_error_handler.ts

@@ -6,31 +6,17 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {
-  ErrorHandler,
-  inject,
-  NgZone,
-  Injectable,
-  ɵZONELESS_ENABLED as ZONELESS_ENABLED,
-} from '@angular/core';
+import {ErrorHandler, inject, NgZone, Injectable, InjectionToken} from '@angular/core';
+
+export const RETHROW_APPLICATION_ERRORS = new InjectionToken<boolean>('rethrow application errors');
 
 @Injectable()
 export class TestBedApplicationErrorHandler {
   private readonly zone = inject(NgZone);
   private readonly userErrorHandler = inject(ErrorHandler);
-  private readonly zoneless = inject(ZONELESS_ENABLED);
   readonly whenStableRejectFunctions: Set<(e: unknown) => void> = new Set();
 
   handleError(e: unknown) {
-    // TODO(atscott): Investigate if we can align the behaviors of zone and zoneless
-    if (this.zoneless) {
-      this.zonelessHandleError(e);
-    } else {
-      this.zone.runOutsideAngular(() => this.userErrorHandler.handleError(e));
-    }
-  }
-
-  private zonelessHandleError(e: unknown) {
     try {
       this.zone.runOutsideAngular(() => this.userErrorHandler.handleError(e));
     } catch (userError: unknown) {

packages/core/testing/src/component_fixture.ts

@@ -81,10 +81,7 @@ export abstract class ComponentFixture<T> {
   /** @internal */
   protected readonly _testAppRef = this._appRef as unknown as TestAppRef;
   private readonly pendingTasks = inject(PendingTasks);
-  /** @internal */
-  protected readonly _appErrorHandler = inject(TestBedApplicationErrorHandler);
-  /** @internal */
-  protected _rejectWhenStablePromiseOnAppError = true;
+  private readonly appErrorHandler = inject(TestBedApplicationErrorHandler);
 
   // TODO(atscott): Remove this from public API
   ngZone = this._noZoneOptionIsSet ? null : this._ngZone;
@@ -137,7 +134,16 @@ export abstract class ComponentFixture<T> {
       return Promise.resolve(false);
     }
 
-    return this._appRef.isStable.pipe(first((stable) => stable)).toPromise();
+    return new Promise((resolve, reject) => {
+      this.appErrorHandler.whenStableRejectFunctions.add(reject);
+      this._appRef.isStable
+        .pipe(first((stable) => stable))
+        .toPromise()
+        .then((v) => {
+          this.appErrorHandler.whenStableRejectFunctions.delete(reject);
+          resolve(v);
+        });
+    });
   }
 
   /**
@@ -200,16 +206,6 @@ export class ScheduledComponentFixture<T> extends ComponentFixture<T> {
     }
   }
 
-  override whenStable(): Promise<any> {
-    return new Promise((resolve, reject) => {
-      this._appErrorHandler.whenStableRejectFunctions.add(reject);
-      super.whenStable().then((v) => {
-        this._appErrorHandler.whenStableRejectFunctions.delete(reject);
-        resolve(v);
-      });
-    });
-  }
-
   override detectChanges(checkNoChanges = true): void {
     if (!checkNoChanges) {
       throw new Error(
@@ -252,11 +248,6 @@ export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
   private beforeRenderSubscription: Subscription | undefined = undefined;
 
   initialize(): void {
-    // TODO(atscott): Determine whether we can align this behavior with the zoneless fixture.
-    // This exists to keep the previous zone-based fixture behavior consistent with how it was before.
-    // However, we currently feel that the zoneless fixture is doing the more correct thing.
-    this._rejectWhenStablePromiseOnAppError = false;
-
     if (this._autoDetect) {
       this.subscribeToAppRefEvents();
     }

packages/core/testing/src/test_bed_common.ts

@@ -73,6 +73,9 @@ export interface TestModuleMetadata {
    * Defaults to `manual`.
    */
   deferBlockBehavior?: DeferBlockBehavior;
+
+  /** @internal */
+  _rethrowApplicationTickErrors?: boolean;
 }
 
 /**

packages/core/testing/src/test_bed_compiler.ts

@@ -15,7 +15,6 @@ import {
   Compiler,
   COMPILER_OPTIONS,
   Component,
-  ErrorHandler,
   Directive,
   Injector,
   inject,
@@ -39,7 +38,6 @@ import {
   ɵcompilePipe as compilePipe,
   ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID,
   ɵDEFER_BLOCK_CONFIG as DEFER_BLOCK_CONFIG,
-  ɵDeferBlockBehavior as DeferBlockBehavior,
   ɵdepsTracker as depsTracker,
   ɵDirectiveDef as DirectiveDef,
   ɵgenerateStandaloneInDeclarationsError,
@@ -67,6 +65,7 @@ import {
   ɵUSE_RUNTIME_DEPS_TRACKER_FOR_JIT as USE_RUNTIME_DEPS_TRACKER_FOR_JIT,
   ɵɵInjectableDeclaration as InjectableDeclaration,
   NgZone,
+  ErrorHandler,
 } from '@angular/core';
 
 import {ComponentDef, ComponentType} from '../../src/render3';
@@ -80,7 +79,10 @@ import {
   Resolver,
 } from './resolvers';
 import {DEFER_BLOCK_DEFAULT_BEHAVIOR, TestModuleMetadata} from './test_bed_common';
-import {TestBedApplicationErrorHandler} from './application_error_handler';
+import {
+  RETHROW_APPLICATION_ERRORS,
+  TestBedApplicationErrorHandler,
+} from './application_error_handler';
 
 enum TestingModuleOverride {
   DECLARATION,
@@ -224,6 +226,10 @@ export class TestBedCompiler {
     if (moduleDef.providers !== undefined) {
       this.providers.push(...moduleDef.providers);
     }
+    this.providers.push({
+      provide: RETHROW_APPLICATION_ERRORS,
+      useValue: moduleDef._rethrowApplicationTickErrors ?? false,
+    });
 
     if (moduleDef.schemas !== undefined) {
       this.schemas.push(...moduleDef.schemas);
@@ -941,10 +947,17 @@ export class TestBedCompiler {
         {
           provide: INTERNAL_APPLICATION_ERROR_HANDLER,
           useFactory: () => {
-            const handler = inject(TestBedApplicationErrorHandler);
-            return (e: unknown) => {
-              handler.handleError(e);
-            };
+            if (inject(ZONELESS_ENABLED) || inject(RETHROW_APPLICATION_ERRORS, {optional: true})) {
+              const handler = inject(TestBedApplicationErrorHandler);
+              return (e: unknown) => {
+                handler.handleError(e);
+              };
+            } else {
+              const userErrorHandler = inject(ErrorHandler);
+              const ngZone = inject(NgZone);
+              return (e: unknown) =>
+                ngZone.runOutsideAngular(() => userErrorHandler.handleError(e));
+            }
           },
         },
         {provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl},