Skip to content

Commit

Permalink
ci: do not run benchmark measurements in circleci (angular#34753)
Browse files Browse the repository at this point in the history
Currently we run all benchmark perf tests in CircleCI. Since we do not
collect any results, we unnecessarily waste CI/RBE resources. Instead,
we should just not run benchmark perf tests in CI, but still run the
functionality e2e tests which ensure that benchmarks are not broken.

We can do this by splitting the perf and e2e tests into separate
files/targets.

PR Close angular#34753
  • Loading branch information
devversion authored and AndrewKushnir committed Jan 29, 2020
1 parent 669df70 commit 4d88b4b
Show file tree
Hide file tree
Showing 33 changed files with 344 additions and 94 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
/dist/
/bazel-out
/integration/bazel/bazel-*
e2e_test.*
*.log
node_modules

Expand Down
7 changes: 5 additions & 2 deletions modules/benchmarks/benchmark_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ load("//tools:defaults.bzl", "protractor_web_test_suite")
"""
Macro that can be used to define a benchmark test. This differentiates from
a normal Protractor test suite because we specify a custom "perf" configuration
that sets up "@angular/benchpress".
that sets up "@angular/benchpress". Benchmark test targets will not run on CI
unless explicitly requested.
"""

def benchmark_test(name, server, deps, tags = []):
Expand All @@ -15,7 +16,9 @@ def benchmark_test(name, server, deps, tags = []):
],
on_prepare = "//modules/benchmarks:start-server.js",
server = server,
tags = tags,
# Benchmark targets should not run on CI by default.
tags = tags + ["manual"],
test_suite_tags = ["manual"],
deps = [
"@npm//yargs",
] + deps,
Expand Down
17 changes: 17 additions & 0 deletions modules/benchmarks/e2e_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
load("//tools:defaults.bzl", "protractor_web_test_suite")

"""
Macro that can be used to define a e2e test in `modules/benchmarks`. Targets created through
this macro differentiate from a "benchmark_test" as they will run on CI and do not run
with `@angular/benchpress`.
"""

def e2e_test(name, server, deps, **kwargs):
protractor_web_test_suite(
name = name,
on_prepare = "//modules/benchmarks:start-server.js",
server = server,
# `yargs` is needed as runtime dependency for the e2e utils.
deps = ["@npm//yargs"] + deps,
**kwargs
)
13 changes: 12 additions & 1 deletion modules/benchmarks/src/largeform/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ package(default_visibility = ["//modules/benchmarks:__subpackages__"])
load("//tools:defaults.bzl", "ts_library")

ts_library(
name = "tests_lib",
name = "perf_tests_lib",
testonly = 1,
srcs = ["largeform_perf.spec.ts"],
tsconfig = "//modules/benchmarks:tsconfig-e2e.json",
deps = [
"//modules/e2e_util",
"@npm//protractor",
],
)

