Skip to content

Commit 488e8a3

Browse files
MeAkibcrisbeto
authored andcommitted
test(multiple): switch remaining tests away from fakeAsync (#33889)
Reworks most of the remaining `fakeAsync` tests in the CDK and Material. The tests that assert on errors reported asynchronously (e.g. thrown from an RxJS subscription) now use `jasmine.spyOnGlobalErrorsAsync`. The `material/list` test was failing without `fakeAsync` because the initial `ngModel` value from `beforeEach` was written at the test's first `await`, clearing the selection. We now flush it before the test starts. Also fixes a `cdk/table` test that was detecting changes on the wrong fixture. (cherry picked from commit 62f58d1)
1 parent f9543da commit 488e8a3

5 files changed

Lines changed: 121 additions & 96 deletions

File tree

‎src/cdk/drag-drop/directives/drop-list-shared.spec.ts‎

Lines changed: 65 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
inject,
1414
signal,
1515
} from '@angular/core';
16-
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
16+
import {ComponentFixture, TestBed} from '@angular/core/testing';
1717
import {Platform, _supportsShadowDom} from '../../platform';
1818
import {CdkScrollable, ViewportRuler} from '../../scrolling';
1919
import {
@@ -1096,19 +1096,19 @@ export function defineCommonDropListTests(config: {
10961096
.toBe(sourceCanvas.toDataURL());
10971097
});
10981098

1099-
// TODO(crisbeto): this one is using `fakeAsync` because we throw an error from a subscription.
1100-
it('should not throw when cloning an invalid canvas', fakeAsync(() => {
1101-
const fixture = createComponent(DraggableWithInvalidCanvasInDropZone);
1102-
fixture.detectChanges();
1103-
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
1099+
it('should not throw when cloning an invalid canvas', async () => {
1100+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
1101+
const fixture = createComponent(DraggableWithInvalidCanvasInDropZone);
1102+
fixture.detectChanges();
1103+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
11041104

1105-
expect(() => {
1106-
startDraggingViaMouse(fixture, item);
1107-
tick();
1108-
}).not.toThrow();
1105+
expect(() => startDraggingViaMouse(fixture, item)).not.toThrow();
1106+
await wait(0);
11091107

1110-
expect(document.querySelector('.cdk-drag-preview canvas')).toBeTruthy();
1111-
}));
1108+
expect(globalErrorSpy).not.toHaveBeenCalled();
1109+
expect(document.querySelector('.cdk-drag-preview canvas')).toBeTruthy();
1110+
});
1111+
});
11121112

11131113
it('should clone the content of descendant input elements', () => {
11141114
const fixture = createComponent(DraggableWithInputsInDropZone);
@@ -2340,29 +2340,32 @@ export function defineCommonDropListTests(config: {
23402340
]);
23412341
});
23422342

2343-
// TODO(crisbeto): this one is using `fakeAsync` because we throw an error from a subscription.
2344-
it('should not throw if an item is removed after dragging has started', fakeAsync(() => {
2345-
const fixture = createComponent(DraggableInDropZone);
2346-
fixture.detectChanges();
2347-
const dragItems = fixture.componentInstance.dragItems;
2348-
const firstElement = dragItems.first.element.nativeElement;
2349-
const lastItemRect = dragItems.last.element.nativeElement.getBoundingClientRect();
2350-
2351-
// Start dragging.
2352-
startDraggingViaMouse(fixture, firstElement);
2343+
it('should not throw if an item is removed after dragging has started', async () => {
2344+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
2345+
const fixture = createComponent(DraggableInDropZone);
2346+
fixture.detectChanges();
2347+
const dragItems = fixture.componentInstance.dragItems;
2348+
const firstElement = dragItems.first.element.nativeElement;
2349+
const lastItemRect = dragItems.last.element.nativeElement.getBoundingClientRect();
23532350

2354-
// Remove the last item.
2355-
fixture.componentInstance.items.pop();
2356-
fixture.changeDetectorRef.markForCheck();
2357-
fixture.detectChanges();
2351+
// Start dragging.
2352+
startDraggingViaMouse(fixture, firstElement);
23582353

2359-
expect(() => {
2360-
// Move the dragged item over where the remove item would've been.
2361-
dispatchMouseEvent(document, 'mousemove', lastItemRect.left + 1, lastItemRect.top + 1);
2354+
// Remove the last item.
2355+
fixture.componentInstance.items.pop();
2356+
fixture.changeDetectorRef.markForCheck();
23622357
fixture.detectChanges();
2363-
flush();
2364-
}).not.toThrow();
2365-
}));
2358+
2359+
expect(() => {
2360+
// Move the dragged item over where the remove item would've been.
2361+
dispatchMouseEvent(document, 'mousemove', lastItemRect.left + 1, lastItemRect.top + 1);
2362+
fixture.detectChanges();
2363+
}).not.toThrow();
2364+
await wait(0);
2365+
2366+
expect(globalErrorSpy).not.toHaveBeenCalled();
2367+
});
2368+
});
23662369

