From 8007632eace2c2f44d1512a9469b12f8d45b6922 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Wed, 28 Feb 2024 16:21:36 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=97=20Turbo-charge=20CircleCI=20parall?= =?UTF-8?q?elization=20for=20faster=20overall=20builds=20(#39861)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Parallelize `dist` builds on CircleCI * Consolidate all the different `dist` jobs in CircleCI under a reusuable file * Split browser tests by test type --- .circleci/config.yml | 141 +++++++++++------- build-system/common/ci.js | 34 ++++- build-system/pr-check/browser-tests.js | 58 +++---- .../pr-check/bundle-size-module-build.js | 42 ------ .../pr-check/bundle-size-nomodule-build.js | 42 ------ build-system/pr-check/dist.js | 111 ++++++++++++++ build-system/pr-check/experiment-build.js | 6 +- build-system/pr-check/module-build.js | 47 ------ build-system/pr-check/nomodule-build.js | 61 -------- build-system/pr-check/parallelization.js | 44 ++++++ build-system/pr-check/unminified-build.js | 6 +- build-system/pr-check/utils.js | 47 +----- build-system/tasks/3p-vendor-helpers.js | 14 ++ build-system/tasks/build.js | 9 +- build-system/tasks/dist.js | 14 +- 15 files changed, 328 insertions(+), 348 deletions(-) delete mode 100644 build-system/pr-check/bundle-size-module-build.js delete mode 100644 build-system/pr-check/bundle-size-nomodule-build.js create mode 100644 build-system/pr-check/dist.js delete mode 100644 build-system/pr-check/module-build.js delete mode 100644 build-system/pr-check/nomodule-build.js create mode 100644 build-system/pr-check/parallelization.js diff --git a/.circleci/config.yml b/.circleci/config.yml index c311c4b9fdb8..a8e2d51596bd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -27,6 +27,24 @@ experiment_job: &experiment_job environment: FLAVOR: experiment<< parameters.exp >> +dist_job: &dist_job + parameters: + module: + description: 'Whether to build Module or Nomodule' + type: enum + enum: ['Module', 'Nomodule'] + purpose: + description: 'What is the downstream purpose of this build' + type: enum + enum: ['Test', 'Bundle Size'] + +test_types_job: &test_types_job + parameters: + test_type: + description: 'Which test type to run' + type: enum + enum: ['Unit', 'Integration', 'End-to-End'] + executors: base-docker-small: docker: @@ -236,41 +254,26 @@ jobs: name: '⭐⭐⭐ Unminified Build ⭐⭐⭐' command: node build-system/pr-check/unminified-build.js - teardown_vm - nomodule_build_test: - executor: - name: node-docker-xlarge - steps: - - setup_vm - - run: - name: '⭐⭐⭐ Nomodule Build ⭐⭐⭐' - command: node build-system/pr-check/nomodule-build.js - - teardown_vm - module_build_test: + dist: executor: name: node-docker-xlarge + <<: *dist_job + parallelism: 3 steps: - setup_vm - run: - name: '⭐⭐⭐ Module Build ⭐⭐⭐' - command: node build-system/pr-check/module-build.js + name: '⭐⭐⭐ << parameters.module >> Build (<< parameters.purpose >>) ⭐⭐⭐' + command: node build-system/pr-check/dist.js --type "<< parameters.module >> Build (<< parameters.purpose >>)" - teardown_vm - nomodule_build_bundle_size: + dist_3p: executor: name: node-docker-xlarge + <<: *dist_job steps: - setup_vm - run: - name: '⭐⭐⭐ Nomodule Build ⭐⭐⭐' - command: node build-system/pr-check/bundle-size-nomodule-build.js - - teardown_vm - module_build_bundle_size: - executor: - name: node-docker-xlarge - steps: - - setup_vm - - run: - name: '⭐⭐⭐ Module Build ⭐⭐⭐' - command: node build-system/pr-check/bundle-size-module-build.js + name: '⭐⭐⭐ << parameters.module >> 3p Build (<< parameters.purpose >>) ⭐⭐⭐' + command: node build-system/pr-check/dist.js --type "<< parameters.module >> 3p Build (<< parameters.purpose >>)" - teardown_vm bundle_size: executor: @@ -395,34 +398,37 @@ jobs: browser_tests_safari: executor: name: macos-medium + <<: *test_types_job steps: - setup_vm - enable_safari_automation - run: - name: '⭐⭐⭐ Browser Tests (Safari) ⭐⭐⭐' - command: node build-system/pr-check/browser-tests.js --browser=safari + name: '⭐⭐⭐ << parameters.test_type >> Tests (Safari) ⭐⭐⭐' + command: node build-system/pr-check/browser-tests.js --browser=safari --type=<< parameters.test_type >> - store_test_output - teardown_vm browser_tests_firefox: executor: name: node-docker-medium + <<: *test_types_job steps: - setup_vm - install_firefox - run: - name: '⭐⭐⭐ Browser Tests (Firefox) ⭐⭐⭐' - command: node build-system/pr-check/browser-tests.js --browser=firefox + name: '⭐⭐⭐ << parameters.test_type >> Tests (Firefox) ⭐⭐⭐' + command: node build-system/pr-check/browser-tests.js --browser=firefox --type=<< parameters.test_type >> - store_test_output - teardown_vm browser_tests_edge: executor: name: node-docker-medium + <<: *test_types_job steps: - setup_vm - install_edge - run: - name: '⭐⭐⭐ Browser Tests (Edge) ⭐⭐⭐' - command: node build-system/pr-check/browser-tests.js --browser=edge + name: '⭐⭐⭐ << parameters.test_type >> Tests (Edge) ⭐⭐⭐' + command: node build-system/pr-check/browser-tests.js --browser=edge --type=<< parameters.test_type >> - store_test_output - teardown_vm experiment_build: @@ -525,23 +531,21 @@ workflows: <<: *push_and_pr_builds requires: - 'Initialize Repository' - - nomodule_build_test: - name: 'Nomodule Build (Test)' - <<: *push_and_pr_builds - requires: - - 'Initialize Repository' - - module_build_test: - name: 'Module Build (Test)' - <<: *push_and_pr_builds - requires: - - 'Initialize Repository' - - nomodule_build_bundle_size: - name: 'Nomodule Build (Bundle Size)' + - dist: + matrix: + parameters: + module: ['Module', 'Nomodule'] + purpose: ['Test', 'Bundle Size'] + name: '⛓️ << matrix.module >> Build (<< matrix.purpose >>)' <<: *push_and_pr_builds requires: - 'Initialize Repository' - - module_build_bundle_size: - name: 'Module Build (Bundle Size)' + - dist_3p: + matrix: + parameters: + module: ['Module', 'Nomodule'] + purpose: ['Test', 'Bundle Size'] + name: '<< matrix.module >> 3p Build (<< matrix.purpose >>)' <<: *push_and_pr_builds requires: - 'Initialize Repository' @@ -549,8 +553,10 @@ workflows: name: 'Bundle Size' <<: *push_and_pr_builds requires: - - 'Nomodule Build (Bundle Size)' - - 'Module Build (Bundle Size)' + - '⛓️ Nomodule Build (Bundle Size)' + - 'Nomodule 3p Build (Bundle Size)' + - '⛓️ Module Build (Bundle Size)' + - 'Module 3p Build (Bundle Size)' - validator_tests: name: 'Validator Tests' <<: *push_and_pr_builds @@ -560,8 +566,10 @@ workflows: name: 'Visual Diff Tests' <<: *push_and_pr_builds requires: - - 'Module Build (Test)' - - 'Nomodule Build (Test)' + - '⛓️ Module Build (Test)' + - 'Module 3p Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - local_unit_tests: name: 'Local Unit Tests' <<: *push_and_pr_builds @@ -584,7 +592,8 @@ workflows: config: ['prod', 'canary'] <<: *push_and_pr_builds requires: - - 'Nomodule Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - module_tests: name: 'Module Tests (<< matrix.config >>)' matrix: @@ -592,29 +601,45 @@ workflows: config: ['prod', 'canary'] <<: *push_and_pr_builds requires: - - 'Nomodule Build (Test)' - - 'Module Build (Test)' + - '⛓️ Module Build (Test)' + - 'Module 3p Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - end_to_end_tests: name: '⛓️ End-to-End Tests' <<: *push_and_pr_builds requires: - - 'Nomodule Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - browser_tests_safari: - name: 'Browser Tests (Safari)' + name: '<< matrix.test_type >> Tests (Safari)' + matrix: + parameters: + test_type: ['Unit', 'Integration', 'End-to-End'] <<: *push_and_pr_builds requires: - 'Initialize for Mac OS' - - 'Nomodule Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - browser_tests_firefox: - name: 'Browser Tests (Firefox)' + name: '<< matrix.test_type >> Tests (Firefox)' + matrix: + parameters: + test_type: ['Unit', 'Integration', 'End-to-End'] <<: *push_and_pr_builds requires: - - 'Nomodule Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - browser_tests_edge: - name: 'Browser Tests (Edge)' + name: '<< matrix.test_type >> Tests (Edge)' + matrix: + parameters: + # Note: we can't run e2e tests on Edge. + test_type: ['Unit', 'Integration'] <<: *push_and_pr_builds requires: - - 'Nomodule Build (Test)' + - '⛓️ Nomodule Build (Test)' + - 'Nomodule 3p Build (Test)' - experiment_build: name: 'Experiment << matrix.exp >> Build' matrix: diff --git a/build-system/common/ci.js b/build-system/common/ci.js index f7556220248b..0d88c17b8084 100644 --- a/build-system/common/ci.js +++ b/build-system/common/ci.js @@ -184,12 +184,34 @@ function circleciPrMergeCommit() { } /** - * Returns an identifier that is unique to each CircleCI job. This is different - * from the workflow ID, which is common across all jobs in a workflow. + * Returns an identifier that is unique to each CircleCI build, discerning for + * parallelized builds + * + * Note that this is different from the workflow ID, which is common across all, + * and also different from the build number, which is common across parallelized + * builds. * @return {string} */ -function circleciBuildNumber() { - return isCircleci ? env('CIRCLE_BUILD_NUM') : ''; +function circleciUniqueBuildNumber() { + return isCircleci + ? `${env('CIRCLE_BUILD_NUM')}.${env('CIRCLE_NODE_INDEX')}` + : ''; +} + +/** + * Returns true for parallelized CircleCI builds. + * @return {boolean} + */ +function circleciIsParallelized() { + return isCircleci && env('CIRCLE_NODE_TOTAL') != '1'; +} + +/** + * Returns the parallelized build's shard number. + * @return {number} + */ +function circleciNodeIndex() { + return isCircleci ? Number(env('CIRCLE_NODE_INDEX')) : 0; } /** @@ -222,7 +244,9 @@ module.exports = { ciPullRequestBranch, ciPullRequestSha, ciPushBranch, - circleciBuildNumber, + circleciUniqueBuildNumber, + circleciIsParallelized, + circleciNodeIndex, circleciPrMergeCommit, ciRepoSlug, isCiBuild, diff --git a/build-system/pr-check/browser-tests.js b/build-system/pr-check/browser-tests.js index 0edf3cb9bfa7..3a910741b637 100644 --- a/build-system/pr-check/browser-tests.js +++ b/build-system/pr-check/browser-tests.js @@ -4,31 +4,34 @@ * @fileoverview Script that runs tests on Safari / Firefox / Edge during CI. */ -const { - skipDependentJobs, - timedExecOrDie, - timedExecOrThrow, -} = require('./utils'); -const {browser} = require('minimist')(process.argv.slice(2)); +const {skipDependentJobs, timedExecOrThrow} = require('./utils'); const {runCiJob} = require('./ci-job'); const {Targets, buildTargetsInclude} = require('./build-targets'); +const argv = require('minimist')(process.argv.slice(2)); +const {type} = argv; +const browser = argv.browser.toLowerCase(); + const jobName = 'browser-tests.js'; +const COMMANDS = { + 'Unit': `amp unit --${browser}`, + 'Integration': `amp integration --nobuild --minified --${browser}`, + 'End-to-End': `amp e2e --nobuild --minified --browsers=${browser}`, +}; + +const INDIVIDUAL_TARGET = { + 'Unit': Targets.UNIT_TEST, + 'Integration': Targets.INTEGRATION_TEST, + 'End-to-End': Targets.E2E_TEST, +}; + /** * Steps to run during push builds. */ function pushBuildWorkflow() { try { - timedExecOrThrow(`amp unit --${browser}`, 'Unit tests failed!'); - timedExecOrThrow( - `amp integration --nobuild --minified --${browser}`, - 'Integration tests failed!' - ); - timedExecOrThrow( - `amp e2e --nobuild --minified --browsers=${browser}`, - 'End-to-end tests failed!' - ); + timedExecOrThrow(COMMANDS[type], `${type} tests failed!`); } catch (e) { if (e.status) { process.exitCode = e.status; @@ -40,32 +43,15 @@ function pushBuildWorkflow() { * Steps to run during PR builds. */ function prBuildWorkflow() { - if ( - !buildTargetsInclude( - Targets.RUNTIME, - Targets.UNIT_TEST, - Targets.E2E_TEST, - Targets.INTEGRATION_TEST - ) - ) { + if (!buildTargetsInclude(Targets.RUNTIME, INDIVIDUAL_TARGET[type])) { skipDependentJobs( jobName, - 'this PR does not affect the runtime, unit tests, integration tests, or end-to-end tests' + `this PR does not affect the runtime or ${type} tests` ); return; } - if (buildTargetsInclude(Targets.RUNTIME, Targets.UNIT_TEST)) { - timedExecOrDie(`amp unit --${browser}`); - } - if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) { - timedExecOrDie(`amp integration --nobuild --minified --${browser}`); - } - if ( - buildTargetsInclude(Targets.RUNTIME, Targets.E2E_TEST) && - ['safari', 'firefox'].includes(browser) // E2E tests can't be run on Edge. - ) { - timedExecOrDie(`amp e2e --nobuild --minified --browsers=${browser}`); - } + + pushBuildWorkflow(); } runCiJob(jobName, pushBuildWorkflow, prBuildWorkflow); diff --git a/build-system/pr-check/bundle-size-module-build.js b/build-system/pr-check/bundle-size-module-build.js deleted file mode 100644 index 79b3c70e00db..000000000000 --- a/build-system/pr-check/bundle-size-module-build.js +++ /dev/null @@ -1,42 +0,0 @@ -'use strict'; - -/** - * @fileoverview Script that builds the module AMP runtime for bundle-size during CI. - */ - -const { - skipDependentJobs, - storeModuleBuildToWorkspace, - timedExecOrDie, -} = require('./utils'); -const {runCiJob} = require('./ci-job'); -const {Targets, buildTargetsInclude} = require('./build-targets'); - -const jobName = 'bundle-size-module-build.js'; - -/** - * Steps to run during push builds. - */ -function pushBuildWorkflow() { - timedExecOrDie( - 'amp dist --noconfig --esm --version_override 0000000000000 --nomanglecache' - ); - storeModuleBuildToWorkspace(); -} - -/** - * Steps to run during PR builds. - */ -function prBuildWorkflow() { - if (buildTargetsInclude(Targets.RUNTIME)) { - pushBuildWorkflow(); - } else { - skipDependentJobs( - jobName, - 'this PR does not affect the runtime', - /* gracefullyHaltNextJobs= */ false - ); - } -} - -runCiJob(jobName, pushBuildWorkflow, prBuildWorkflow); diff --git a/build-system/pr-check/bundle-size-nomodule-build.js b/build-system/pr-check/bundle-size-nomodule-build.js deleted file mode 100644 index 697c08194733..000000000000 --- a/build-system/pr-check/bundle-size-nomodule-build.js +++ /dev/null @@ -1,42 +0,0 @@ -'use strict'; - -/** - * @fileoverview Script that builds the nomodule AMP runtime for bundle-size during CI. - */ - -const { - skipDependentJobs, - storeNomoduleBuildToWorkspace, - timedExecOrDie, -} = require('./utils'); -const {runCiJob} = require('./ci-job'); -const {Targets, buildTargetsInclude} = require('./build-targets'); - -const jobName = 'bundle-size-nomodule-build.js'; - -/** - * Steps to run during push builds. - */ -function pushBuildWorkflow() { - timedExecOrDie( - 'amp dist --noconfig --version_override 0000000000000 --nomanglecache' - ); - storeNomoduleBuildToWorkspace(); -} - -/** - * Steps to run during PR builds. - */ -function prBuildWorkflow() { - if (buildTargetsInclude(Targets.RUNTIME)) { - pushBuildWorkflow(); - } else { - skipDependentJobs( - jobName, - 'this PR does not affect the runtime', - /* gracefullyHaltNextJobs= */ false - ); - } -} - -runCiJob(jobName, pushBuildWorkflow, prBuildWorkflow); diff --git a/build-system/pr-check/dist.js b/build-system/pr-check/dist.js new file mode 100644 index 000000000000..b865df7f227e --- /dev/null +++ b/build-system/pr-check/dist.js @@ -0,0 +1,111 @@ +'use strict'; + +/** + * @fileoverview Script that builds the module AMP runtime for bundle-size during CI. + */ + +const { + skipDependentJobs, + storeBuildOutputToWorkspace, + timedExecOrDie, +} = require('./utils'); +const {runCiJob} = require('./ci-job'); +const {Targets, buildTargetsInclude} = require('./build-targets'); +const {maybeParallelizeCommand} = require('./parallelization'); + +const {type} = require('minimist')(process.argv.slice(2)); + +const jobName = 'dist.js'; + +// Mapping from build type name to the command to execute. +const COMMANDS = { + 'Module Build (Test)': 'amp dist --esm --fortesting', + 'Module 3p Build (Test)': + 'amp dist --esm --fortesting --core_runtime_only --vendor_configs', + 'Nomodule Build (Test)': 'amp dist --fortesting', + 'Nomodule 3p Build (Test)': + 'amp dist --fortesting --core_runtime_only --vendor_configs', + 'Module Build (Bundle Size)': + 'amp dist --noconfig --esm --version_override 0000000000000 --nomanglecache', + 'Module 3p Build (Bundle Size)': + 'amp dist --noconfig --esm --core_runtime_only --vendor_configs --version_override 0000000000000 --nomanglecache', + 'Nomodule Build (Bundle Size)': + 'amp dist --noconfig --version_override 0000000000000 --nomanglecache', + 'Nomodule 3p Build (Bundle Size)': + 'amp dist --noconfig --core_runtime_only --vendor_configs --version_override 0000000000000 --nomanglecache', +}; + +// Mapping from Build type name to which build targets should trigger this build in pull requests. +const PR_TARGETS = { + 'Module Build (Test)': [ + Targets.RUNTIME, + Targets.INTEGRATION_TEST, + Targets.VISUAL_DIFF, + ], + 'Module 3p Build (Test)': [ + Targets.RUNTIME, + Targets.INTEGRATION_TEST, + Targets.VISUAL_DIFF, + ], + 'Nomodule Build (Test)': [ + Targets.RUNTIME, + Targets.INTEGRATION_TEST, + Targets.E2E_TEST, + Targets.VISUAL_DIFF, + ], + 'Nomodule 3p Build (Test)': [ + Targets.RUNTIME, + Targets.INTEGRATION_TEST, + Targets.E2E_TEST, + Targets.VISUAL_DIFF, + ], + 'Module Build (Bundle Size)': [Targets.RUNTIME], + 'Module 3p Build (Bundle Size)': [Targets.RUNTIME], + 'Nomodule Build (Bundle Size)': [Targets.RUNTIME], + 'Nomodule 3p Build (Bundle Size)': [Targets.RUNTIME], +}; + +// We require a special exception for running an empty visual diff build when skipping module test builds. +// Run this on the 3p Build since that one is not parallelized! +const RUN_ON_SKIP = { + 'Module 3p Build (Test)': () => { + timedExecOrDie('amp visual-diff --empty'); + }, +}; + +// We gracefully halt uneffected tests, but want to run the Bundle Size step regardless. +const gracefullyHaltNextJobs = type.endsWith('(Test)'); + +/** + * Steps to run during push builds. + */ +function pushBuildWorkflow() { + const command = maybeParallelizeCommand( + COMMANDS[type], + 'extensions/amp-*', + (results) => + `--extensions=${results.replaceAll(/\bextensions\//g, '').replaceAll(' ', ',')}` + ); + + timedExecOrDie(command); + storeBuildOutputToWorkspace(); +} + +/** + * Steps to run during PR builds. + */ +function prBuildWorkflow() { + if (buildTargetsInclude(...PR_TARGETS[type])) { + pushBuildWorkflow(); + return; + } + + RUN_ON_SKIP[type]?.(); + skipDependentJobs( + jobName, + 'this PR does not affect relevant files for this build', + gracefullyHaltNextJobs + ); +} + +runCiJob(jobName, pushBuildWorkflow, prBuildWorkflow); diff --git a/build-system/pr-check/experiment-build.js b/build-system/pr-check/experiment-build.js index 1b0f1c8d34ad..90baa9399d5a 100644 --- a/build-system/pr-check/experiment-build.js +++ b/build-system/pr-check/experiment-build.js @@ -5,8 +5,8 @@ */ const { - skipDependentJobs: skipDependentJobs, - storeExperimentBuildToWorkspace, + skipDependentJobs, + storeBuildOutputToWorkspace, timedExecOrDie, } = require('./utils'); const {experiment} = require('minimist')(process.argv.slice(2)); @@ -24,7 +24,7 @@ function pushBuildWorkflow() { const config = getExperimentConfig(experiment); const defineFlag = `--define_experiment_constant ${config.define_experiment_constant}`; timedExecOrDie(`amp dist --fortesting ${defineFlag}`); - storeExperimentBuildToWorkspace(experiment); + storeBuildOutputToWorkspace(); } /** diff --git a/build-system/pr-check/module-build.js b/build-system/pr-check/module-build.js deleted file mode 100644 index 3f359f257262..000000000000 --- a/build-system/pr-check/module-build.js +++ /dev/null @@ -1,47 +0,0 @@ -'use strict'; - -/** - * @fileoverview Script that builds the module AMP runtime during CI. - */ - -const { - skipDependentJobs, - storeModuleBuildToWorkspace, - timedExecOrDie, -} = require('./utils'); -const {runCiJob} = require('./ci-job'); -const {Targets, buildTargetsInclude} = require('./build-targets'); - -const jobName = 'module-build.js'; - -/** - * Steps to run during push builds. - */ -function pushBuildWorkflow() { - timedExecOrDie('amp dist --esm --fortesting'); - storeModuleBuildToWorkspace(); -} - -/** - * Steps to run during PR builds. - */ -function prBuildWorkflow() { - if ( - buildTargetsInclude( - Targets.RUNTIME, - Targets.INTEGRATION_TEST, - Targets.VISUAL_DIFF - ) - ) { - timedExecOrDie('amp dist --esm --fortesting'); - storeModuleBuildToWorkspace(); - } else { - timedExecOrDie('amp visual-diff --empty'); - skipDependentJobs( - jobName, - 'this PR does not affect the runtime, integration tests, or visual diff tests' - ); - } -} - -runCiJob(jobName, pushBuildWorkflow, prBuildWorkflow); diff --git a/build-system/pr-check/nomodule-build.js b/build-system/pr-check/nomodule-build.js deleted file mode 100644 index d640a8738643..000000000000 --- a/build-system/pr-check/nomodule-build.js +++ /dev/null @@ -1,61 +0,0 @@ -'use strict'; - -/** - * @fileoverview Script that builds the nomodule AMP runtime during CI. - */ - -const { - abortTimedJob, - skipDependentJobs, - startTimer, - storeNomoduleBuildToWorkspace, - timedExecOrDie, - timedExecWithError, -} = require('./utils'); -const {log} = require('../common/logging'); -const {red, yellow} = require('kleur/colors'); -const {runCiJob} = require('./ci-job'); -const {Targets, buildTargetsInclude} = require('./build-targets'); - -const jobName = 'nomodule-build.js'; - -/** - * Steps to run during push builds. - */ -function pushBuildWorkflow() { - timedExecOrDie('amp dist --fortesting'); - storeNomoduleBuildToWorkspace(); -} - -/** - * Steps to run during PR builds. - * @return {Promise} - */ -async function prBuildWorkflow() { - const startTime = startTimer(jobName); - if ( - buildTargetsInclude( - Targets.RUNTIME, - Targets.INTEGRATION_TEST, - Targets.E2E_TEST, - Targets.VISUAL_DIFF - ) - ) { - const process = timedExecWithError('amp dist --fortesting'); - if (process.status !== 0) { - const message = process?.error - ? process.error.message - : 'Unknown error, check logs'; - log(red('ERROR'), yellow(message)); - return abortTimedJob(jobName, startTime); - } - storeNomoduleBuildToWorkspace(); - } else { - skipDependentJobs( - jobName, - 'this PR does not affect the runtime, integration tests, end-to-end tests, or visual diff tests' - ); - } -} - -runCiJob(jobName, pushBuildWorkflow, prBuildWorkflow); diff --git a/build-system/pr-check/parallelization.js b/build-system/pr-check/parallelization.js new file mode 100644 index 000000000000..be12f461f616 --- /dev/null +++ b/build-system/pr-check/parallelization.js @@ -0,0 +1,44 @@ +/** + * @fileoverview helpper functions for parallelizations. + */ + +const fs = require('node:fs'); +const tempy = require('tempy'); +const {circleciIsParallelized} = require('../common/ci'); +const {timedExecOrDie} = require('./utils'); + +/** + * Splits command execution using a glob string on parallelized CircleCI builds. + * + * If no parallelization is detected, simply returns the command as-is. + * If parallelization is detected, uses the glob to add an argument to the + * command by passing it through an optional callback function. + * + * Optional callback: A function that receives a space-delimited list as + * determines by CircleCI, and returns the new argument to add as a string. + * e.g., `(items) => '--files=' + item.replaceAll(' ', ',')` will add + * '--files=x,y,z' to the command. + * + * Note: the glob returns results with spaces in them this cab get messy. + * + * @param {string} command a CLI command. e.g., `amp dist --fortesting` + * @param {string} glob e.g., `extensions/amp-*` + * @param {(results: string) => string} callback optional callback. See + * function description. + * @return {string} the CLI command that should be executed. + */ +function maybeParallelizeCommand(command, glob, callback = (s) => s) { + if (!circleciIsParallelized()) { + return command; + } + + const tempFileName = tempy.file(); + timedExecOrDie( + `circleci tests glob ${glob} | circleci tests run --command=">${tempFileName} xargs echo -n"` + ); + const globAndRunResults = fs.readFileSync(tempFileName, {encoding: 'utf-8'}); + + return `${command} ${callback(globAndRunResults)}`; +} + +module.exports = {maybeParallelizeCommand}; diff --git a/build-system/pr-check/unminified-build.js b/build-system/pr-check/unminified-build.js index dc3c49292e91..fa5198f2cb6a 100644 --- a/build-system/pr-check/unminified-build.js +++ b/build-system/pr-check/unminified-build.js @@ -6,7 +6,7 @@ const { skipDependentJobs, - storeUnminifiedBuildToWorkspace, + storeBuildOutputToWorkspace, timedExecOrDie, } = require('./utils'); const {runCiJob} = require('./ci-job'); @@ -19,7 +19,7 @@ const jobName = 'unminified-build.js'; */ function pushBuildWorkflow() { timedExecOrDie('amp build --fortesting'); - storeUnminifiedBuildToWorkspace(); + storeBuildOutputToWorkspace(); } /** @@ -28,7 +28,7 @@ function pushBuildWorkflow() { function prBuildWorkflow() { if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) { timedExecOrDie('amp build --fortesting'); - storeUnminifiedBuildToWorkspace(); + storeBuildOutputToWorkspace(); } else { skipDependentJobs( jobName, diff --git a/build-system/pr-check/utils.js b/build-system/pr-check/utils.js index f52a95c62ef4..878451fed4ee 100644 --- a/build-system/pr-check/utils.js +++ b/build-system/pr-check/utils.js @@ -3,7 +3,7 @@ const fs = require('fs-extra'); const { ciPullRequestSha, - circleciBuildNumber, + circleciUniqueBuildNumber, isCiBuild, isCircleciBuild, } = require('../common/ci'); @@ -21,10 +21,6 @@ const {exec, execOrDie, execOrThrow, execWithError} = require('../common/exec'); const {getLoggingPrefix, logWithoutTimestamp} = require('../common/logging'); const {getStdout} = require('../common/process'); -const UNMINIFIED_CONTAINER_DIRECTORY = 'unminified'; -const NOMODULE_CONTAINER_DIRECTORY = 'nomodule'; -const MODULE_CONTAINER_DIRECTORY = 'module'; - const FILELIST_PATH = '/tmp/filelist.txt'; const BUILD_OUTPUT_DIRS = ['build', 'dist', 'dist.3p', 'dist.tools']; @@ -99,7 +95,7 @@ function printChangeSummary() { function signalGracefulHalt() { if (isCircleciBuild()) { const loggingPrefix = getLoggingPrefix(); - const sentinelFile = `/tmp/workspace/.CI_GRACEFULLY_HALT_${circleciBuildNumber()}`; + const sentinelFile = `/tmp/workspace/.CI_GRACEFULLY_HALT_${circleciUniqueBuildNumber()}`; fs.closeSync(fs.openSync(sentinelFile, 'w')); logWithoutTimestamp( `${loggingPrefix} Created ${cyan(sentinelFile)} to signal graceful halt.` @@ -218,10 +214,9 @@ const timedExecOrThrow = timedExecFn(execOrThrow); /** * Stores build files to the CI workspace. - * @param {string} containerDirectory - * @private */ -function storeBuildToWorkspace_(containerDirectory) { +function storeBuildOutputToWorkspace() { + const containerDirectory = circleciUniqueBuildNumber(); if (isCircleciBuild()) { fs.ensureDirSync(`/tmp/workspace/builds/${containerDirectory}`); for (const outputDir of BUILD_OUTPUT_DIRS) { @@ -236,35 +231,6 @@ function storeBuildToWorkspace_(containerDirectory) { } } -/** - * Stores unminified build files to the CI workspace. - */ -function storeUnminifiedBuildToWorkspace() { - storeBuildToWorkspace_(UNMINIFIED_CONTAINER_DIRECTORY); -} - -/** - * Stores nomodule build files to the CI workspace. - */ -function storeNomoduleBuildToWorkspace() { - storeBuildToWorkspace_(NOMODULE_CONTAINER_DIRECTORY); -} - -/** - * Stores module build files to the CI workspace. - */ -function storeModuleBuildToWorkspace() { - storeBuildToWorkspace_(MODULE_CONTAINER_DIRECTORY); -} - -/** - * Stores an experiment's build files to the CI workspace. - * @param {string} exp one of 'experimentA', 'experimentB', or 'experimentC'. - */ -function storeExperimentBuildToWorkspace(exp) { - storeBuildToWorkspace_(exp); -} - /** * Generates a file with a comma-separated list of test file paths that CircleCI * should execute in a parallelized job shard. @@ -298,9 +264,6 @@ module.exports = { timedExecOrDie, timedExecWithError, timedExecOrThrow, - storeUnminifiedBuildToWorkspace, - storeNomoduleBuildToWorkspace, - storeModuleBuildToWorkspace, - storeExperimentBuildToWorkspace, + storeBuildOutputToWorkspace, generateCircleCiShardTestFileList, }; diff --git a/build-system/tasks/3p-vendor-helpers.js b/build-system/tasks/3p-vendor-helpers.js index 32fb18ad997f..695ee56296b3 100644 --- a/build-system/tasks/3p-vendor-helpers.js +++ b/build-system/tasks/3p-vendor-helpers.js @@ -7,8 +7,21 @@ const {VERSION} = require('../compile/internal-version'); const {watchDebounceDelay} = require('./helpers'); const {watch} = require('chokidar'); +const argv = require('minimist')(process.argv.slice(2)); + const SRCPATH = ['3p/vendors/*.js']; +/** + * Returns true when the CLI flags indicate that vendor configs should be built. + * @return {boolean} + */ +function shouldBuildVendorConfigs() { + return ( + argv.vendor_configs || + (!argv.core_runtime_only && !argv.extensions && !argv.extensions_from) + ); +} + /** * Entry point for 'amp ad-vendor-configs' * Compile all the vendor configs and drop in the dist folder @@ -123,6 +136,7 @@ function listVendors() { } module.exports = { + shouldBuildVendorConfigs, buildVendorConfigs, doBuild3pVendor, generateBundles, diff --git a/build-system/tasks/build.js b/build-system/tasks/build.js index e259d5f0b9ad..43a4609d7e58 100644 --- a/build-system/tasks/build.js +++ b/build-system/tasks/build.js @@ -10,7 +10,10 @@ const { exitCtrlcHandler, } = require('../common/ctrlcHandler'); const {buildExtensions} = require('./extension-helpers'); -const {buildVendorConfigs} = require('./3p-vendor-helpers'); +const { + buildVendorConfigs, + shouldBuildVendorConfigs, +} = require('./3p-vendor-helpers'); const {compileCss} = require('./css'); const {parseExtensionFlags} = require('./extension-helpers'); const {buildStoryLocalization} = require('./build-story-localization'); @@ -57,7 +60,7 @@ async function build() { await buildExtensions(options); // This step is to be run only during a full `amp build`. - if (!argv.core_runtime_only) { + if (shouldBuildVendorConfigs()) { await buildVendorConfigs(options); } if (!argv.watch) { @@ -80,6 +83,8 @@ build.flags = { extensions_from: 'Build only the extensions from the listed AMP(s)', noextensions: 'Build with no extensions', core_runtime_only: 'Build only the core runtime', + vendor_configs: + 'Build 3p party vendor configuration files (defaults to true unless one of --core_runtime_only, --extensions, or --extensions_from is set)', coverage: 'Add code coverage instrumentation to JS files using istanbul', version_override: 'Override the version written to AMP_CONFIG', watch: 'Watch for changes in files, re-builds when detected', diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js index 5a53d98cac68..f276e3db3c67 100644 --- a/build-system/tasks/dist.js +++ b/build-system/tasks/dist.js @@ -24,7 +24,10 @@ const { } = require('../compile/internal-version'); const {buildCompiler} = require('../compile/build-compiler'); const {buildExtensions, parseExtensionFlags} = require('./extension-helpers'); -const {buildVendorConfigs} = require('./3p-vendor-helpers'); +const { + buildVendorConfigs, + shouldBuildVendorConfigs, +} = require('./3p-vendor-helpers'); const {compileCss, copyCss} = require('./css'); const {compileJison} = require('./compile-jison'); const {formatExtractedMessages} = require('../compile/log-messages'); @@ -130,12 +133,7 @@ async function dist() { await buildExtensions(options); // This step is to be run only during a full `amp dist`. - if ( - !argv.core_runtime_only && - !argv.extensions && - !argv.extensions_from && - !argv.noextensions - ) { + if (shouldBuildVendorConfigs()) { await buildVendorConfigs(options); } @@ -397,6 +395,8 @@ dist.flags = { extensions_from: 'Build only the extensions from the listed AMP(s)', noextensions: 'Build with no extensions', core_runtime_only: 'Build only the core runtime', + vendor_configs: + 'Build 3p party vendor configuration files (defaults to true unless one of --core_runtime_only, --extensions, or --extensions_from is set)', full_sourcemaps: 'Include source code content in sourcemaps', sourcemap_url: 'Set a custom sourcemap URL with placeholder {version}', type: 'Point sourcemap to fetch files from the correct GitHub tag',