Skip to content

perf(cdk/testing): reduce change detections when running parallel actions #21071

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.

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
merged 1 commit into from
Nov 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 51 additions & 1 deletion src/cdk/testing/change-detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,63 @@ export async function manualChangeDetection<T>(fn: () => Promise<T>) {
return batchChangeDetection(fn, false);
}



/**
* Resolves the given list of async values in parallel (i.e. via Promise.all) while batching change
* detection over the entire operation such that change detection occurs exactly once before
* resolving the values and once after.
* @param values A getter for the async values to resolve in parallel with batched change detection.
* @return The resolved values.
*/
export function parallel<T1, T2, T3, T4, T5>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch, is there some way to do this without having a separate declaration for each number of params?

This is what I get for putting off actually using the function in our own tests 😞

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't. I took this from TS's built-in type for Promise.all and I've seen rxjs do something similar.

values: () =>
[T1 | PromiseLike<T1>, T2 | PromiseLike<T2>, T3 | PromiseLike<T3>, T4 | PromiseLike<T4>,
T5 | PromiseLike<T5>
]): Promise<[T1, T2, T3, T4, T5]>;

/**
* Resolves the given list of async values in parallel (i.e. via Promise.all) while batching change
* detection over the entire operation such that change detection occurs exactly once before
* resolving the values and once after.
* @param values A getter for the async values to resolve in parallel with batched change detection.
* @return The resolved values.
*/
export function parallel<T1, T2, T3, T4>(
values: () =>
[T1 | PromiseLike<T1>, T2 | PromiseLike<T2>, T3 | PromiseLike<T3>, T4 | PromiseLike<T4>]):
Promise<[T1, T2, T3, T4]>;