23672370
it('should not be able to start a drag sequence while another one is still active', async () => {
23682371
const fixture = createComponent(DraggableInDropZone);
@@ -4703,33 +4706,43 @@ export function defineCommonDropListTests(config: {
47034706
.toBe(targetContainer);
47044707
});
47054708

4706-
// TODO(crisbeto): this one is using `fakeAsync` because we throw an error from a subscription.
4707-
it('should throw if the items are not inside of the alternate container', fakeAsync(() => {
4708-
const fixture = createComponent(DraggableWithInvalidAlternateContainer);
4709-
fixture.detectChanges();
4709+
it('should throw if the items are not inside of the alternate container', async () => {
4710+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
4711+
const fixture = createComponent(DraggableWithInvalidAlternateContainer);
4712+
fixture.detectChanges();
47104713

4711-
expect(() => {
47124714
const item = fixture.componentInstance.dragItems.first.element.nativeElement;
47134715
startDraggingViaMouse(fixture, item);
4714-
tick();
4715-
}).toThrowError(
4716-
/Invalid DOM structure for drop list\. All items must be placed directly inside of the element container/,
4717-
);
4718-
}));
4716+
await wait(0);
4717+
4718+
expect(globalErrorSpy).toHaveBeenCalledWith(
4719+
jasmine.objectContaining({
4720+
message: jasmine.stringMatching(
4721+
/Invalid DOM structure for drop list\. All items must be placed directly inside of the element container/,
4722+
),
4723+
}),
4724+
);
4725+
});
4726+
});
47194727

4720-
// TODO(crisbeto): this one is using `fakeAsync` because we throw an error from a subscription.
4721-
it('should throw if the alternate container cannot be found', fakeAsync(() => {
4722-
const fixture = createComponent(DraggableWithMissingAlternateContainer);
4723-
fixture.detectChanges();
4728+
it('should throw if the alternate container cannot be found', async () => {
4729+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
4730+
const fixture = createComponent(DraggableWithMissingAlternateContainer);
4731+
fixture.detectChanges();
47244732

4725-
expect(() => {
47264733
const item = fixture.componentInstance.dragItems.first.element.nativeElement;
47274734
startDraggingViaMouse(fixture, item);
4728-
tick();
4729-
}).toThrowError(
4730-
/CdkDropList could not find an element container matching the selector "does-not-exist"/,
4731-
);
4732-
}));
4735+
await wait(0);
4736+
4737+
expect(globalErrorSpy).toHaveBeenCalledWith(
4738+
jasmine.objectContaining({
4739+
message: jasmine.stringMatching(
4740+
/CdkDropList could not find an element container matching the selector "does-not-exist"/,
4741+
),
4742+
}),
4743+
);
4744+
});
4745+
});
47334746
});
47344747

