Skip to content
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

chore(infrastructure): Fail builds/tests on non-zero exit codes #3211

Merged
merged 1 commit into from
Jul 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions test/screenshot/infra/commands/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
7 changes: 5 additions & 2 deletions test/screenshot/infra/commands/proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ const {TEST_DIR_RELATIVE_PATH} = require('../lib/constants');

class ProtoCommand {
/**
* @param {boolean} isWatching
* @return {!Promise<number|undefined>} 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'));

Expand All @@ -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
);
}
}
}
Expand Down
18 changes: 4 additions & 14 deletions test/screenshot/infra/lib/cloud-storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<!ChildProcessSpawnResult>}
* @return {!SpawnSyncReturns}
* @private
*/
async spawnGsutilUploadProcess_(reportData, localSourceDir) {
spawnGsutilUploadProcess_(reportData, localSourceDir) {
const removeTrailingSlash = (filePath) => filePath.replace(new RegExp('/+$'), '');

// Normalize directory paths by removing trailing slashes
Expand Down
17 changes: 14 additions & 3 deletions test/screenshot/infra/lib/process-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

const childProcess = require('child_process');
const ps = require('ps-node');
const {ExitCode} = require('../lib/constants');

class ProcessManager {
/**
Expand Down Expand Up @@ -46,9 +47,10 @@ class ProcessManager {
* @param {string} cmd
* @param {!Array<string>} 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',
Expand All @@ -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;
}

/**
Expand Down
8 changes: 0 additions & 8 deletions test/screenshot/infra/types/node-api-externs.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,6 @@
* }} ChildProcessSpawnOptions
*/

/**
* @typedef {{
* status: number,
* signal: ?string,
* pid: number,
* }} ChildProcessSpawnResult
*/


/*
* Resemble.js API
Expand Down
4 changes: 2 additions & 2 deletions test/screenshot/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ 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,',
'or by rejecting a promise which was not handled with .catch().',
].join(' ');
console.error('\n');
console.error(message);
console.error(error);
console.error(formatError(err));
process.exit(ExitCode.UNHANDLED_PROMISE_REJECTION);
});

Expand Down