Skip to content

test(button-harness): add performance tests for buttons using the pro… #20222

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
79 changes: 79 additions & 0 deletions test/benchmarks/material/button-harness/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
load("@npm_angular_dev_infra_private//benchmark/component_benchmark:component_benchmark.bzl", "component_benchmark")
load("//tools:defaults.bzl", "ng_test_library", "ng_web_test_suite", "ts_library")

package(default_visibility = ["//visibility:public"])

# These are two separate files that provide the same tidy interface for running a performance
# benchmark.
#
# The testbed and protractor tests use different methods for measuring time. Since Karma does not
# patch console.time, the results are not pushed back from the page to the Karma server. Instead
# we use performance.now() to measure performance.

ts_library(
name = "constants",
srcs = [":constants.ts"],
)

ts_library(
name = "protractor-benchmark-utilities",
srcs = [":protractor-benchmark-utilities.ts"],
deps = [
":constants",
"@npm//@angular/dev-infra-private",
"@npm//@types/node",
],
)

ts_library(
name = "testbed-benchmark-utilities",
srcs = [":testbed-benchmark-utilities.ts"],
deps = [":constants"],
)

# ProtractorHarnessEnvironment

component_benchmark(
name = "e2e_benchmark",
driver = ":protractor.perf-spec.ts",
driver_deps = [
":constants",
":protractor-benchmark-utilities",
"@npm//protractor",
"@npm//@types/jasmine",
"@npm//@types/node",
"//src/cdk/testing",
"//src/material/button/testing",
"//src/cdk/testing/protractor",
],
ng_deps = [
":constants",
"@npm//@angular/core",
"@npm//@angular/platform-browser",
"//src/material/button",
],
ng_srcs = [":app.module.ts"],
prefix = "",
styles = ["//src/material/prebuilt-themes:indigo-pink"],
)

# TestbedHarnessEnvironment

ng_test_library(
name = "unit_tests_lib",
srcs = ["testbed.perf-spec.ts"],
deps = [
":constants",
":testbed-benchmark-utilities",
"//src/cdk/testing",
"//src/cdk/testing/testbed",
"//src/material/button",
"//src/material/button/testing",
"@npm//@angular/dev-infra-private",
],
)

ng_web_test_suite(
name = "unit_benchmark_tests",
deps = [":unit_tests_lib"],
)
36 changes: 36 additions & 0 deletions test/benchmarks/material/button-harness/app.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {Component, NgModule, ViewEncapsulation} from '@angular/core';
import {BrowserModule} from '@angular/platform-browser';
import {MatButtonModule} from '@angular/material/button';
import {NUM_BUTTONS} from './constants';

/** component: mat-button-harness-test */

@Component({
selector: 'app-root',
template: `
<button *ngFor="let val of vals" mat-button> {{ val }} </button>
`,
encapsulation: ViewEncapsulation.None,
styleUrls: ['//src/material/core/theming/prebuilt/indigo-pink.css'],
})
export class ButtonHarnessTest {
vals = Array.from({ length: NUM_BUTTONS }, (_, i) => i);
}

@NgModule({
declarations: [ButtonHarnessTest],
imports: [
BrowserModule,
MatButtonModule,
],
bootstrap: [ButtonHarnessTest],
})
export class AppModule {}
17 changes: 17 additions & 0 deletions test/benchmarks/material/button-harness/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/


/**
* Benchpress gives us fine-grained metrics on browser interactions. In some cases, we want to
* just get a whole measurement of how long the entire interaction took.
*/
export const USE_BENCHPRESS = false;

export const NUM_BUTTONS = 25;
export const MIDDLE_BUTTON = `${Math.floor(NUM_BUTTONS / 2)}`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {runBenchmark} from '@angular/dev-infra-private/benchmark/driver-utilities';
Copy link
Member

Choose a reason for hiding this comment

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

Are these utilities specific to button? Would it make more sense to put this into something higher, or even in dev-infra? At least as part of this PR it could be moved into test/benchmarks/utils?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I'm not sure. I created the protractor and testbed benchmark utils so that their tests would be cleaner & simpler.

The testbed one uses console.warn which no other test is gonna want to use. The protractor one is more likely to be reusable since we might actually want to get the full time of a test instead of getting the sub-metrics returned by benchpress

I also think I need to go back and try to set up default params for runBenchmark again. I tried to do it a while back and surprisingly had a bunch of weird typescript errors throw

Do you think it would be sufficient to try and simplify the runBenchmark params instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think simplifying runBenchmark is what we want. I asked in the past for making ignoreBrowserSynchronization the default. I'd love us simplifying this.

Copy link
Member

Choose a reason for hiding this comment

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