47354748
describe('with an anchor', () => {

‎src/cdk/table/table.spec.ts‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
signal,
1717
} from '@angular/core';
1818
import {By} from '@angular/platform-browser';
19-
import {ComponentFixture, fakeAsync, flush, TestBed} from '@angular/core/testing';
19+
import {ComponentFixture, TestBed} from '@angular/core/testing';
2020
import {BehaviorSubject, Observable, combineLatest, of as observableOf} from 'rxjs';
2121
import {map} from 'rxjs/operators';
2222
import {CdkColumnDef} from './cell';
@@ -694,16 +694,19 @@ describe('CdkTable', () => {
694694
).not.toThrow();
695695
});
696696

697-
it('should throw an error if a column definition is requested but not defined after render', fakeAsync(() => {
697+
it('should throw an error if a column definition is requested but not defined after render', async () => {
698698
const columnDefinitionMissingAfterRenderFixture = TestBed.createComponent(
699699
MissingColumnDefAfterRenderCdkTableApp,
700700
);
701-
expect(() => {
702-
columnDefinitionMissingAfterRenderFixture.detectChanges();
703-
flush();
704-
columnDefinitionMissingAfterRenderFixture.detectChanges();
705-
}).toThrowError(getTableUnknownColumnError('column_a').message);
706-
}));
701+
columnDefinitionMissingAfterRenderFixture.detectChanges();
702+
703+
// Wait for the `setTimeout` in the component to add the missing column.
704+
await new Promise(resolve => setTimeout(resolve));
705+
706+
expect(() => columnDefinitionMissingAfterRenderFixture.detectChanges()).toThrowError(
707+
getTableUnknownColumnError('column_a').message,
708+
);
709+
});
707710