ts_library(
name = "e2e_tests_lib",
testonly = 1,
srcs = ["largeform.spec.ts"],
tsconfig = "//modules/benchmarks:tsconfig-e2e.json",
deps = [
"//modules/e2e_util",
"@npm//@types/jasminewd2",
Expand Down
29 changes: 29 additions & 0 deletions modules/benchmarks/src/largeform/largeform.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* @license
* Copyright Google Inc. 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 {$, By, element} from 'protractor';

import {openBrowser, verifyNoBrowserErrors} from '../../../e2e_util/e2e_util';

describe('largeform benchmark', () => {

afterEach(verifyNoBrowserErrors);

it('should work for ng2', () => {
openBrowser({
url: '/',
params: [{name: 'copies', value: 1}],
ignoreBrowserSynchronization: true,
});
$('#createDom').click();
expect(element.all(By.css('input[name=value0]')).get(0).getAttribute('value'))
.toBe('someValue0');
$('#destroyDom').click();
expect(element.all(By.css('input[name=value0]')).count()).toBe(0);
});
});
17 changes: 2 additions & 15 deletions modules/benchmarks/src/largeform/largeform_perf.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {$, By, element} from 'protractor';
import {$} from 'protractor';

import {openBrowser, verifyNoBrowserErrors} from '../../../e2e_util/e2e_util';
import {verifyNoBrowserErrors} from '../../../e2e_util/e2e_util';
import {runBenchmark} from '../../../e2e_util/perf_util';

interface Worker {
Expand All @@ -29,19 +29,6 @@ describe('largeform benchmark spec', () => {

afterEach(verifyNoBrowserErrors);

it('should work for ng2', () => {
openBrowser({
url: '/',
params: [{name: 'copies', value: 1}],
ignoreBrowserSynchronization: true,
});
$('#createDom').click();
expect(element.all(By.css('input[name=value0]')).get(0).getAttribute('value'))
.toBe('someValue0');
$('#destroyDom').click();
expect(element.all(By.css('input[name=value0]')).count()).toBe(0);
});

[CreateAndDestroyWorker].forEach((worker) => {
describe(worker.id, () => {
it('should run for ng2', done => {
Expand Down
9 changes: 8 additions & 1 deletion modules/benchmarks/src/largeform/ng2/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//tools:defaults.bzl", "ng_module", "ts_devserver")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

package(default_visibility = ["//modules/benchmarks:__subpackages__"])

Expand Down Expand Up @@ -38,5 +39,11 @@ ts_devserver(
benchmark_test(
name = "perf",
server = ":devserver",
deps = ["//modules/benchmarks/src/largeform:tests_lib"],
deps = ["//modules/benchmarks/src/largeform:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":devserver",
deps = ["//modules/benchmarks/src/largeform:e2e_tests_lib"],
)
13 changes: 12 additions & 1 deletion modules/benchmarks/src/largetable/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,22 @@ ts_library(
)

ts_library(
name = "perf_lib",
name = "perf_tests_lib",
testonly = 1,
srcs = ["largetable_perf.spec.ts"],
deps = [
"//modules/e2e_util",
"@npm//protractor",
],
)

ts_library(
name = "e2e_tests_lib",
testonly = 1,
srcs = ["largetable.spec.ts"],
tsconfig = "//modules/benchmarks:tsconfig-e2e.json",
deps = [
"//modules/e2e_util",
"@npm//protractor",
],
)
9 changes: 8 additions & 1 deletion modules/benchmarks/src/largetable/baseline/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//tools:defaults.bzl", "ts_devserver", "ts_library")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

package(default_visibility = ["//modules/benchmarks:__subpackages__"])

Expand All @@ -24,5 +25,11 @@ ts_devserver(
benchmark_test(
name = "perf",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:perf_lib"],
deps = ["//modules/benchmarks/src/largetable:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:e2e_tests_lib"],
)
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//tools:defaults.bzl", "ts_devserver", "ts_library")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

package(default_visibility = ["//modules/benchmarks:__subpackages__"])

Expand Down Expand Up @@ -27,5 +28,11 @@ ts_devserver(
benchmark_test(
name = "perf",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:perf_lib"],
deps = ["//modules/benchmarks/src/largetable:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:e2e_tests_lib"],
)
9 changes: 8 additions & 1 deletion modules/benchmarks/src/largetable/iv/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//tools:defaults.bzl", "ts_devserver")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

package(default_visibility = ["//modules/benchmarks:__subpackages__"])

Expand All @@ -13,5 +14,11 @@ ts_devserver(
benchmark_test(
name = "perf",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:perf_lib"],
deps = ["//modules/benchmarks/src/largetable:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:e2e_tests_lib"],
)
29 changes: 29 additions & 0 deletions modules/benchmarks/src/largetable/largetable.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* @license
* Copyright Google Inc. 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 {$} from 'protractor';

import {openBrowser, verifyNoBrowserErrors} from '../../../e2e_util/e2e_util';

describe('largetable benchmark', () => {
afterEach(verifyNoBrowserErrors);

it(`should render the table`, () => {
openBrowser({
url: '',
ignoreBrowserSynchronization: true,
params: [{name: 'cols', value: 5}, {name: 'rows', value: 5}],
});
$('#createDom').click();
expect($('#root').getText()).toContain('0/0');
$('#createDom').click();
expect($('#root').getText()).toContain('A/A');
$('#destroyDom').click();
expect($('#root').getText() as any).toEqual('');
});
});
16 changes: 0 additions & 16 deletions modules/benchmarks/src/largetable/largetable_perf.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
*/

import {$} from 'protractor';

import {openBrowser} from '../../../e2e_util/e2e_util';
import {runBenchmark, verifyNoBrowserErrors} from '../../../e2e_util/perf_util';

interface Worker {
Expand Down Expand Up @@ -48,20 +46,6 @@ describe('largetable benchmark perf', () => {

afterEach(verifyNoBrowserErrors);

it(`should render the table for ${testPackageName}`, () => {
openBrowser({
url: '',
ignoreBrowserSynchronization: true,
params: [{name: 'cols', value: 5}, {name: 'rows', value: 5}],
});
$('#createDom').click();
expect($('#root').getText()).toContain('0/0');
$('#createDom').click();
expect($('#root').getText()).toContain('A/A');
$('#destroyDom').click();
expect($('#root').getText() as any).toEqual('');
});

[CreateOnlyWorker, CreateAndDestroyWorker, UpdateWorker].forEach((worker) => {
describe(worker.id, () => {
it(`should run benchmark for ${testPackageName}`, done => {
Expand Down
9 changes: 8 additions & 1 deletion modules/benchmarks/src/largetable/ng2/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//tools:defaults.bzl", "ng_module", "ng_rollup_bundle", "ts_devserver")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

package(default_visibility = ["//modules/benchmarks:__subpackages__"])

Expand Down Expand Up @@ -40,5 +41,11 @@ ts_devserver(
benchmark_test(
name = "perf",
server = ":prodserver",
deps = ["//modules/benchmarks/src/largetable:perf_lib"],
deps = ["//modules/benchmarks/src/largetable:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":prodserver",
deps = ["//modules/benchmarks/src/largetable:e2e_tests_lib"],
)
9 changes: 8 additions & 1 deletion modules/benchmarks/src/largetable/ng2_switch/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//tools:defaults.bzl", "ng_module", "ts_devserver")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

package(default_visibility = ["//modules/benchmarks:__subpackages__"])

Expand Down Expand Up @@ -36,5 +37,11 @@ ts_devserver(
benchmark_test(
name = "perf",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:perf_lib"],
deps = ["//modules/benchmarks/src/largetable:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":devserver",
deps = ["//modules/benchmarks/src/largetable:e2e_tests_lib"],
)
10 changes: 9 additions & 1 deletion modules/benchmarks/src/largetable/render3/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package(default_visibility = ["//visibility:public"])

load("//tools:defaults.bzl", "ng_module", "ng_rollup_bundle", "ts_devserver")
load("//modules/benchmarks:benchmark_test.bzl", "benchmark_test")
load("//modules/benchmarks:e2e_test.bzl", "e2e_test")

ng_module(
name = "largetable_lib",
Expand Down Expand Up @@ -43,5 +44,12 @@ benchmark_test(
name = "perf",
server = ":devserver",
tags = ["ivy-only"],
deps = ["//modules/benchmarks/src/largetable:perf_lib"],
deps = ["//modules/benchmarks/src/largetable:perf_tests_lib"],
)

e2e_test(
name = "e2e",
server = ":devserver",
tags = ["ivy-only"],
deps = ["//modules/benchmarks/src/largetable:e2e_tests_lib"],
)
Loading

0 comments on commit 4d88b4b

Please sign in to comment.