Skip to content

Commit f000788

Browse files
committed
address feedback
1 parent 96f80a0 commit f000788

File tree

3 files changed

+20
-124
lines changed

3 files changed

+20
-124
lines changed

src/cdk/testing/harness-environment.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,16 +126,25 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
126126
}
127127

128128
async batchCD<T>(fn: () => Promise<T>) {
129+
// Flipping on the `isCDBatching` flag will turn all calls to `forceStabilize` into no-ops to
130+
// prevent triggering redundant change detection. However, we want to make sure we trigger
131+
// change detection exactly once before flipping this flag to ensure that any pending changes
132+
// are flushed before the harness tries to read state from DOM elements.
129133
const alreadyBatching = isCDBatching;
130134
if (!alreadyBatching) {
131135
await this.forceStabilize();
132136
isCDBatching = true;
133137
}
138+
134139
const result = await fn();
140+
141+
// We also want to run change detection exactly once after the batched operation is complete.
142+
// This will ensure that any changes from interacting with DOM elements are properly flushed.
135143
if (!alreadyBatching) {
136144
isCDBatching = false;
137145
await this.forceStabilize();
138146
}
147+
139148
return result;
140149
}
141150

@@ -185,6 +194,10 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
185194
const skipSelectorCheck = (elementQueries.length === 0 && harnessTypes.size === 1) ||
186195
harnessQueries.length === 0;
187196

197+
// We want to batch change detection while we're filtering harnesses, because harness predicates
198+
// may trigger change detection by reading state from DOM elements. If not batched these change
199+
// detections would be triggered once per potential match element which could cause significant
200+
// slowdown.
188201
const perElementMatches = await this.batchCD(() =>
189202
Promise.all(rawElements.map(async rawElement => {
190203
const testElement = this.createTestElement(rawElement);

src/cdk/testing/test-harnesses.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,13 @@ 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.
160+
By default, test harnesses will run Angular's change detection before reading the state of a DOM
161+
element and after interacting with a DOM element. If you're performing a large number of operations,
162+
this may slow down your tests. To avoid running change detection for every operation, you can use
163+
the `batchCD` method on `ComponentHarness` and `HarnessLoader` to run change detection just once
164+
before the batch operation and once after. When using `Promise.all` to parallelize operations, you
165+
almost always want to use `batchCD`, as it doesn't make sense to trigger change detection multiple
166+
times in parallel.
166167

167168
For example, consider the following example of reading both the `checked` and `indeterminate` state
168169
off of a checkbox harness:

test/benchmarks/material/button-harness/testbed.perf-spec.ts

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

0 commit comments

Comments
 (0)