Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/public-api/core/testing/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion packages/core/test/component_fixture_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ describe('ComponentFixture', () => {
const element = componentFixture.debugElement.children[0];
dispatchEvent(element.nativeElement, 'click');

expect(componentFixture.isStable()).toBe(true);
atscott marked this conversation as resolved.
Show resolved Hide resolved
expect(componentFixture.nativeElement).toHaveText('11');
});

Expand Down
76 changes: 11 additions & 65 deletions packages/core/testing/src/component_fixture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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.');
}
Expand All @@ -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;

Expand All @@ -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) => {
Expand All @@ -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.');
Expand Down
37 changes: 18 additions & 19 deletions packages/forms/test/template_integration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand All @@ -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', () => {
Expand Down