Skip to content

Commit 30eb1c5

Browse files
committed
refactor: use jasmine expectAsync instead of try/catch
Currently tests that assert errors to be thrown/or not from asynchronous methods use `try/catch` in order to use `await`. We added a helper method for this in the private testing utilities. This one reduced the `try/catch` burden. That helper method has downsides though because it doesn't allow us to expect no error to be thrown. We should remove this method in favor of using jasmine's `expectAsync` function. This one gives us more flexibility and is also more "jamsine"-idiomatic.
1 parent a0aee92 commit 30eb1c5

File tree

23 files changed

+125
-181
lines changed

23 files changed

+125
-181
lines changed

package.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585
"@types/glob": "^5.0.33",
8686
"@types/gulp": "3.8.32",
8787
"@types/inquirer": "^0.0.43",
88-
"@types/jasmine": "^3.4.0",
88+
"@types/jasmine": "^3.5.4",
8989
"@types/marked": "^0.4.2",
9090
"@types/merge2": "^0.3.30",
9191
"@types/minimist": "^1.2.0",
@@ -113,12 +113,12 @@
113113
"highlight.js": "^9.11.0",
114114
"husky": "^1.3.1",
115115
"inquirer": "^6.2.0",
116-
"jasmine-core": "^3.3.0",
116+
"jasmine-core": "^3.5.0",
117117
"karma": "^4.4.1",
118118
"karma-browserstack-launcher": "^1.3.0",
119119
"karma-chrome-launcher": "^2.2.0",
120120
"karma-firefox-launcher": "^1.0.1",
121-
"karma-jasmine": "^2.0.1",
121+
"karma-jasmine": "^3.1.1",
122122
"karma-parallel": "^0.3.0",
123123
"karma-requirejs": "^1.1.0",
124124
"karma-sauce-launcher": "^2.0.2",
@@ -156,6 +156,8 @@
156156
},
157157
"resolutions": {
158158
"dgeni-packages/typescript": "3.7.4",
159+
"@bazel/jasmine/jasmine": "3.5.0",
160+
"@bazel/jasmine/jasmine-core": "3.5.0",
159161
"**/graceful-fs": "4.2.2"
160162
}
161163
}

src/cdk/a11y/key-manager/list-key-manager.spec.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
99
import {FocusKeyManager} from './focus-key-manager';
1010
import {ListKeyManager, ListKeyManagerModifierKey, ListKeyManagerOption} from './list-key-manager';
1111

