From 715f2c52b0b27117ebcfe58492c769ba8232b879 Mon Sep 17 00:00:00 2001 From: "Andrew C. Dvorak" Date: Wed, 25 Jul 2018 13:03:25 -0700 Subject: [PATCH] chore(infrastructure): Fail builds on non-zero exit codes (#3211) ```bash npm run screenshot:test ``` ![image](https://user-images.githubusercontent.com/409245/43218543-87ae6c1c-8ff9-11e8-800c-e80457c0d08d.png) ![image](https://user-images.githubusercontent.com/409245/43218559-943f8aec-8ff9-11e8-9166-774102ca3794.png) --- test/screenshot/infra/commands/build.js | 16 ++++++++-------- test/screenshot/infra/commands/proto.js | 7 +++++-- test/screenshot/infra/lib/cloud-storage.js | 18 ++++-------------- test/screenshot/infra/lib/process-manager.js | 17 ++++++++++++++--- .../screenshot/infra/types/node-api-externs.js | 8 -------- test/screenshot/run.js | 4 ++-- 6 files changed, 33 insertions(+), 37 deletions(-) diff --git a/test/screenshot/infra/commands/build.js b/test/screenshot/infra/commands/build.js index 6e15aacc5a0..07d9ed75bfa 100644 --- a/test/screenshot/infra/commands/build.js +++ b/test/screenshot/infra/commands/build.js @@ -55,8 +55,8 @@ class BuildCommand { this.logger_.foldStart('screenshot.build', 'Compiling source files'); - this.buildProtoFiles_(shouldWatch); - this.buildHtmlFiles_(shouldWatch); + await this.buildProtoFiles_(shouldWatch); + await this.buildHtmlFiles_(shouldWatch); if (shouldWatch) { this.processManager_.spawnChildProcess('npm', ['run', 'screenshot:webpack', '--', '--watch']); @@ -71,12 +71,12 @@ class BuildCommand { * @param {boolean} shouldWatch * @private */ - buildProtoFiles_(shouldWatch) { - const buildRightNow = () => this.protoCommand_.runAsync(); + async buildProtoFiles_(shouldWatch) { + const buildRightNow = async () => this.protoCommand_.runAsync(shouldWatch); const buildDelayed = debounce(buildRightNow, 1000); if (!shouldWatch) { - buildRightNow(); + await buildRightNow(); return; } @@ -93,12 +93,12 @@ class BuildCommand { * @param {boolean} shouldWatch * @private */ - buildHtmlFiles_(shouldWatch) { - const buildRightNow = () => this.indexCommand_.runAsync(); + async buildHtmlFiles_(shouldWatch) { + const buildRightNow = async () => this.indexCommand_.runAsync(); const buildDelayed = debounce(buildRightNow, 1000); if (!shouldWatch) { - buildRightNow(); + await buildRightNow(); return; } diff --git a/test/screenshot/infra/commands/proto.js b/test/screenshot/infra/commands/proto.js index fa2a22cbb9f..8be1fde6323 100644 --- a/test/screenshot/infra/commands/proto.js +++ b/test/screenshot/infra/commands/proto.js @@ -24,9 +24,10 @@ const {TEST_DIR_RELATIVE_PATH} = require('../lib/constants'); class ProtoCommand { /** + * @param {boolean} isWatching * @return {!Promise} Process exit code. If no exit code is returned, `0` is assumed. */ - async runAsync() { + async runAsync(isWatching = false) { const processManager = new ProcessManager(); const protoFilePaths = glob.sync(path.join(TEST_DIR_RELATIVE_PATH, '**/*.proto')); @@ -35,7 +36,9 @@ class ProtoCommand { for (const protoFilePath of protoFilePaths) { const jsFilePath = protoFilePath.replace(/.proto$/, '.pb.js'); - processManager.spawnChildProcessSync(cmd, args.concat(`--out=${jsFilePath}`, protoFilePath)); + processManager.spawnChildProcessSync( + cmd, args.concat(`--out=${jsFilePath}`, protoFilePath), undefined, isWatching + ); } } } diff --git a/test/screenshot/infra/lib/cloud-storage.js b/test/screenshot/infra/lib/cloud-storage.js index d94d28b20e2..7b4876b7c58 100644 --- a/test/screenshot/infra/lib/cloud-storage.js +++ b/test/screenshot/infra/lib/cloud-storage.js @@ -85,28 +85,18 @@ class CloudStorage { return; } - console.log(`\nUploading ${noun} to GCS...\n`); - - /** @type {!ChildProcessSpawnResult} */ - const gsutilProcess = await this.spawnGsutilUploadProcess_(reportData, localSourceDir); - - /** @type {number} */ - const exitCode = gsutilProcess.status; - + console.log(`Uploading ${noun} to GCS...\n`); + this.spawnGsutilUploadProcess_(reportData, localSourceDir); console.log(''); - - if (exitCode !== 0) { - throw new Error(`gsutil processes exited with code ${exitCode}`); - } } /** * @param {!mdc.proto.ReportData} reportData * @param {string} localSourceDir - * @return {!Promise} + * @return {!SpawnSyncReturns} * @private */ - async spawnGsutilUploadProcess_(reportData, localSourceDir) { + spawnGsutilUploadProcess_(reportData, localSourceDir) { const removeTrailingSlash = (filePath) => filePath.replace(new RegExp('/+$'), ''); // Normalize directory paths by removing trailing slashes diff --git a/test/screenshot/infra/lib/process-manager.js b/test/screenshot/infra/lib/process-manager.js index 13971a1257a..f4d641c86e6 100644 --- a/test/screenshot/infra/lib/process-manager.js +++ b/test/screenshot/infra/lib/process-manager.js @@ -18,6 +18,7 @@ const childProcess = require('child_process'); const ps = require('ps-node'); +const {ExitCode} = require('../lib/constants'); class ProcessManager { /** @@ -46,9 +47,10 @@ class ProcessManager { * @param {string} cmd * @param {!Array} args * @param {!ChildProcessSpawnOptions=} opts - * @return {!ChildProcessSpawnResult} + * @param {boolean=} isWatching + * @return {!SpawnSyncReturns} */ - spawnChildProcessSync(cmd, args, opts = {}) { + spawnChildProcessSync(cmd, args, opts = {}, isWatching = false) { /** @type {!ChildProcessSpawnOptions} */ const defaultOpts = { stdio: 'inherit', @@ -61,7 +63,16 @@ class ProcessManager { console.log(`${cmd} ${args.join(' ')}`); - return childProcess.spawnSync(cmd, args, mergedOpts); + const result = childProcess.spawnSync(cmd, args, mergedOpts); + if (result.status !== ExitCode.OK) { + const message = `${cmd} process exited with code ${result.status}`; + if (isWatching) { + console.error(message); + } else { + throw new Error(message); + } + } + return result; } /** diff --git a/test/screenshot/infra/types/node-api-externs.js b/test/screenshot/infra/types/node-api-externs.js index 081021d010f..6ef804c90cf 100644 --- a/test/screenshot/infra/types/node-api-externs.js +++ b/test/screenshot/infra/types/node-api-externs.js @@ -34,14 +34,6 @@ * }} ChildProcessSpawnOptions */ -/** - * @typedef {{ - * status: number, - * signal: ?string, - * pid: number, - * }} ChildProcessSpawnResult - */ - /* * Resemble.js API diff --git a/test/screenshot/run.js b/test/screenshot/run.js index d5cee2e99fa..36ee21e3ac7 100644 --- a/test/screenshot/run.js +++ b/test/screenshot/run.js @@ -87,7 +87,7 @@ process.on('exit', () => { }); // TODO(acdvorak): Create a centralized class to manage global exit handlers -process.on('unhandledRejection', (error) => { +process.on('unhandledRejection', (err) => { const message = [ 'UnhandledPromiseRejectionWarning: Unhandled promise rejection.', 'This error originated either by throwing inside of an async function without a catch block,', @@ -95,7 +95,7 @@ process.on('unhandledRejection', (error) => { ].join(' '); console.error('\n'); console.error(message); - console.error(error); + console.error(formatError(err)); process.exit(ExitCode.UNHANDLED_PROMISE_REJECTION); });