From 971981e1dfec7dffa1ae1ceff2bea0670406da21 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 15 May 2025 16:40:46 -0700 Subject: [PATCH] fix(core): TestBed.tick should ensure test components are synchronized (#61382) This ensures that `TestBed.tick` updates any components created with `TestBed.createComponent`, regardless of whether autoDetectChanges is on. PR Close #61382 --- .../core/src/application/application_ref.ts | 9 ++- .../scheduling/ng_zone_scheduling.ts | 5 +- packages/core/test/render3/reactivity_spec.ts | 20 +++---- packages/core/test/test_bed_spec.ts | 55 +++++++++++++++++++ .../core/testing/src/component_fixture.ts | 30 +++++----- packages/core/testing/src/test_bed.ts | 12 +++- 6 files changed, 99 insertions(+), 32 deletions(-) diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts index 73df5543e31..30bfd874fa1 100644 --- a/packages/core/src/application/application_ref.ts +++ b/packages/core/src/application/application_ref.ts @@ -318,12 +318,17 @@ export class ApplicationRef { // Needed for ComponentFixture temporarily during migration of autoDetect behavior // Eventually the hostView of the fixture should just attach to ApplicationRef. - private externalTestViews: Set> = new Set(); + private allTestViews: Set> = new Set(); + private autoDetectTestViews: Set> = new Set(); + private includeAllTestViews = false; /** @internal */ afterTick = new Subject(); /** @internal */ get allViews(): Array> { - return [...this.externalTestViews.keys(), ...this._views]; + return [ + ...(this.includeAllTestViews ? this.allTestViews : this.autoDetectTestViews).keys(), + ...this._views, + ]; } /** diff --git a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts index 9ba7e9c9fc6..2aac739e1fa 100644 --- a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts +++ b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts @@ -8,7 +8,7 @@ import {Subscription} from 'rxjs'; -import {ApplicationRef} from '../../application/application_ref'; +import {ApplicationRef, ApplicationRefDirtyFlags} from '../../application/application_ref'; import { ENVIRONMENT_INITIALIZER, EnvironmentInjector, @@ -58,7 +58,8 @@ export class NgZoneChangeDetectionScheduler { } this.zone.run(() => { try { - this.applicationRef.tick(); + this.applicationRef.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeGlobal; + this.applicationRef._tick(); } catch (e) { this.applicationErrorHandler(e); } diff --git a/packages/core/test/render3/reactivity_spec.ts b/packages/core/test/render3/reactivity_spec.ts index 5d38b3e0666..4a8d8c41c52 100644 --- a/packages/core/test/render3/reactivity_spec.ts +++ b/packages/core/test/render3/reactivity_spec.ts @@ -806,10 +806,8 @@ describe('reactivity', () => { } } - const fixture = TestBed.createComponent(TestCmp); + TestBed.createComponent(TestCmp); TestBed.tick(); - expect(log).toEqual([]); - fixture.detectChanges(); expect(log).toEqual(['init', 'effect']); }); @@ -879,17 +877,17 @@ describe('reactivity', () => { vcr = inject(ViewContainerRef); } - const fixture = TestBed.createComponent(DriverCmp); - fixture.detectChanges(); + const componentRef = createComponent(DriverCmp, { + environmentInjector: TestBed.inject(EnvironmentInjector), + }); + componentRef.changeDetectorRef.detectChanges(); - fixture.componentInstance.vcr.createComponent(TestCmp); + componentRef.instance.vcr.createComponent(TestCmp); // Verify that simply creating the component didn't schedule the effect. - TestBed.tick(); + TestBed.inject(ApplicationRef).tick(); expect(log).toEqual([]); - - // Running change detection should schedule and run the effect. - fixture.detectChanges(); + componentRef.changeDetectorRef.detectChanges(); expect(log).toEqual(['init', 'effect']); }); @@ -918,8 +916,6 @@ describe('reactivity', () => { const fixture = TestBed.createComponent(TestCmp); TestBed.tick(); - expect(log).toEqual([]); - fixture.detectChanges(); expect(log).toEqual(['init', 'effect']); }); diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index c380b38f2cc..238110f032c 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -40,6 +40,8 @@ import { ɵɵsetNgModuleScope as setNgModuleScope, ɵɵtext as text, DOCUMENT, + signal, + provideZonelessChangeDetection, } from '../src/core'; import {DeferBlockBehavior} from '../testing'; import {TestBed, TestBedImpl} from '../testing/src/test_bed'; @@ -50,6 +52,7 @@ import {NgModuleType} from '../src/render3'; import {depsTracker} from '../src/render3/deps_tracker/deps_tracker'; import {setClassMetadataAsync} from '../src/render3/metadata'; import { + ComponentFixtureAutoDetect, TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT, THROW_ON_UNKNOWN_ELEMENTS_DEFAULT, THROW_ON_UNKNOWN_PROPERTIES_DEFAULT, @@ -2273,6 +2276,58 @@ describe('TestBed', () => { expect(TestBed.runInInjectionContext(functionThatUsesInject)).toEqual(expectedValue); }); + + describe('TestBed.tick', () => { + @Component({ + template: '{{state()}}', + }) + class Thing1 { + state = signal(1); + } + + describe('with zone change detection', () => { + it('should update fixtures with autoDetect', () => { + TestBed.configureTestingModule({ + providers: [{provide: ComponentFixtureAutoDetect, useValue: true}], + }); + const {nativeElement, componentInstance} = TestBed.createComponent(Thing1); + expect(nativeElement.textContent).toBe('1'); + + componentInstance.state.set(2); + TestBed.tick(); + expect(nativeElement.textContent).toBe('2'); + }); + + it('should update fixtures without autoDetect', () => { + const {nativeElement, componentInstance} = TestBed.createComponent(Thing1); + expect(nativeElement.textContent).toBe(''); // change detection didn't run yet + + componentInstance.state.set(2); + TestBed.tick(); + expect(nativeElement.textContent).toBe('2'); + }); + }); + + describe('with zoneless change detection', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideZonelessChangeDetection()], + }); + }); + + it('should update fixtures with zoneless', async () => { + const fixture = TestBed.createComponent(Thing1); + await fixture.whenStable(); + + const {nativeElement, componentInstance} = fixture; + expect(nativeElement.textContent).toBe('1'); + + componentInstance.state.set(2); + TestBed.tick(); + expect(nativeElement.textContent).toBe('2'); + }); + }); + }); }); describe('TestBed defer block behavior', () => { diff --git a/packages/core/testing/src/component_fixture.ts b/packages/core/testing/src/component_fixture.ts index 56462a38c0f..190d6194dc5 100644 --- a/packages/core/testing/src/component_fixture.ts +++ b/packages/core/testing/src/component_fixture.ts @@ -33,8 +33,9 @@ import {DeferBlockFixture} from './defer'; import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone} from './test_bed_common'; interface TestAppRef { - externalTestViews: Set; - skipCheckNoChangesForExternalTestViews: Set; + allTestViews: Set; + includeAllTestViews: boolean; + autoDetectTestViews: Set; } /** @@ -106,13 +107,15 @@ export class ComponentFixture { this.nativeElement = this.elementRef.nativeElement; this.componentRef = componentRef; + this._testAppRef.allTestViews.add(this.componentRef.hostView); if (this.autoDetect) { - this._testAppRef.externalTestViews.add(this.componentRef.hostView); + this._testAppRef.autoDetectTestViews.add(this.componentRef.hostView); this.scheduler?.notify(ɵNotificationSource.ViewAttached); this.scheduler?.notify(ɵNotificationSource.MarkAncestorsForTraversal); } this.componentRef.hostView.onDestroy(() => { - this._testAppRef.externalTestViews.delete(this.componentRef.hostView); + this._testAppRef.allTestViews.delete(this.componentRef.hostView); + this._testAppRef.autoDetectTestViews.delete(this.componentRef.hostView); }); // Create subscriptions outside the NgZone so that the callbacks run outside // of NgZone. @@ -150,12 +153,10 @@ export class ComponentFixture { if (this.zonelessEnabled) { try { - this._testAppRef.externalTestViews.add(this.componentRef.hostView); + this._testAppRef.includeAllTestViews = true; this._appRef.tick(); } finally { - if (!this.autoDetect) { - this._testAppRef.externalTestViews.delete(this.componentRef.hostView); - } + this._testAppRef.includeAllTestViews = false; } } else { // Run the change detection inside the NgZone so that any async tasks as part of the change @@ -203,12 +204,10 @@ export class ComponentFixture { throw new Error('Cannot call autoDetectChanges when ComponentFixtureNoNgZone is set.'); } - if (autoDetect !== this.autoDetect) { - if (autoDetect) { - this._testAppRef.externalTestViews.add(this.componentRef.hostView); - } else { - this._testAppRef.externalTestViews.delete(this.componentRef.hostView); - } + if (autoDetect) { + this._testAppRef.autoDetectTestViews.add(this.componentRef.hostView); + } else { + this._testAppRef.autoDetectTestViews.delete(this.componentRef.hostView); } this.autoDetect = autoDetect; @@ -282,7 +281,8 @@ export class ComponentFixture { */ destroy(): void { this.subscriptions.unsubscribe(); - this._testAppRef.externalTestViews.delete(this.componentRef.hostView); + this._testAppRef.autoDetectTestViews.delete(this.componentRef.hostView); + this._testAppRef.allTestViews.delete(this.componentRef.hostView); if (!this._isDestroyed) { this.componentRef.destroy(); this._isDestroyed = true; diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index d9eea2fbbd5..63be037eb42 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -826,7 +826,17 @@ export class TestBedImpl implements TestBed { * @publicApi */ tick(): void { - this.inject(ApplicationRef).tick(); + const appRef = this.inject(ApplicationRef); + try { + // TODO(atscott): ApplicationRef.tick should set includeAllTestViews to true itself rather than doing this here and in ComponentFixture + // The behavior should be that TestBed.tick, ComponentFixture.detectChanges, and ApplicationRef.tick all result in the test fixtures + // getting synchronized, regardless of whether they are autoDetect: true. + // Automatic scheduling (zone or zoneless) will call _tick which will _not_ include fixtures with autoDetect: false + (appRef as any).includeAllTestViews = true; + appRef.tick(); + } finally { + (appRef as any).includeAllTestViews = false; + } } }