Skip to content

Commit 96f80a0

Browse files
committed
perf(cdk/testing): allow batching of change detection
1 parent 98d4799 commit 96f80a0

File tree

6 files changed

+212
-20
lines changed

6 files changed

+212
-20
lines changed

src/cdk/testing/component-harness.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ export interface HarnessLoader {
101101
* @return A list instances of the given harness type.
102102
*/
103103
getAllHarnesses<T extends ComponentHarness>(query: HarnessQuery<T>): Promise<T[]>;
104+
105+
batchCD<T>(fn: () => Promise<T>): Promise<T>;
104106
}
105107

106108
/**
@@ -240,6 +242,8 @@ export interface LocatorFactory {
240242
* authors to wait for async tasks outside of the Angular zone.
241243
*/
242244
waitForTasksOutsideAngular(): Promise<void>;
245+
246+
batchCD<T>(fn: () => Promise<T>): Promise<T>;
243247
}
244248

245249
/**
@@ -373,6 +377,10 @@ export abstract class ComponentHarness {
373377
protected async waitForTasksOutsideAngular() {
374378
return this.locatorFactory.waitForTasksOutsideAngular();
375379
}
380+
381+
async batchCD<T>(fn: () => Promise<T>) {
382+
return this.locatorFactory.batchCD(fn);
383+
}
376384
}
377385

378386

@@ -486,7 +494,11 @@ export class HarnessPredicate<T extends ComponentHarness> {
486494
* @return A list of harnesses that satisfy this predicate.
487495
*/
488496
async filter(harnesses: T[]): Promise<T[]> {
489-
const results = await Promise.all(harnesses.map(h => this.evaluate(h)));
497+
if (harnesses.length === 0) {
498+
return [];
499+
}
500+
const results =
501+
await harnesses[0].batchCD(() => Promise.all(harnesses.map(h => this.evaluate(h))));
490502
return harnesses.filter((_, i) => results[i]);
491503
}
492504

