Skip to content

refactor(cdk/overlay): switch tests away from fakeAsync #33192

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

Merged
crisbeto merged 1 commit into from
May 3, 2026
Merged

refactor(cdk/overlay): switch tests away from fakeAsync #33192

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
16 changes: 8 additions & 8 deletions src/cdk/overlay/overlay-directives.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
WritableSignal,
ChangeDetectionStrategy,
} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {Subject} from 'rxjs';
import {Direction} from '../bidi';
import {A, ESCAPE} from '../keycodes';
Expand Down Expand Up @@ -178,7 +178,7 @@ describe('Overlay directives', () => {
expect(event.defaultPrevented).toBe(false);
});

it('should prevent closing via clicks on the backdrop by default', fakeAsync(() => {
it('should prevent closing via clicks on the backdrop by default', () => {
fixture.componentInstance.hasBackdrop = true;
fixture.componentInstance.isOpen = true;
fixture.changeDetectorRef.markForCheck();
Expand All @@ -189,7 +189,7 @@ describe('Overlay directives', () => {
fixture.detectChanges();

expect(overlayContainerElement.textContent!.trim()).toBeTruthy();
}));
});

it('should prevent closing via the escape key with disableClose option', () => {
fixture.componentInstance.isOpen = true;
Expand All @@ -204,7 +204,7 @@ describe('Overlay directives', () => {
expect(event.defaultPrevented).toBe(false);
});

it('should not depend on the order in which the `origin` and `open` are set', waitForAsync(() => {
it('should not depend on the order in which the `origin` and `open` are set', () => {
fixture.destroy();

const propOrderFixture = TestBed.createComponent(ConnectedOverlayPropertyInitOrder);
Expand All @@ -217,7 +217,7 @@ describe('Overlay directives', () => {
overlayDirective.origin = propOrderFixture.componentInstance.trigger;
propOrderFixture.detectChanges();
}).not.toThrow();
}));
});

describe('inputs', () => {
it('should set the width', () => {
Expand Down Expand Up @@ -321,7 +321,7 @@ describe('Overlay directives', () => {
expect(overlayContainerElement.querySelector('.cdk-overlay-backdrop')).toBeNull();
});

it('should be able to change hasBackdrop after the overlay has been initialized', fakeAsync(() => {
it('should be able to change hasBackdrop after the overlay has been initialized', async () => {
// Open once with a backdrop
fixture.componentInstance.hasBackdrop = true;
fixture.componentInstance.isOpen = true;
Expand All @@ -333,7 +333,7 @@ describe('Overlay directives', () => {
fixture.componentInstance.isOpen = false;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();
tick(500);
await new Promise(resolve => setTimeout(resolve, 500));

// Open again without a backdrop.
fixture.componentInstance.hasBackdrop = false;
Expand All @@ -342,7 +342,7 @@ describe('Overlay directives', () => {
fixture.detectChanges();

expect(overlayContainerElement.querySelector('.cdk-overlay-backdrop')).toBeFalsy();
}));
});

it('should set the custom backdrop class', () => {
fixture.componentInstance.hasBackdrop = true;
Expand Down
118 changes: 42 additions & 76 deletions src/cdk/overlay/overlay.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,7 @@ import {
signal,
ChangeDetectionStrategy,
} from '@angular/core';
import {
ComponentFixture,
TestBed,
fakeAsync,
flush,
tick,
waitForAsync,
} from '@angular/core/testing';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {Direction, Directionality} from '../bidi';
import {CdkPortal, ComponentPortal, TemplatePortal} from '../portal';
import {dispatchFakeEvent} from '../testing/private';
Expand Down Expand Up @@ -77,7 +70,11 @@ describe('Overlay', () => {
overlayContainer.ngOnDestroy();
}

beforeEach(waitForAsync(setup));
function wait(milliseconds: number) {
return new Promise(resolve => setTimeout(resolve, milliseconds));
}

beforeEach(setup);
afterEach(cleanup);

it('should load a component into an overlay', () => {
Expand Down Expand Up @@ -299,7 +296,7 @@ describe('Overlay', () => {
expect(overlayRef.getConfig().direction).toBe('ltr');
});

it('should clear out all DOM element references on dispose', fakeAsync(() => {
it('should clear out all DOM element references on dispose', async () => {
const overlayRef = createOverlayRef(injector, {hasBackdrop: true});
overlayRef.attach(componentPortal);

Expand All @@ -312,7 +309,7 @@ describe('Overlay', () => {
.toBeTruthy();

overlayRef.dispose();
tick(500);
await wait(500);

expect(overlayRef.hostElement)
.withContext('Expected overlay host not to be referenced.')
Expand All @@ -323,30 +320,7 @@ describe('Overlay', () => {
expect(overlayRef.backdropElement)
.withContext('Expected backdrop element not to be referenced.')
.toBeFalsy();
}));

it('should clear the backdrop timeout if the transition finishes first', fakeAsync(() => {
const overlayRef = createOverlayRef(injector, {hasBackdrop: true});

overlayRef.attach(componentPortal);
overlayRef.detach();

const backdrop = overlayContainerElement.querySelector('.cdk-overlay-backdrop')!;
dispatchFakeEvent(backdrop, 'transitionend');

// Note: we don't `tick` or `flush` here. The assertion is that
// `fakeAsync` will throw if we have an unflushed timer.
}));

it('should clear the backdrop timeout if the overlay is disposed', fakeAsync(() => {
const overlayRef = createOverlayRef(injector, {hasBackdrop: true});
overlayRef.attach(componentPortal);
overlayRef.detach();
overlayRef.dispose();

// Note: we don't `tick` or `flush` here. The assertion is that
// `fakeAsync` will throw if we have an unflushed timer.
}));
});

it('should be able to use the `Overlay` provider during app initialization', () => {
/** Dummy provider that depends on `Overlay`. */
Expand Down Expand Up @@ -466,7 +440,7 @@ describe('Overlay', () => {
expect(() => overlayRef.removePanelClass([''])).not.toThrow();
});

it('should detach a component-based overlay when the view is destroyed', fakeAsync(() => {
it('should detach a component-based overlay when the view is destroyed', async () => {
const overlayRef = createOverlayRef(injector);
const paneElement = overlayRef.overlayElement;

Expand All @@ -476,25 +450,26 @@ describe('Overlay', () => {
expect(paneElement.childNodes.length).not.toBe(0);

viewContainerFixture.destroy();
flush();
await viewContainerFixture.whenStable();

expect(paneElement.childNodes.length).toBe(0);
}));
});

it('should detach a template-based overlay when the view is destroyed', fakeAsync(() => {
it('should detach a template-based overlay when the view is destroyed', async () => {
const overlayRef = createOverlayRef(injector);
const paneElement = overlayRef.overlayElement;

overlayRef.attach(templatePortal);
viewContainerFixture.detectChanges();
await viewContainerFixture.whenStable();

expect(paneElement.childNodes.length).not.toBe(0);

viewContainerFixture.destroy();
flush();
await wait(100);

expect(paneElement.childNodes.length).toBe(0);
}));
});

it('should do nothing when trying to attach a disposed overlay', () => {
const overlayRef = createOverlayRef(injector);
Expand All @@ -510,17 +485,17 @@ describe('Overlay', () => {
config = new OverlayConfig();
});

it('should apply the positioning strategy', fakeAsync(() => {
it('should apply the positioning strategy', async () => {
config.positionStrategy = new FakePositionStrategy();

createOverlayRef(injector, config).attach(componentPortal);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(overlayContainerElement.querySelectorAll('.fake-positioned').length).toBe(1);
}));
});

it('should have the overlay in the DOM in position strategy when reattaching', fakeAsync(() => {
it('should have the overlay in the DOM in position strategy when reattaching', async () => {
let overlayPresentInDom = false;

config.positionStrategy = {
Expand All @@ -537,17 +512,17 @@ describe('Overlay', () => {
.toBeTruthy();

overlayRef.detach();
tick();
await wait(0);

overlayRef.attach(componentPortal);
tick();
await wait(0);

expect(overlayPresentInDom)
.withContext('Expected host element to be attached to the DOM.')
.toBeTruthy();
}));
});

it('should not apply the position if it detaches before the zone stabilizes', fakeAsync(() => {
it('should not apply the position if it detaches before the zone stabilizes', async () => {
config.positionStrategy = new FakePositionStrategy();

const overlayRef = createOverlayRef(injector, config);
Expand All @@ -557,12 +532,12 @@ describe('Overlay', () => {
overlayRef.attach(componentPortal);
overlayRef.detach();
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(config.positionStrategy.apply).not.toHaveBeenCalled();
}));
});

it('should be able to swap position strategies', fakeAsync(() => {
it('should be able to swap position strategies', async () => {
const firstStrategy = new FakePositionStrategy();
const secondStrategy = new FakePositionStrategy();

Expand All @@ -577,7 +552,7 @@ describe('Overlay', () => {
const overlayRef = createOverlayRef(injector, config);
overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(firstStrategy.attach).toHaveBeenCalledTimes(1);
expect(firstStrategy.apply).toHaveBeenCalledTimes(1);
Expand All @@ -587,17 +562,17 @@ describe('Overlay', () => {

overlayRef.updatePositionStrategy(secondStrategy);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(firstStrategy.attach).toHaveBeenCalledTimes(1);
expect(firstStrategy.apply).toHaveBeenCalledTimes(1);
expect(firstStrategy.dispose).toHaveBeenCalledTimes(1);

expect(secondStrategy.attach).toHaveBeenCalledTimes(1);
expect(secondStrategy.apply).toHaveBeenCalledTimes(1);
}));
});

it('should not do anything when trying to swap a positioning strategy with itself', fakeAsync(() => {
it('should not do anything when trying to swap a positioning strategy with itself', async () => {
const strategy = new FakePositionStrategy();

spyOn(strategy, 'attach');
Expand All @@ -609,20 +584,20 @@ describe('Overlay', () => {
const overlayRef = createOverlayRef(injector, config);
overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(strategy.attach).toHaveBeenCalledTimes(1);
expect(strategy.apply).toHaveBeenCalledTimes(1);
expect(strategy.dispose).not.toHaveBeenCalled();

overlayRef.updatePositionStrategy(strategy);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(strategy.attach).toHaveBeenCalledTimes(1);
expect(strategy.apply).toHaveBeenCalledTimes(1);
expect(strategy.dispose).not.toHaveBeenCalled();
}));
});

it('should not throw when disposing multiple times in a row', () => {
const overlayRef = createOverlayRef(injector);
Expand All @@ -636,15 +611,6 @@ describe('Overlay', () => {
overlayRef.dispose();
}).not.toThrow();
});

it('should not trigger timers when disposing of an overlay', fakeAsync(() => {
const overlayRef = createOverlayRef(injector, {hasBackdrop: true});
overlayRef.attach(templatePortal);
overlayRef.dispose();

// The assertion here is that `fakeAsync` doesn't flag
// any pending timeouts after the test is done.
}));
});

describe('size', () => {
Expand Down Expand Up @@ -1050,7 +1016,7 @@ describe('Overlay', () => {
expect(fakeScrollStrategy.overlayRef).toBeNull();
});

it('should be able to swap scroll strategies', fakeAsync(() => {
it('should be able to swap scroll strategies', async () => {
const firstStrategy = new FakeScrollStrategy();
const secondStrategy = new FakeScrollStrategy();

Expand All @@ -1065,7 +1031,7 @@ describe('Overlay', () => {

overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(firstStrategy.attach).toHaveBeenCalledTimes(1);
expect(firstStrategy.enable).toHaveBeenCalledTimes(1);
Expand All @@ -1075,7 +1041,7 @@ describe('Overlay', () => {

overlayRef.updateScrollStrategy(secondStrategy);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(firstStrategy.attach).toHaveBeenCalledTimes(1);
expect(firstStrategy.enable).toHaveBeenCalledTimes(1);
Expand All @@ -1084,9 +1050,9 @@ describe('Overlay', () => {

expect(secondStrategy.attach).toHaveBeenCalledTimes(1);
expect(secondStrategy.enable).toHaveBeenCalledTimes(1);
}));
});

it('should not do anything when trying to swap a scroll strategy with itself', fakeAsync(() => {
it('should not do anything when trying to swap a scroll strategy with itself', async () => {
const strategy = new FakeScrollStrategy();

spyOn(strategy, 'attach');
Expand All @@ -1098,7 +1064,7 @@ describe('Overlay', () => {

overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(strategy.attach).toHaveBeenCalledTimes(1);
expect(strategy.enable).toHaveBeenCalledTimes(1);
Expand All @@ -1107,13 +1073,13 @@ describe('Overlay', () => {

overlayRef.updateScrollStrategy(strategy);
viewContainerFixture.detectChanges();
tick();
await viewContainerFixture.whenStable();

expect(strategy.attach).toHaveBeenCalledTimes(1);
expect(strategy.enable).toHaveBeenCalledTimes(1);
expect(strategy.disable).not.toHaveBeenCalled();
expect(strategy.detach).not.toHaveBeenCalled();
}));
});
});
});

Expand Down
Loading
Loading