708711
it('should throw an error if the row definitions are missing', () => {
709712
expect(() =>
@@ -820,17 +823,21 @@ describe('CdkTable', () => {
820823
expect(updatedRows[2].classList).toContain('default-row');
821824
});
822825

823-
it('should error if there is row data that does not have a matching row template', fakeAsync(() => {
824-
const whenRowWithoutDefaultFixture = TestBed.createComponent(
825-
WhenRowWithoutDefaultCdkTableApp,
826-
);
827-
const data = whenRowWithoutDefaultFixture.componentInstance.dataSource.data;
828-
expect(() => {
826+
it('should error if there is row data that does not have a matching row template', async () => {
827+
// The error is thrown inside a subscription so RxJS reports it asynchronously.
828+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
829+
const whenRowWithoutDefaultFixture = TestBed.createComponent(
830+
WhenRowWithoutDefaultCdkTableApp,
831+
);
832+
const data = whenRowWithoutDefaultFixture.componentInstance.dataSource.data;
829833
whenRowWithoutDefaultFixture.detectChanges();
830-
flush();
831-
fixture.detectChanges();
832-
}).toThrowError(getTableMissingMatchingRowDefError(data[0]).message);
833-
}));
834+
await new Promise(resolve => setTimeout(resolve));
835+
836+
expect(globalErrorSpy).toHaveBeenCalledWith(
837+
new Error(getTableMissingMatchingRowDefError(data[0]).message),
838+
);
839+
});
840+
});
834841

835842
it('should fail when multiple rows match data without multiTemplateDataRows', () => {
836843
let whenFixture = TestBed.createComponent(WhenRowMultipleDefaultsCdkTableApp);

‎src/cdk/tree/control/nested-tree-control.spec.ts‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import {fakeAsync, flush} from '@angular/core/testing';
21
import {of as observableOf} from 'rxjs';
32
import {NestedTreeControl} from './nested-tree-control';
43

@@ -114,19 +113,19 @@ describe('CdkNestedTreeControl', () => {
114113
.toBe(totalNumber);
115114
});
116115

117-
// Note that this needs to be `fakeAsync` in order to
118-
// catch the error inside an observable correctly.
119-
it('should handle null children', fakeAsync(() => {
120-
const nodes = generateData(3, 2);
116+
it('should handle null children', async () => {
117+
// Errors inside an observable are reported asynchronously so we need to check for global ones.
118+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
119+
const nodes = generateData(3, 2);
121120

122-
nodes[1].children = null!;
123-
treeControl.dataNodes = nodes;
121+
nodes[1].children = null!;
122+
treeControl.dataNodes = nodes;
124123

125-
expect(() => {
126-
treeControl.expandAll();
127-
flush();
128-
}).not.toThrow();
129-
}));
124+
expect(() => treeControl.expandAll()).not.toThrow();
125+
await new Promise(resolve => setTimeout(resolve));
126+
expect(globalErrorSpy).not.toHaveBeenCalled();
127+
});
128+
});
130129

131130
describe('with children array', () => {
132131
let getStaticChildren = (node: TestData) => node.children;

‎src/material/list/selection-list.spec.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
QueryList,
1414
ViewChildren,
1515
} from '@angular/core';
16-
import {ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
16+
import {ComponentFixture, TestBed} from '@angular/core/testing';
1717
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
1818
import {ThemePalette} from '../core';
1919
import {By} from '@angular/platform-browser';
@@ -1339,8 +1339,11 @@ describe('MatSelectionList with forms', () => {
13391339
.toBe(false);
13401340
});
13411341

1342-
// TODO: this seems tricky to switch away from `fakeAsync` for some reason.
1343-
it('should remove a selected option from the value on destroy', fakeAsync(() => {
1342+
it('should remove a selected option from the value on destroy', async () => {
1343+
// Flush the initial `ngModel` value before the test starts so that it doesn't overwrite the
1344+
// selection further down.
1345+
await fixture.whenStable();
1346+
13441347
listOptions[1].selected = true;
13451348
listOptions[2].selected = true;
13461349

@@ -1350,10 +1353,10 @@ describe('MatSelectionList with forms', () => {
13501353

13511354
fixture.componentInstance.options.pop();
13521355
fixture.detectChanges();
1353-
tick();
1356+
await fixture.whenStable();
13541357

13551358
expect(fixture.componentInstance.selectedOptions).toEqual(['opt2']);
1356-
}));
1359+
});
13571360

13581361
it('should update the model if an option got selected via the model', async () => {
13591362
expect(fixture.componentInstance.selectedOptions).toEqual([]);

‎src/material/table/table.spec.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {AfterViewInit, Component, OnInit, ViewChild, ChangeDetectionStrategy} from '@angular/core';
2-
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
2+
import {ComponentFixture, TestBed} from '@angular/core/testing';
33
import {MatTable, MatTableDataSource, MatTableModule} from './index';
44
import {DataSource} from '@angular/cdk/table';
55
import {BehaviorSubject, Observable} from 'rxjs';
@@ -187,15 +187,18 @@ describe('MatTable', () => {
187187
expect(stuckCellElement.classList).toContain('mat-mdc-table-sticky');
188188
});
189189

190-
// Note: needs to be fakeAsync so it catches the error.
191-
it('should not throw when a row definition is on an ng-container', fakeAsync(() => {
192-
const fixture = TestBed.createComponent(TableWithNgContainerRow);
190+
it('should not throw when a row definition is on an ng-container', async () => {
191+
// Some of the table's logic runs asynchronously so we need to check for global errors.
192+
await jasmine.spyOnGlobalErrorsAsync(async globalErrorSpy => {
193+
const fixture = TestBed.createComponent(TableWithNgContainerRow);
193194

194-
expect(() => {
195-
fixture.detectChanges();
196-
tick();
197-
}).not.toThrow();
198-
}));
195+
expect(() => fixture.detectChanges()).not.toThrow();
196+
await fixture.whenStable();
197+
await new Promise(resolve => setTimeout(resolve));
198+
199+
expect(globalErrorSpy).not.toHaveBeenCalled();
200+
});
201+
});
199202

200203
it('should be able to render a flexbox-based table', () => {
201204
expect(() => {

0 commit comments

Comments
 (0)