Skip to content

Commit

Permalink
🏗 Optimize the running of various checks during local dev and CI buil…
Browse files Browse the repository at this point in the history
…ds (#32257)
  • Loading branch information
rsimha committed Jan 28, 2021
1 parent d3d8f8f commit 4bb96e1
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 113 deletions.
79 changes: 61 additions & 18 deletions build-system/pr-check/build-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* determine which tasks are required to run for pull request builds.
*/
const config = require('../test-configs/config');
const globby = require('globby');
const minimatch = require('minimatch');
const path = require('path');
const {cyan} = require('ansi-colors');
Expand All @@ -33,6 +34,13 @@ const {isCiBuild} = require('../common/ci');
*/
let buildTargets;

/**
* Used to prevent the repeated expansion of globs during PR jobs.
*/
let lintFiles;
let presubmitFiles;
let prettifyFiles;

/***
* All of AMP's build targets that can be tested during CI.
*
Expand All @@ -45,10 +53,12 @@ const Targets = {
DEV_DASHBOARD: 'DEV_DASHBOARD',
DOCS: 'DOCS',
E2E_TEST: 'E2E_TEST',
FLAG_CONFIG: 'FLAG_CONFIG',
INTEGRATION_TEST: 'INTEGRATION_TEST',
LINT: 'LINT',
OWNERS: 'OWNERS',
PACKAGE_UPGRADE: 'PACKAGE_UPGRADE',
PRESUBMIT: 'PRESUBMIT',
PRETTIFY: 'PRETTIFY',
RENOVATE_CONFIG: 'RENOVATE_CONFIG',
RUNTIME: 'RUNTIME',
SERVER: 'SERVER',
Expand All @@ -58,6 +68,26 @@ const Targets = {
VISUAL_DIFF: 'VISUAL_DIFF',
};

/**
* Files matching these targets are known not to affect the runtime. For all
* other targets, we play safe and default to adding the RUNTIME target, which
* will trigger all the runtime tests.
*/
const nonRuntimeTargets = [
Targets.AVA,
Targets.CACHES_JSON,
Targets.DEV_DASHBOARD,
Targets.DOCS,
Targets.E2E_TEST,
Targets.INTEGRATION_TEST,
Targets.OWNERS,
Targets.RENOVATE_CONFIG,
Targets.UNIT_TEST,
Targets.VALIDATOR,
Targets.VALIDATOR_WEBUI,
Targets.VISUAL_DIFF,
];

/**
* Checks if the given file is an OWNERS file.
*
Expand Down Expand Up @@ -86,6 +116,8 @@ function isValidatorFile(file) {

/**
* A dictionary of functions that match a given file to a given build target.
* Owners files are special because they live all over the repo, so most target
* matchers must first make sure they're not matching an owners file.
*/
const targetMatchers = {
[Targets.AVA]: (file) => {
Expand Down Expand Up @@ -153,12 +185,6 @@ const targetMatchers = {
})
);
},
[Targets.FLAG_CONFIG]: (file) => {
if (isOwnersFile(file)) {
return false;
}
return file.startsWith('build-system/global-configs/');
},
[Targets.INTEGRATION_TEST]: (file) => {
if (isOwnersFile(file)) {
return false;
Expand All @@ -172,12 +198,28 @@ const targetMatchers = {
})
);
},
[Targets.LINT]: (file) => {
if (isOwnersFile(file)) {
return false;
}
return lintFiles.includes(file);
},
[Targets.OWNERS]: (file) => {
return isOwnersFile(file) || file == 'build-system/tasks/check-owners.js';
},
[Targets.PACKAGE_UPGRADE]: (file) => {
return file.endsWith('package.json') || file.endsWith('package-lock.json');
},
[Targets.PRESUBMIT]: (file) => {
if (isOwnersFile(file)) {
return false;
}
return presubmitFiles.includes(file);
},
[Targets.PRETTIFY]: (file) => {
// OWNERS files can be prettified.
return prettifyFiles.includes(file);
},
[Targets.RENOVATE_CONFIG]: (file) => {
return (
file == '.renovaterc.json' ||
Expand Down Expand Up @@ -254,34 +296,35 @@ function determineBuildTargets() {
return buildTargets;
}
buildTargets = new Set();
lintFiles = globby.sync(config.lintGlobs);
presubmitFiles = globby.sync(config.presubmitGlobs);
prettifyFiles = globby.sync(config.prettifyGlobs);
const filesChanged = gitDiffNameOnlyMaster();
for (const file of filesChanged) {
let matched = false;
let isRuntimeFile = true;
Object.keys(targetMatchers).forEach((target) => {
const matcher = targetMatchers[target];
if (matcher(file)) {
buildTargets.add(target);
matched = true;
if (target in nonRuntimeTargets) {
isRuntimeFile = false;
}
}
});
if (!matched) {
buildTargets.add(Targets.RUNTIME); // Default to RUNTIME for files that don't match a target.
if (isRuntimeFile) {
buildTargets.add(Targets.RUNTIME);
}
}
const loggingPrefix = getLoggingPrefix();
logWithoutTimestamp(
`${loggingPrefix} Detected build targets:`,
cyan(Array.from(buildTargets).sort().join(', '))
);
// Test the runtime for babel plugin and server changes.
if (
buildTargets.has(Targets.BABEL_PLUGIN) ||
buildTargets.has(Targets.SERVER)
) {
buildTargets.add(Targets.RUNTIME);
}
// Test all targets during CI builds for package upgrades.
if (isCiBuild() && buildTargets.has(Targets.PACKAGE_UPGRADE)) {
logWithoutTimestamp(
`${loggingPrefix} Running all tests since this PR contains package upgrades...`
);
const allTargets = Object.keys(targetMatchers);
allTargets.forEach((target) => buildTargets.add(target));
}
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/bundle-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function pushBuildWorkflow() {
}

function prBuildWorkflow() {
if (buildTargetsInclude(Targets.RUNTIME, Targets.FLAG_CONFIG)) {
if (buildTargetsInclude(Targets.RUNTIME)) {
downloadNomoduleOutput();
downloadModuleOutput();
timedExecOrDie('gulp bundle-size --on_pr_build');
Expand Down
25 changes: 18 additions & 7 deletions build-system/pr-check/checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ const jobName = 'checks.js';

function pushBuildWorkflow() {
timedExecOrDie('gulp update-packages');
timedExecOrDie('gulp check-exact-versions');
timedExecOrDie('gulp presubmit');
timedExecOrDie('gulp lint');
timedExecOrDie('gulp prettify');
timedExecOrDie('gulp presubmit');
timedExecOrDie('gulp ava');
timedExecOrDie('gulp babel-plugin-tests');
timedExecOrDie('gulp caches-json');
timedExecOrDie('gulp dev-dashboard-tests');
timedExecOrDie('gulp check-exact-versions');
timedExecOrDie('gulp check-renovate-config');
timedExecOrDie('gulp server-tests');
timedExecOrDie('gulp dep-check');
Expand All @@ -48,11 +48,17 @@ async function prBuildWorkflow() {
await reportAllExpectedTests();
timedExecOrDie('gulp update-packages');

timedExecOrDie('gulp check-exact-versions');
timedExecOrDie('gulp lint');
timedExecOrDie('gulp prettify');
timedExecOrDie('gulp presubmit');
timedExecOrDie('gulp performance-urls');
if (buildTargetsInclude(Targets.PRESUBMIT)) {
timedExecOrDie('gulp presubmit');
}

if (buildTargetsInclude(Targets.LINT)) {
timedExecOrDie('gulp lint');
}

if (buildTargetsInclude(Targets.PRETTIFY)) {
timedExecOrDie('gulp prettify');
}

if (buildTargetsInclude(Targets.AVA)) {
timedExecOrDie('gulp ava');
Expand Down Expand Up @@ -80,6 +86,10 @@ async function prBuildWorkflow() {
timedExecOrDie('gulp check-owners --local_changes');
}

if (buildTargetsInclude(Targets.PACKAGE_UPGRADE)) {
timedExecOrDie('gulp check-exact-versions');
}

if (buildTargetsInclude(Targets.RENOVATE_CONFIG)) {
timedExecOrDie('gulp check-renovate-config');
}
Expand All @@ -92,6 +102,7 @@ async function prBuildWorkflow() {
timedExecOrDie('gulp dep-check');
timedExecOrDie('gulp check-types');
timedExecOrDie('gulp check-sourcemaps');
timedExecOrDie('gulp performance-urls');
}
}

Expand Down
11 changes: 2 additions & 9 deletions build-system/pr-check/cross-browser-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,25 +91,18 @@ async function prBuildWorkflow() {
if (
!buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.UNIT_TEST,
Targets.INTEGRATION_TEST
)
) {
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, unit tests, or integration tests'
'this PR does not affect the runtime, unit tests, or integration tests'
);
return;
}
timedExecOrDie('gulp update-packages');
if (
buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.INTEGRATION_TEST
)
) {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
timedExecOrDie('gulp dist --fortesting');
runIntegrationTestsForPlatform();
}
Expand Down
6 changes: 2 additions & 4 deletions build-system/pr-check/e2e-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,14 @@ function pushBuildWorkflow() {
}

function prBuildWorkflow() {
if (
buildTargetsInclude(Targets.RUNTIME, Targets.FLAG_CONFIG, Targets.E2E_TEST)
) {
if (buildTargetsInclude(Targets.RUNTIME, Targets.E2E_TEST)) {
downloadNomoduleOutput();
timedExecOrDie('gulp update-packages');
timedExecOrDie('gulp e2e --nobuild --headless --compiled');
} else {
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, or end-to-end tests'
'this PR does not affect the runtime or end-to-end tests'
);
}
}
Expand Down
10 changes: 2 additions & 8 deletions build-system/pr-check/module-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,14 @@ function prBuildWorkflow() {
// TODO(#31102): This list must eventually match the same buildTargets check
// found in pr-check/nomodule-build.js as we turn on the systems that
// run against the module build. (ex. visual diffs, e2e, etc.)
if (
buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.INTEGRATION_TEST
)
) {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
timedExecOrDie('gulp update-packages');
timedExecOrDie('gulp dist --esm --fortesting');
uploadModuleOutput();
} else {
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, or integration tests'
'this PR does not affect the runtime or integration tests'
);
}
}
Expand Down
10 changes: 2 additions & 8 deletions build-system/pr-check/module-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,15 @@ function pushBuildWorkflow() {
}

function prBuildWorkflow() {
if (
buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.INTEGRATION_TEST
)
) {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
downloadNomoduleOutput();
downloadModuleOutput();
timedExecOrDie('gulp update-packages');
timedExecOrDie('gulp integration --nobuild --compiled --headless --esm');
} else {
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, or integration tests'
'this PR does not affect the runtime or integration tests'
);
}
}
Expand Down
4 changes: 1 addition & 3 deletions build-system/pr-check/nomodule-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ async function prBuildWorkflow() {
if (
buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.INTEGRATION_TEST,
Targets.E2E_TEST,
Targets.VISUAL_DIFF
Expand All @@ -70,8 +69,7 @@ async function prBuildWorkflow() {
await signalPrDeployUpload('skipped');
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, integration ' +
'tests, end-to-end tests, or visual diff tests'
'this PR does not affect the runtime, integration tests, end-to-end tests, or visual diff tests'
);
}
}
Expand Down
10 changes: 2 additions & 8 deletions build-system/pr-check/nomodule-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,14 @@ function pushBuildWorkflow() {
}

function prBuildWorkflow() {
if (
buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.INTEGRATION_TEST
)
) {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
downloadNomoduleOutput();
timedExecOrDie('gulp update-packages');
timedExecOrDie('gulp integration --nobuild --compiled --headless');
} else {
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, or integration tests'
'this PR does not affect the runtime or integration tests'
);
}
}
Expand Down
10 changes: 2 additions & 8 deletions build-system/pr-check/unminified-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,14 @@ function pushBuildWorkflow() {
}

function prBuildWorkflow() {
if (
buildTargetsInclude(
Targets.RUNTIME,
Targets.FLAG_CONFIG,
Targets.INTEGRATION_TEST
)
) {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
timedExecOrDie('gulp update-packages');
timedExecOrDie('gulp build --fortesting');
uploadUnminifiedOutput();
} else {
printSkipMessage(
jobName,
'this PR does not affect the runtime, flag configs, or integration tests'
'this PR does not affect the runtime or integration tests'
);
}
}
Expand Down

0 comments on commit 4bb96e1

Please sign in to comment.