@@ -497,7 +509,7 @@ export class HarnessPredicate<T extends ComponentHarness> {
497509
* and resolves to false otherwise.
498510
*/
499511
async evaluate(harness: T): Promise<boolean> {
500-
const results = await Promise.all(this._predicates.map(p => p(harness)));
512+
const results = await harness.batchCD(() => Promise.all(this._predicates.map(p => p(harness))));
501513
return results.reduce((combined, current) => combined && current, true);
502514
}
503515

src/cdk/testing/harness-environment.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ type ParsedQueries<T extends ComponentHarness> = {
3636
harnessTypes: Set<ComponentHarnessConstructor<T>>,
3737
};
3838

39+
let isCDBatching = false;
40+
3941
/**
4042
* Base harness environment class that can be extended to allow `ComponentHarness`es to be used in
4143
* different test environments (e.g. testbed, protractor, etc.). This class implements the
@@ -119,6 +121,24 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
119121
return (await this.getAllRawElements(selector)).map(e => this.createEnvironment(e));
120122
}
121123

124+
protected isCDBatching() {
125+
return isCDBatching;
126+
}
127+
128+
async batchCD<T>(fn: () => Promise<T>) {
129+
const alreadyBatching = isCDBatching;
130+
if (!alreadyBatching) {
131+
await this.forceStabilize();
132+
isCDBatching = true;
133+
}
134+
const result = await fn();
135+
if (!alreadyBatching) {
136+
isCDBatching = false;
137+
await this.forceStabilize();
138+
}
139+
return result;
140+
}
141+
122142
/** Creates a `ComponentHarness` for the given harness type with the given raw host element. */
123143
protected createComponentHarness<T extends ComponentHarness>(
124144
harnessType: ComponentHarnessConstructor<T>, element: E): T {
@@ -165,17 +185,18 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
165185
const skipSelectorCheck = (elementQueries.length === 0 && harnessTypes.size === 1) ||
166186
harnessQueries.length === 0;
167187

168-
const perElementMatches = await Promise.all(rawElements.map(async rawElement => {
169-
const testElement = this.createTestElement(rawElement);
170-
const allResultsForElement = await Promise.all(
171-
// For each query, get `null` if it doesn't match, or a `TestElement` or
172-
// `ComponentHarness` as appropriate if it does match. This gives us everything that
173-
// matches the current raw element, but it may contain duplicate entries (e.g. multiple
174-
// `TestElement` or multiple `ComponentHarness` of the same type.
175-
allQueries.map(query =>
176-
this._getQueryResultForElement(query, rawElement, testElement, skipSelectorCheck)));
177-
return _removeDuplicateQueryResults(allResultsForElement);
178-
}));
188+
const perElementMatches = await this.batchCD(() =>
189+
Promise.all(rawElements.map(async rawElement => {
190+
const testElement = this.createTestElement(rawElement);
191+
const allResultsForElement = await Promise.all(
192+
// For each query, get `null` if it doesn't match, or a `TestElement` or
193+
// `ComponentHarness` as appropriate if it does match. This gives us everything that
194+
// matches the current raw element, but it may contain duplicate entries (e.g.
195+
// multiple `TestElement` or multiple `ComponentHarness` of the same type).
196+
allQueries.map(query => this._getQueryResultForElement(
197+
query, rawElement, testElement, skipSelectorCheck)));
198+
return _removeDuplicateQueryResults(allResultsForElement);
199+
})));
179200
return ([] as any).concat(...perElementMatches);
180201
}
181202

src/cdk/testing/test-harnesses.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,23 @@ Note that `await` statements block the execution of your test until the associat
157157
resolves. When reading multiple properties of a harness it may not be necessary to block on the
158158
first before asking for the next, in these cases use `Promise.all` to parallelize.
159159

160+
Another thing to consider when parallelizing multiple operations is the component harnesses
161+
automatically trigger change detection before reading state from an element and after interacting
162+
with it. When you're running multiple operations in a `Promise.all`, you don't need change detection
163+
to be triggered multiple times. In order to optimize performance, both `HarnessLoader` and
164+
`ComponentHarness` offer a `batchCD` method that can be used to ensure change detection is only
165+
triggered once before the batch operation, and once after.
166+
160167
For example, consider the following example of reading both the `checked` and `indeterminate` state
161168
off of a checkbox harness:
162169

163170
```ts
164171
it('reads properties in parallel', async () => {
165172
const checkboxHarness = loader.getHarness(MyCheckboxHarness);
166-
const [checked, indeterminate] = await Promise.all([
173+
const [checked, indeterminate] = await loader.batchCD(() => Promise.all([
167174
checkboxHarness.isChecked(),
168175
checkboxHarness.isIndeterminate()
169-
]);
176+
]));
170177

171178
// ... make some assertions
172179
});

src/cdk/testing/testbed/testbed-harness-environment.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,14 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment<Element> {
7979
}
8080

8181
async forceStabilize(): Promise<void> {
82-
if (this._destroyed) {
83-
throw Error('Harness is attempting to use a fixture that has already been destroyed.');
84-
}
82+
if (!this.isCDBatching()) {
83+
if (this._destroyed) {
84+
throw Error('Harness is attempting to use a fixture that has already been destroyed.');
85+
}
8586

86-
this._fixture.detectChanges();
87-
await this._fixture.whenStable();
87+
this._fixture.detectChanges();
88+
await this._fixture.whenStable();
89+
}
8890
}
8991

9092
async waitForTasksOutsideAngular(): Promise<void> {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,38 @@ describe('TestbedHarnessEnvironment', () => {
102102
);
103103
expect(await (await harness.deepShadow()).text()).toBe('Shadow 2');
104104
});
105+
106+
it('should batch change detection', async () => {
107+
const detectChangesSpy = spyOn(fixture, 'detectChanges').and.callThrough();
108+
const harness =
109+
await TestbedHarnessEnvironment.harnessForFixture(fixture, MainComponentHarness);
110+
detectChangesSpy.calls.reset();
111+
await harness.batchCD(async () => {
112+
expect(detectChangesSpy).toHaveBeenCalledTimes(1);
113+
const button = await harness.button();
114+
await button.text();
115+
await button.click();
116+
expect(detectChangesSpy).toHaveBeenCalledTimes(1);
117+
});
118+
expect(detectChangesSpy).toHaveBeenCalledTimes(2);
119+
});
120+
121+
it('should handle nested calls to batch change detection', async () => {
122+
const detectChangesSpy = spyOn(fixture, 'detectChanges').and.callThrough();
123+
const harness =
124+
await TestbedHarnessEnvironment.harnessForFixture(fixture, MainComponentHarness);
125+
detectChangesSpy.calls.reset();
126+
await harness.batchCD(async () => {
127+
const button = await harness.button();
128+
await button.text();
129+
await button.click();
130+
await harness.batchCD(async () => {
131+
await button.text();
132+
await button.click();
133+
});
134+
});
135+
expect(detectChangesSpy).toHaveBeenCalledTimes(2);
136+
});
105137
});
106138
}
107139
});
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {HarnessLoader} from '@angular/cdk/testing';
10+
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
11+
import {Component} from '@angular/core';
12+
import {ComponentFixture, TestBed} from '@angular/core/testing';
13+
import {MatButtonModule} from '@angular/material/button';
14+
import {MatButtonHarness} from '@angular/material/button/testing/button-harness';
15+
import {MIDDLE_BUTTON, NUM_BUTTONS} from './constants';
16+
import {benchmark} from './testbed-benchmark-utilities';
17+
18+
describe('performance for the testbed harness environment', () => {
19+
let fixture: ComponentFixture<ButtonHarnessTest>;
20+
let loader: HarnessLoader;
21+
22+
beforeAll(() => {
23+
jasmine.DEFAULT_TIMEOUT_INTERVAL = 36000000;
24+
});
25+
26+
beforeEach(async () => {
27+
await TestBed.configureTestingModule({
28+
imports: [MatButtonModule],
29+
declarations: [ButtonHarnessTest],
30+
}).compileComponents();
31+
32+
fixture = TestBed.createComponent(ButtonHarnessTest);
33+
fixture.detectChanges();
34+
loader = TestbedHarnessEnvironment.loader(fixture);
35+
});
36+
37+
describe('(baseline)', () => {
38+
it('should find a button', async () => {
39+
await benchmark('(baseline) find a button', async () => {
40+
document.querySelector('button');
41+
});
42+
});
43+
44+
it('should find all buttons', async () => {
45+
await benchmark('(baseline) find all buttons', async () => {
46+
document.querySelectorAll('button');
47+
});
48+
});
49+
50+
it('should find a button via text filter', async () => {
51+
await benchmark('(baseline) find a button via text filter', async () => {
52+
return Array.from(document.querySelectorAll('button'))
53+
.filter(b => b.innerText === MIDDLE_BUTTON);
54+
});
55+
});
56+
57+
it('should click a button', async () => {
58+
const button = document.querySelector('button')!;
59+
await benchmark('(baseline) click a button', async () => {
60+
button.click();
61+
fixture.detectChanges();
62+
});
63+
});
64+
65+
it('should click all buttons', async () => {
66+
const buttons = Array.prototype.slice.call(document.querySelectorAll('button'));
67+
await benchmark('(baseline) click all buttons', async () => {
68+
buttons.forEach(button => button.click());
69+
fixture.detectChanges();
70+
});
71+
});
72+
});
73+
74+
describe('(with harness)', () => {
75+
it('should find a button', async () => {
76+
await benchmark('(with harness) find a button', async () => {
77+
await loader.getHarness(MatButtonHarness);
78+
});
79+
});
80+
81+
it('should find all buttons', async () => {
82+
await benchmark('(with harness) find all buttons', async () => {
83+
await loader.getAllHarnesses(MatButtonHarness);
84+
});
85+
});
86+
87+
it('should find a button via text filter', async () => {
88+
await benchmark('(with harness) find a button via text filter', async () => {
89+
await loader.getAllHarnesses(MatButtonHarness.with({text: MIDDLE_BUTTON}));
90+
});
91+
});
92+
93+
it('should click a button', async () => {
94+
const button = await loader.getHarness(MatButtonHarness);
95+
await benchmark('(with harness) click a button', async () => {
96+
await button.click();
97+
});
98+
});
99+
100+
it('should click all buttons', async () => {
101+
const buttons = await loader.getAllHarnesses(MatButtonHarness);
102+
await benchmark('(with harness) click all buttons', async () => {
103+
await loader.batchCD(() => Promise.all(buttons.map(button => button.click())));
104+
});
105+
});
106+
});
107+
108+
it('should fail intentionally so performance numbers are logged', fail);
109+
});
110+
111+
@Component({
112+
template: `
113+
<button *ngFor="let val of vals" mat-button> {{ val }} </button>
114+
`,
115+
})
116+
export class ButtonHarnessTest {
117+
vals = Array.from({ length: NUM_BUTTONS }, (_, i) => i);
118+
}

0 commit comments

Comments
 (0)