Regardless of the runBenchmark API being simpler though; I think it might be good to move these shorthands (for using console.warn etc) into a higher-level because I assume we'll have more harness benchmarks, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe @mmalerba only needs this harness benchmark

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. I didn't know this is the case. They sounded quite generic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah these are not intended as performance tests for the button harness, they're intended as tests for the harness framework itself, they just happen to be using button to test that. I actually think we should change the directory name to make that more clear. How about test/benchmarks/cdk/testing/*?

Copy link
Member

Choose a reason for hiding this comment

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

That now makes sense 😄 thanks Miles for clarifying!

import {USE_BENCHPRESS} from './constants';

/**
* Records the performance of the given function.
*
* @param id A unique identifier.
* @param callback A function whose performance will be recorded.
*/
export async function benchmark(id: string, callback: () => Promise<unknown>) {
if (USE_BENCHPRESS) {
await benchmarkWithBenchpress(id, callback);
} else {
await benchmarkWithConsoleAPI(id, callback);
}
}

/**
* A simple wrapper for runBenchmark which is a wrapper for benchpress.
*/
async function benchmarkWithBenchpress(id: string, callback: () => Promise<unknown>) {
await runBenchmark({
id,
url: '',
ignoreBrowserSynchronization: true,
params: [],
work: async () => await callback(),
});
}

/**
* Measures the time it takes to invoke the given function and prints the duration to the console.
*/
async function benchmarkWithConsoleAPI(id: string, callback: () => Promise<unknown>, runs = 5) {
const t0 = Number(process.hrtime.bigint()) / 1000000;
for (let i = 0; i < runs; i++) {
await callback();
}
const t1 = Number(process.hrtime.bigint()) / 1000000;
console.warn(`${id}: ${((t1 - t0) / runs).toFixed(2)}ms (avg over ${runs} runs)`);
}
101 changes: 101 additions & 0 deletions test/benchmarks/material/button-harness/protractor.perf-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {HarnessLoader} from '@angular/cdk/testing';
import {MatButtonHarness} from '@angular/material/button/testing/button-harness';
import {ProtractorHarnessEnvironment} from '@angular/cdk/testing/protractor';
import {$$, element, by, browser} from 'protractor';
import {benchmark} from './protractor-benchmark-utilities';
import {NUM_BUTTONS} from './constants';

const FIRST_BUTTON = '0';
const MIDDLE_BUTTON = `${Math.floor(NUM_BUTTONS / 2)}`;
const LAST_BUTTON = `${NUM_BUTTONS - 1}`;

describe('baseline tests for interacting with the page through Protractor directly', () => {
beforeEach(async () => {
await browser.get('');
});

it('(baseline) should retrieve all of the buttons', async () => {
await benchmark('(baseline) get every button', async () => {
await $$('.mat-button');
});
});

it('(baseline) should click the first button', async () => {
await benchmark('(baseline) click first button', async () => {
await element(by.buttonText(FIRST_BUTTON)).click();
});
});

it('(baseline) should click the middle button', async () => {
await benchmark('(baseline) click middle button', async () => {
await element(by.buttonText(MIDDLE_BUTTON)).click();
});
});

it('(baseline) should click the last button', async () => {
await benchmark('(baseline) click last button', async () => {
await element(by.buttonText(LAST_BUTTON)).click();
});
});

it('(baseline) should click all of the buttons', async () => {
await benchmark('(baseline) click every button', async () => {
const buttons = $$('.mat-button');
await buttons.each(async (button) => await button!.click());
});
});
});

describe('performance tests for the protractor button harness', () => {
let loader: HarnessLoader;

beforeEach(async () => {
await browser.get('');
loader = ProtractorHarnessEnvironment.loader();
});

it('should retrieve all of the buttons', async () => {
await benchmark('get every button', async () => {
await loader.getAllHarnesses(MatButtonHarness);
});
});

it('should click the first button', async () => {
await benchmark('click first button', async () => {
const button = await loader.getHarness(MatButtonHarness.with({text: FIRST_BUTTON}));
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: Is it intentional that the loading/harness retrieval is inside here? I was under the impression that this benchmark is only about clicking, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to address this along with the previous comment in my next commit (in this same PR)

I want to get the bulk of the changes in first before I work on consolidating the two tests into one shared spec, and I'll be making this fix as part of that change.

await button.click();
});
});

it('should click the middle button', async () => {
await benchmark('click middle button', async () => {
const button = await loader.getHarness(MatButtonHarness.with({text: MIDDLE_BUTTON}));
await button.click();
});
});

it('should click the last button', async () => {
await benchmark('click last button', async () => {
const button = await loader.getHarness(MatButtonHarness.with({text: LAST_BUTTON}));
await button.click();
});
});

it('should click all of the buttons', async () => {
await benchmark('click every button', async () => {
const buttons = await loader.getAllHarnesses(MatButtonHarness);
for (let i = 0; i < buttons.length; i++) {
const button = buttons[i];
await button.click();
}
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

/**
* Measures the time it takes to invoke the given function and prints the duration to the console.
* @param id A unique identifier for the callback being measured.
* @param callback A function whose performance will be recorded.
* @param runs The number of times to run the callback.
*/
export async function benchmark(id: string, callback: () => Promise<unknown>, runs = 5) {
// Currently karma does not send the logs from console.time to the server so console.time will
// not show. As an alternative, we use performance.now()
const t0 = performance.now();
for (let i = 0; i < runs; i++) {
await callback();
}
const t1 = performance.now();
console.warn(`${id}: ${((t1 - t0) / runs).toFixed(2)}ms (avg over ${runs} runs)`);
}
Loading