12-
1312
class FakeFocusable {
1413
/** Whether the item is disabled or not. */
1514
disabled = false;
@@ -68,7 +67,14 @@ describe('Key managers', () => {
6867
});
6968

7069
describe('ListKeyManager', () => {
71-
let keyManager: ListKeyManager<FakeFocusable>;
70+
// We have a spy on the `setActiveItem` method of the list key manager. That method has
71+
// multiple overloads and TypeScript is unable to infer the right parameters when calls are
72+
// checked using jasmine's `hasBeenCalledWith` matcher. We work around this by explicitly
73+
// specifying the overload signature that should be used.
74+
// TODO: remove if https://github.com/DefinitelyTyped/DefinitelyTyped/issues/42455 is solved.
75+
let keyManager: Omit<ListKeyManager<FakeFocusable>, 'setActiveItem'> & {
76+
setActiveItem(index: number): void;
77+
};
7278

7379
beforeEach(() => {
7480
itemList.items = [

src/cdk/schematics/ng-update/test-cases/misc/global-stylesheets.spec.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,9 @@ describe('global stylesheets migration', () => {
3434
writeFile('/sub_project/node_modules/materialize.css/package.json', '');
3535
writeFile('/sub_project/assets/test.css', subProjectStylesheet);
3636

37-
let error: any = null;
38-
try {
39-
await runFixers()
40-
} catch (e) {
41-
error = e;
42-
}
43-
44-
expect(error).toBeNull();
37+
// Run the fixers and expect no error to be thrown.
38+
await expectAsync(runFixers()).not.toBeRejected();
39+
4540
// if the external stylesheet that is not of a project target would have been checked
4641
// by accident, the stylesheet would differ from the original file content.
4742
expect(appTree.readContent('/sub_project/assets/test.css')).toBe(subProjectStylesheet);

src/cdk/testing/private/expect-async-error.ts

Lines changed: 0 additions & 26 deletions
This file was deleted.

src/cdk/testing/private/public-api.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
export * from './expect-async-error';
109
export * from './wrapped-error-message';
1110
export * from './mock-ng-zone';
1211
export * from './text-dedent';

src/cdk/testing/tests/protractor.e2e.spec.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
HarnessLoader,
55
TestElement
66
} from '@angular/cdk/testing';
7-
import {expectAsyncError} from '@angular/cdk/testing/private';
87
import {ProtractorHarnessEnvironment} from '@angular/cdk/testing/protractor';
98
import {browser} from 'protractor';
109
import {MainComponentHarness} from './harnesses/main-component-harness';
@@ -392,11 +391,16 @@ describe('ProtractorHarnessEnvironment', () => {
392391
});
393392

394393
it('should throw when multiple queries fail to match', async () => {
395-
await expectAsyncError(() => harness.missingElementsAndHarnesses(),
396-
'Error: Failed to find element matching one of the following queries:' +
397-
'\n(TestElement for element matching selector: ".not-found"),' +
398-
'\n(SubComponentHarness with host element matching selector: "test-sub" satisfying' +
399-
' the constraints: title = /not found/)');
394+
try {
395+
await harness.missingElementsAndHarnesses();
396+
fail('Expected to throw.');
397+
} catch (e) {
398+
expect(e.message).toBe(
399+
'Failed to find element matching one of the following queries:' +
400+
'\n(TestElement for element matching selector: ".not-found"),' +
401+
'\n(SubComponentHarness with host element matching selector: "test-sub" satisfying' +
402+
' the constraints: title = /not found/)');
403+
}
400404
});
401405

402406
it('should check if element is focused', async () => {

src/cdk/testing/tests/testbed.spec.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
HarnessLoader,
55
TestElement
66
} from '@angular/cdk/testing';
7-
import {expectAsyncError} from '@angular/cdk/testing/private';
87
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
98
import {async, ComponentFixture, fakeAsync, TestBed} from '@angular/core/testing';
109
import {FakeOverlayHarness} from './harnesses/fake-overlay-harness';
@@ -435,11 +434,16 @@ describe('TestbedHarnessEnvironment', () => {
435434
});
436435

437436
it('should throw when multiple queries fail to match', async () => {
438-
await expectAsyncError(() => harness.missingElementsAndHarnesses(),
439-
'Error: Failed to find element matching one of the following queries:' +
437+
try {
438+
await harness.missingElementsAndHarnesses();
439+
fail('Expected to throw.');
440+
} catch (e) {
441+
expect(e.message).toBe(
442+
'Failed to find element matching one of the following queries:' +
440443
'\n(TestElement for element matching selector: ".not-found"),' +
441444
'\n(SubComponentHarness with host element matching selector: "test-sub" satisfying' +
442445
' the constraints: title = /not found/)');
446+
}
443447
});
444448

445449
it('should check if element is focused', async () => {

src/material/autocomplete/testing/shared.spec.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {OverlayContainer} from '@angular/cdk/overlay';
2-
import {expectAsyncError} from '@angular/cdk/testing/private';
32
import {HarnessLoader} from '@angular/cdk/testing';
43
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
54
import {Component} from '@angular/core';
@@ -131,8 +130,8 @@ export function runHarnessTests(
131130
it('should throw when selecting an option that is not available', async () => {
132131
const input = await loader.getHarness(autocompleteHarness.with({selector: '#plain'}));
133132
await input.enterText('New');
134-
await expectAsyncError(() => input.selectOption({text: 'Texas'}),
135-
/Error: Could not find a mat-option matching {"text":"Texas"}/);
133+
await expectAsync(input.selectOption({text: 'Texas'})).toBeRejectedWithError(
134+
/Could not find a mat-option matching {"text":"Texas"}/);
136135
});
137136
}
138137

src/material/grid-list/testing/shared.spec.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {HarnessLoader} from '@angular/cdk/testing';
2-
import {expectAsyncError} from '@angular/cdk/testing/private';
32
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
43
import {Component} from '@angular/core';
54
import {ComponentFixture, TestBed} from '@angular/core/testing';
@@ -91,8 +90,8 @@ export function runHarnessTests(
9190
]);
9291
expect(await tiles[0].getHeaderText()).toBe('Three');
9392
expect(await tiles[1].getHeaderText()).toBe('Three');
94-
await expectAsyncError(
95-
() => gridList.getTileAtPosition({row: 2, column: 0}), /Could not find tile/);
93+
await expectAsync(gridList.getTileAtPosition({row: 2, column: 0}))
94+
.toBeRejectedWithError(/Could not find tile/);
9695

9796
// Update the fourth tile to span over two rows. The previous position
9897
// should now be valid and the fourth tile should be returned.

src/material/menu/testing/shared.spec.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {OverlayContainer} from '@angular/cdk/overlay';
2-
import {expectAsyncError} from '@angular/cdk/testing/private';
32
import {HarnessLoader} from '@angular/cdk/testing';
43
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
54
import {Component} from '@angular/core';
@@ -155,8 +154,8 @@ export function runHarnessTests(
155154

156155
it('should throw when item is not found', async () => {
157156
const menu1 = await loader.getHarness(MatMenuHarness.with({triggerText: 'Menu 1'}));
158-
await expectAsyncError(() => menu1.clickItem({text: 'Fake Item'}),
159-
/Error: Could not find item matching {"text":"Fake Item"}/);
157+
await expectAsync(menu1.clickItem({text: 'Fake Item'})).toBeRejectedWithError(
158+
/Could not find item matching {"text":"Fake Item"}/);
160159
});
161160

162161
it('should select item in nested menu', async () => {
@@ -167,8 +166,8 @@ export function runHarnessTests(
167166

168167
it('should throw when intermediate item does not have submenu', async () => {
169168
const menu1 = await loader.getHarness(MatMenuHarness.with({triggerText: 'Menu 1'}));
170-
await expectAsyncError(() => menu1.clickItem({text: 'Leaf Item 1'}, {}),
171-
/Error: Item matching {"text":"Leaf Item 1"} does not have a submenu/);
169+
await expectAsync(menu1.clickItem({text: 'Leaf Item 1'}, {})).toBeRejectedWithError(
170+
/Item matching {"text":"Leaf Item 1"} does not have a submenu/);
172171
});
173172
});
174173
}

src/material/paginator/testing/shared.spec.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {NoopAnimationsModule} from '@angular/platform-browser/animations';
55
import {ComponentFixture, TestBed} from '@angular/core/testing';
66
import {MatPaginatorModule, PageEvent, MatPaginator} from '@angular/material/paginator';
77
import {MatPaginatorHarness} from './paginator-harness';
8-
import {expectAsyncError} from '@angular/cdk/testing/private';
98

109
/** Shared tests to run on both the original and MDC-based paginator. */
1110
export function runHarnessTests(
@@ -91,8 +90,8 @@ export function runHarnessTests(
9190
instance.showFirstLastButtons = false;
9291
fixture.detectChanges();
9392

94-
await expectAsyncError(() => paginator.goToFirstPage(),
95-
/Error: Could not find first page button inside paginator/);
93+
await expectAsync(paginator.goToFirstPage()).toBeRejectedWithError(
94+
/Could not find first page button inside paginator/);
9695
});
9796

9897
it('should throw an error if the last page button is not available', async () => {
@@ -101,8 +100,8 @@ export function runHarnessTests(
101100
instance.showFirstLastButtons = false;
102101
fixture.detectChanges();
103102

104-
await expectAsyncError(() => paginator.goToLastPage(),
105-
/Error: Could not find last page button inside paginator/);
103+
await expectAsync(paginator.goToLastPage()).toBeRejectedWithError(
104+
/Could not find last page button inside paginator/);
106105
});
107106

108107
it('should throw an error if the page size selector is not available', async () => {
@@ -111,8 +110,8 @@ export function runHarnessTests(
111110
instance.pageSizeOptions = [];
112111
fixture.detectChanges();
113112

114-
await expectAsyncError(() => paginator.setPageSize(10),
115-
/Error: Cannot find page size selector in paginator/);
113+
await expectAsync(paginator.setPageSize(10)).toBeRejectedWithError(
114+
/Cannot find page size selector in paginator/);
116115
});
117116
}
118117

src/material/radio/testing/shared.spec.ts

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {Platform} from '@angular/cdk/platform';
2-
import {expectAsyncError} from '@angular/cdk/testing/private';
32
import {HarnessLoader} from '@angular/cdk/testing';
43
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
54
import {Component} from '@angular/core';
@@ -57,15 +56,10 @@ export function runHarnessTests(radioModule: typeof MatRadioModule,
5756
fixture.componentInstance.thirdGroupButtonName = 'other-name';
5857
fixture.detectChanges();
5958

60-
let errorMessage: string | null = null;
61-
try {
62-
await loader.getAllHarnesses(radioGroupHarness.with({name: 'third-group-name'}));
63-
} catch (e) {
64-
errorMessage = e.toString();
65-
}
66-
67-
expect(errorMessage).toMatch(
68-
/locator found a radio-group with name "third-group-name".*have mismatching names/);
59+
await expectAsync(
60+
loader.getAllHarnesses(radioGroupHarness.with({name: 'third-group-name'})))
61+
.toBeRejectedWithError(
62+
/locator found a radio-group with name "third-group-name".*have mismatching names/);
6963
});
7064

7165
it('should get name of radio-group', async () => {
@@ -83,14 +77,8 @@ export function runHarnessTests(radioModule: typeof MatRadioModule,
8377
fixture.componentInstance.thirdGroupButtonName = 'other-button-name';
8478
fixture.detectChanges();
8579

86-
let errorMessage: string | null = null;
87-
try {
88-
await groups[2].getName();
89-
} catch (e) {
90-
errorMessage = e.toString();
91-
}
92-
93-
expect(errorMessage).toMatch(/Radio buttons in radio-group have mismatching names./);
80+
await expectAsync(groups[2].getName())
81+
.toBeRejectedWithError(/Radio buttons in radio-group have mismatching names./);
9482
});
9583

9684
it('should get id of radio-group', async () => {
@@ -149,8 +137,8 @@ export function runHarnessTests(radioModule: typeof MatRadioModule,
149137

150138
it('should throw error when checking invalid radio button', async () => {
151139
const group = await loader.getHarness(radioGroupHarness.with({name: 'my-group-1-name'}));
152-
await expectAsyncError(() => group.checkRadioButton({label: 'opt4'}),
153-
/Error: Could not find radio button matching {"label":"opt4"}/);
140+
await expectAsync(group.checkRadioButton({label: 'opt4'})).toBeRejectedWithError(
141+
/Could not find radio button matching {"label":"opt4"}/);
154142
});
155143
});
156144

src/material/schematics/ng-add/index.spec.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -244,15 +244,8 @@ describe('ng-add schematic', () => {
244244
it('should throw an error if the "build" target has been changed', async () => {
245245
overwriteTargetBuilder(appTree, 'build', 'thirdparty-builder');
246246

247-
let message: string|null = null;
248-
249-
try {
250-
await runner.runSchematicAsync('ng-add-setup-project', {}, appTree).toPromise();
251-
} catch (e) {
252-
message = e.message;
253-
}
254-
255-
expect(message).toMatch(/not using the default builders.*build/);
247+
await expectAsync(runner.runSchematicAsync('ng-add-setup-project', {}, appTree).toPromise())
248+
.toBeRejectedWithError(/not using the default builders.*build/);
256249
});
257250

258251
it('should warn if the "test" target has been changed', async () => {

src/material/schematics/ng-generate/address-form/index.spec.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,10 @@ describe('Material address-form schematic', () => {
4343

4444
it('should throw if no name has been specified', async () => {
4545
const appTree = await createTestApp(runner);
46-
let message: string|null = null;
4746

48-
try {
49-
await runner.runSchematicAsync('address-form', {project: 'material'}, appTree).toPromise();
50-
} catch (e) {
51-
message = e.message;
52-
}
53-
54-
expect(message).toMatch(/required property 'name'/);
47+
await expectAsync(
48+
runner.runSchematicAsync('address-form', {project: 'material'}, appTree).toPromise())
49+
.toBeRejectedWithError(/required property 'name'/);
5550
});
5651

5752
describe('style option', () => {

src/material/schematics/ng-generate/dashboard/index.spec.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,10 @@ describe('material-dashboard-schematic', () => {
5454

5555
it('should throw if no name has been specified', async () => {
5656
const appTree = await createTestApp(runner);
57-
let message: string|null = null;
5857

59-
try {
60-
await runner.runSchematicAsync('dashboard', {project: 'material'}, appTree).toPromise();
61-
} catch (e) {
62-
message = e.message;
63-
}
64-
65-
expect(message).toMatch(/required property 'name'/);
58+
await expectAsync(
59+
runner.runSchematicAsync('dashboard', {project: 'material'}, appTree).toPromise())
60+
.toBeRejectedWithError(/required property 'name'/);
6661
});
6762

6863
describe('style option', () => {

src/material/schematics/ng-generate/navigation/index.spec.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,10 @@ describe('material-navigation-schematic', () => {
6262

6363
it('should throw if no name has been specified', async () => {
6464
const appTree = await createTestApp(runner);
65-
let message: string|null = null;
6665

67-
try {
68-
await runner.runSchematicAsync('navigation', {project: 'material'}, appTree).toPromise();
69-
} catch (e) {
70-
message = e.message;
71-
}
72-
73-
expect(message).toMatch(/required property 'name'/);
66+
await expectAsync(
67+
runner.runSchematicAsync('navigation', {project: 'material'}, appTree).toPromise())
68+
.toBeRejectedWithError(/required property 'name'/);
7469
});
7570

7671
describe('style option', () => {

0 commit comments

Comments
 (0)