/**
* Resolves the given list of async values in parallel (i.e. via Promise.all) while batching change
* detection over the entire operation such that change detection occurs exactly once before
* resolving the values and once after.
* @param values A getter for the async values to resolve in parallel with batched change detection.
* @return The resolved values.
*/
export async function parallel<T>(values: () => Iterable<T | PromiseLike<T>>) {
export function parallel<T1, T2, T3>(
values: () => [T1 | PromiseLike<T1>, T2 | PromiseLike<T2>, T3 | PromiseLike<T3>]):
Promise<[T1, T2, T3]>;

/**
* Resolves the given list of async values in parallel (i.e. via Promise.all) while batching change
* detection over the entire operation such that change detection occurs exactly once before
* resolving the values and once after.
* @param values A getter for the async values to resolve in parallel with batched change detection.
* @return The resolved values.
*/
export function parallel<T1, T2>(values: () => [T1 | PromiseLike<T1>, T2 | PromiseLike<T2>]):
Promise<[T1, T2]>;

/**
* Resolves the given list of async values in parallel (i.e. via Promise.all) while batching change
* detection over the entire operation such that change detection occurs exactly once before
* resolving the values and once after.
* @param values A getter for the async values to resolve in parallel with batched change detection.
* @return The resolved values.
*/
export function parallel<T>(values: () => (T | PromiseLike<T>)[]): Promise<T[]>;

export async function parallel<T>(values: () => Iterable<T | PromiseLike<T>>): Promise<T[]> {
return batchChangeDetection(() => Promise.all(values()), true);
}
4 changes: 2 additions & 2 deletions src/cdk/testing/harness-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,12 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac

const perElementMatches = await parallel(() => rawElements.map(async rawElement => {
const testElement = this.createTestElement(rawElement);
const allResultsForElement = await Promise.all(
const allResultsForElement = await parallel(
// For each query, get `null` if it doesn't match, or a `TestElement` or
// `ComponentHarness` as appropriate if it does match. This gives us everything that
// matches the current raw element, but it may contain duplicate entries (e.g.
// multiple `TestElement` or multiple `ComponentHarness` of the same type).
allQueries.map(query => this._getQueryResultForElement(
() => allQueries.map(query => this._getQueryResultForElement(
query, rawElement, testElement, skipSelectorCheck)));
return _removeDuplicateQueryResults(allResultsForElement);
}));
Expand Down
11 changes: 6 additions & 5 deletions src/cdk/testing/tests/cross-environment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ComponentHarness,
ComponentHarnessConstructor,
HarnessLoader,
parallel,
TestElement,
} from '@angular/cdk/testing';
import {MainComponentHarness} from './harnesses/main-component-harness';
Expand Down Expand Up @@ -214,7 +215,7 @@ export function crossEnvironmentSpecs(
});

it('should load optional harness with ancestor selector restriction', async () => {
const [subcomp1, subcomp2] = await Promise.all([
const [subcomp1, subcomp2] = await parallel(() => [
harness.optionalAncestorRestrictedSubcomponent(),
harness.optionalAncestorRestrictedMissingSubcomponent()
]);
Expand All @@ -224,14 +225,14 @@ export function crossEnvironmentSpecs(
});

it('should load all harnesses with ancestor selector restriction', async () => {
const [subcomps1, subcomps2] = await Promise.all([
const [subcomps1, subcomps2] = await parallel(() => [
harness.allAncestorRestrictedSubcomponent(),
harness.allAncestorRestrictedMissingSubcomponent()
]);
expect(subcomps1.length).toBe(2);
expect(subcomps2.length).toBe(0);
const [title1, title2] =
await Promise.all(subcomps1.map(async comp => (await comp.title()).text()));
await parallel(() => subcomps1.map(async comp => (await comp.title()).text()));
expect(title1).toBe('List of other 1');
expect(title2).toBe('List of other 2');
});
Expand Down Expand Up @@ -421,7 +422,7 @@ export function crossEnvironmentSpecs(
});

it('should be able to set the value of a select in single selection mode', async () => {
const [select, value, changeEventCounter] = await Promise.all([
const [select, value, changeEventCounter] = await parallel(() => [
harness.singleSelect(),
harness.singleSelectValue(),
harness.singleSelectChangeEventCounter()
Expand All @@ -434,7 +435,7 @@ export function crossEnvironmentSpecs(
});

it('should be able to set the value of a select in multi-selection mode', async () => {
const [select, value, changeEventCounter] = await Promise.all([
const [select, value, changeEventCounter] = await parallel(() => [
harness.multiSelect(),
harness.multiSelectValue(),
harness.multiSelectChangeEventCounter()
Expand Down
5 changes: 4 additions & 1 deletion src/cdk/testing/tests/protractor.e2e.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {HarnessLoader} from '@angular/cdk/testing';
import {ProtractorHarnessEnvironment} from '@angular/cdk/testing/protractor';
import {browser, by, element as protractorElement, ElementFinder} from 'protractor';
import {parallel} from '../change-detection';
import {crossEnvironmentSpecs} from './cross-environment.spec';
import {MainComponentHarness} from './harnesses/main-component-harness';

Expand Down Expand Up @@ -70,7 +71,9 @@ describe('ProtractorHarnessEnvironment', () => {
const harness = await ProtractorHarnessEnvironment.loader({queryFn: piercingQueryFn})
.getHarness(MainComponentHarness);
const shadows = await harness.shadows();
expect(await Promise.all(shadows.map(el => el.text()))).toEqual(['Shadow 1', 'Shadow 2']);
expect(await parallel(() => {
return shadows.map(el => el.text());
})).toEqual(['Shadow 1', 'Shadow 2']);
});

it('should allow querying across shadow boundary', async () => {
Expand Down
6 changes: 4 additions & 2 deletions src/cdk/testing/tests/testbed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('TestbedHarnessEnvironment', () => {
// promises.
detectChangesSpy.calls.reset();
expect(detectChangesSpy).toHaveBeenCalledTimes(0);
await parallel<unknown>(() => {
await parallel(() => {
// Chain together our promises to ensure the before clause runs first and the after clause
// runs last.
const before =
Expand Down Expand Up @@ -153,7 +153,9 @@ describe('TestbedHarnessEnvironment', () => {
fixture, MainComponentHarness, {queryFn: piercingQuerySelectorAll},
);
const shadows = await harness.shadows();
expect(await Promise.all(shadows.map(el => el.text()))).toEqual(['Shadow 1', 'Shadow 2']);
expect(await parallel(() => {
return shadows.map(el => el.text());
})).toEqual(['Shadow 1', 'Shadow 2']);
});

it('should allow querying across shadow boundary', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
ComponentHarnessConstructor,
HarnessPredicate,
HarnessQuery,
parallel,
TestElement
} from '@angular/cdk/testing';
import {FormFieldHarnessFilters} from '@angular/material/form-field/testing';
Expand Down Expand Up @@ -90,7 +91,7 @@ export class MatFormFieldHarness extends ComponentHarness {
return this.locatorForOptional(type)();
}
const hostEl = await this.host();
const [isInput, isSelect] = await Promise.all([
const [isInput, isSelect] = await parallel(() => [
hostEl.hasClass('mat-mdc-form-field-type-mat-input'),
hostEl.hasClass('mat-mdc-form-field-type-mat-select'),
]);
Expand Down Expand Up @@ -138,7 +139,7 @@ export class MatFormFieldHarness extends ComponentHarness {
async getThemeColor(): Promise<'primary'|'accent'|'warn'> {
const hostEl = await this.host();
const [isAccent, isWarn] =
await Promise.all([hostEl.hasClass('mat-accent'), hostEl.hasClass('mat-warn')]);
await parallel(() => [hostEl.hasClass('mat-accent'), hostEl.hasClass('mat-warn')]);
if (isAccent) {
return 'accent';
} else if (isWarn) {
Expand All @@ -149,12 +150,14 @@ export class MatFormFieldHarness extends ComponentHarness {

/** Gets error messages which are currently displayed in the form-field. */
async getTextErrors(): Promise<string[]> {
return Promise.all((await this._errors()).map(e => e.text()));
const errors = await this._errors();
return parallel(() => errors.map(e => e.text()));
}

/** Gets hint messages which are currently displayed in the form-field. */
async getTextHints(): Promise<string[]> {
return Promise.all((await this._hints()).map(e => e.text()));
const hints = await this._hints();
return parallel(() => hints.map(e => e.text()));
}

/**
Expand Down Expand Up @@ -240,7 +243,7 @@ export class MatFormFieldHarness extends ComponentHarness {
// is not able to forward any control status classes. Therefore if either the
// "ng-touched" or "ng-untouched" class is set, we know that it has a form control
const [isTouched, isUntouched] =
await Promise.all([hostEl.hasClass('ng-touched'), hostEl.hasClass('ng-untouched')]);
await parallel(() => [hostEl.hasClass('ng-touched'), hostEl.hasClass('ng-untouched')]);
return isTouched || isUntouched;
}
}
10 changes: 6 additions & 4 deletions src/material-experimental/mdc-list/testing/list-harness-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import {
ComponentHarness,
ComponentHarnessConstructor,
HarnessPredicate
HarnessPredicate,
parallel
} from '@angular/cdk/testing';
import {DividerHarnessFilters, MatDividerHarness} from '@angular/material/divider/testing';
import {BaseListItemHarnessFilters, SubheaderHarnessFilters} from './list-harness-filters';
Expand Down Expand Up @@ -53,8 +54,9 @@ export abstract class MatListHarnessBase<
* @return The list of items matching the given filters, grouped into sections by subheader.
*/
async getItemsGroupedBySubheader(filters?: F): Promise<ListSection<C>[]> {
const listSections = [];
let currentSection: {items: C[], heading?: Promise<string>} = {items: []};
type Section = {items: C[], heading?: Promise<string>};
const listSections: Section[] = [];
let currentSection: Section = {items: []};
const itemsAndSubheaders =
await this.getItemsWithSubheadersAndDividers({item: filters, divider: false});

Expand All @@ -74,7 +76,7 @@ export abstract class MatListHarnessBase<
}

// Concurrently wait for all sections to resolve their heading if present.
return Promise.all(listSections.map(async (s) =>
return parallel(() => listSections.map(async (s) =>
({items: s.items, heading: await s.heading})));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
ComponentHarness,
ComponentHarnessConstructor,
ContentContainerComponentHarness,
HarnessPredicate
HarnessPredicate,
parallel
} from '@angular/cdk/testing';
import {BaseListItemHarnessFilters, SubheaderHarnessFilters} from './list-harness-filters';

Expand Down Expand Up @@ -74,7 +75,8 @@ export abstract class MatListItemHarnessBase

/** Gets the lines of text (`mat-line` elements) in this nav list item. */
async getLinesText(): Promise<string[]> {
return Promise.all((await this._lines()).map(l => l.text()));
const lines = await this._lines();
return parallel(() => lines.map(l => l.text()));
}

/** Whether this list item has an avatar. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {HarnessPredicate} from '@angular/cdk/testing';
import {HarnessPredicate, parallel} from '@angular/cdk/testing';
import {MatListOptionCheckboxPosition} from '@angular/material-experimental/mdc-list';
import {MatListHarnessBase} from './list-harness-base';
import {
Expand Down Expand Up @@ -46,7 +46,7 @@ export class MatSelectionListHarness extends MatListHarnessBase<
*/
async selectItems(...filters: ListOptionHarnessFilters[]): Promise<void> {
const items = await this._getItems(filters);
await Promise.all(items.map(item => item.select()));
await parallel(() => items.map(item => item.select()));
}

/**
Expand All @@ -55,15 +55,15 @@ export class MatSelectionListHarness extends MatListHarnessBase<
*/
async deselectItems(...filters: ListItemHarnessFilters[]): Promise<void> {
const items = await this._getItems(filters);
await Promise.all(items.map(item => item.deselect()));
await parallel(() => items.map(item => item.deselect()));
}

/** Gets all items matching the given list of filters. */
private async _getItems(filters: ListOptionHarnessFilters[]): Promise<MatListOptionHarness[]> {
if (!filters.length) {
return this.getItems();
}
const matches = await Promise.all(
const matches = await parallel(() =>
filters.map(filter => this.locatorForAll(MatListOptionHarness.with(filter))()));
return matches.reduce((result, current) => [...result, ...current], []);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {HarnessPredicate} from '@angular/cdk/testing';
import {HarnessPredicate, parallel} from '@angular/cdk/testing';
import {MatFormFieldControlHarness} from '@angular/material/form-field/testing/control';
import {
MatOptionHarness,
Expand Down Expand Up @@ -118,14 +118,15 @@ export class MatSelectHarness extends MatFormFieldControlHarness {
async clickOptions(filter: OptionHarnessFilters = {}): Promise<void> {
await this.open();

const [isMultiple, options] = await Promise.all([this.isMultiple(), this.getOptions(filter)]);
const [isMultiple, options] =
await parallel(() => [this.isMultiple(), this.getOptions(filter)]);

if (options.length === 0) {
throw Error('Select does not have options matching the specified filter');
}

if (isMultiple) {
await Promise.all(options.map(option => option.click()));
await parallel(() => options.map(option => option.click()));
} else {
await options[0].click();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion';
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
import {ComponentHarness, HarnessPredicate, parallel} from '@angular/cdk/testing';
import {SliderHarnessFilters} from '@angular/material/slider/testing';

/** Harness for interacting with a MDC mat-slider in tests. */
Expand Down Expand Up @@ -95,7 +95,7 @@ export class MatSliderHarness extends ComponentHarness {
await this.waitForTasksOutsideAngular();

const [sliderEl, trackContainer] =
await Promise.all([this.host(), this._trackContainer()]);
await parallel(() => [this.host(), this._trackContainer()]);
let percentage = await this._calculatePercentage(value);
const {width} = await trackContainer.getDimensions();

Expand Down Expand Up @@ -133,7 +133,7 @@ export class MatSliderHarness extends ComponentHarness {

/** Calculates the percentage of the given value. */
private async _calculatePercentage(value: number) {
const [min, max] = await Promise.all([this.getMinValue(), this.getMaxValue()]);
const [min, max] = await parallel(() => [this.getMinValue(), this.getMaxValue()]);
return (value - min) / (max - min);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {AriaLivePoliteness} from '@angular/cdk/a11y';
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
import {ComponentHarness, HarnessPredicate, parallel} from '@angular/cdk/testing';
import {SnackBarHarnessFilters} from './snack-bar-harness-filters';

/** Harness for interacting with an MDC-based mat-snack-bar in tests. */
Expand Down Expand Up @@ -97,7 +97,7 @@ export class MatSnackBarHarness extends ComponentHarness {
// element isn't in the DOM by seeing that its width and height are zero.

const host = await this.host();
const [exit, dimensions] = await Promise.all([
const [exit, dimensions] = await parallel(() => [
// The snackbar container is marked with the "exit" attribute after it has been dismissed
// but before the animation has finished (after which it's removed from the DOM).
host.getAttribute('mat-exit'),
Expand Down
Loading