From 7e4f7019efbfcfcb344e0c66d7d58e7607aae373 Mon Sep 17 00:00:00 2001 From: Esther Kim Date: Thu, 14 Feb 2019 17:00:38 -0500 Subject: [PATCH] Review changes --- .travis.yml | 7 ------- build-system/pr-check/build.js | 16 +++++++++------ build-system/pr-check/checks.js | 17 ++++++++++----- build-system/pr-check/dist-test.js | 18 +++++++++++----- build-system/pr-check/local-test.js | 25 +++++++++++++++-------- build-system/pr-check/remote-test.js | 11 ++++++---- build-system/pr-check/utils.js | 8 ++++---- build-system/pr-check/validator.js | 9 ++++++-- build-system/pr-check/visual-diff-test.js | 21 ++++++++++++------- 9 files changed, 84 insertions(+), 48 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4bf798530dc49..53c0101829b6d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,4 @@ language: node_js -sudo: false dist: trusty node_js: - "lts/*" @@ -14,10 +13,6 @@ notifications: webhooks: - http://savage.nonblocking.io:8080/savage/travis before_install: - #- export CHROME_BIN=google-chrome-stable - #- export DISPLAY=:99.0 - #- unset _JAVA_OPTIONS # JVM heap sizes break closure compiler. #11203. - #- sh -e /etc/init.d/xvfb start - curl -o- -L https://yarnpkg.com/install.sh | bash - export PATH="$HOME/.yarn/bin:$HOME/.config/yarn/global/node_modules/.bin:$PATH" - pip install urllib3[secure] @@ -64,7 +59,6 @@ jobs: - stage: test name: "Dist, Bundle Size, Single Pass Tests" before_install: - - export CHROME_BIN=google-chrome-stable - export DISPLAY=:99.0 - sh -e /etc/init.d/xvfb start # Explicitly add yarn and gsutil install because this section overrides before_install parent section @@ -89,7 +83,6 @@ jobs: - stage: test name: "Local Tests" before_install: - - export CHROME_BIN=google-chrome-stable - export DISPLAY=:99.0 - sh -e /etc/init.d/xvfb start # Explicitly add yarn and gsutil install because this section overrides before_install parent section diff --git a/build-system/pr-check/build.js b/build-system/pr-check/build.js index b39150d2f798e..964efbc040b95 100644 --- a/build-system/pr-check/build.js +++ b/build-system/pr-check/build.js @@ -21,7 +21,6 @@ * This is run during the CI stage = build; job = build. */ -//TODO(estherkim): move to util file const colors = require('ansi-colors'); const { gitBranchName, @@ -36,12 +35,17 @@ const { isTravisBuild, travisPullRequestSha, } = require('../travis'); +const { + startTimer, + stopTimer, + timedExecOrDie: timedExecOrDieBase, + zipBuildOutput} = require('./utils'); const {determineBuildTargets} = require('./build-target'); const {getStderr} = require('../exec'); -const {startTimer, stopTimer, timedExecOrDie, zipBuildOutput} = require('./utils'); - const FILENAME = 'build.js'; const FILELOGPREFIX = colors.bold(colors.yellow(`${FILENAME}:`)); +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); /** * Prints a summary of files changed by, and commits included in the PR. @@ -119,9 +123,9 @@ function main() { const buildTargets = determineBuildTargets(); if (buildTargets.has('RUNTIME') || - buildTargets.has('UNIT_TEST') || - buildTargets.has('INTEGRATION_TEST') || - buildTargets.has('BUILD_SYSTEM')) { + buildTargets.has('UNIT_TEST') || + buildTargets.has('INTEGRATION_TEST') || + buildTargets.has('BUILD_SYSTEM')) { timedExecOrDie('gulp update-packages'); timedExecOrDie('gulp css'); diff --git a/build-system/pr-check/checks.js b/build-system/pr-check/checks.js index 1c93faf1b66d7..a35665bb50cd1 100644 --- a/build-system/pr-check/checks.js +++ b/build-system/pr-check/checks.js @@ -22,11 +22,16 @@ * This is run during the CI stage = build; job = checks. */ +const { + startTimer, + stopTimer, + timedExecOrDie: timedExecOrDieBase} = require('./utils'); const {determineBuildTargets} = require('./build-target'); const {isTravisPushBuild} = require('../travis'); -const {startTimer, stopTimer, timedExecOrDie} = require('./utils'); const FILENAME = 'checks.js'; +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); function main() { const startTime = startTimer(FILENAME); @@ -46,15 +51,17 @@ function main() { } else { if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM')) { + buildTargets.has('BUILD_SYSTEM')) { + timedExecOrDie('gulp ava'); timedExecOrDie('node node_modules/jest/bin/jest.js'); } if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM') || - buildTargets.has('UNIT_TEST') || - buildTargets.has('INTEGRATION_TEST')) { + buildTargets.has('BUILD_SYSTEM') || + buildTargets.has('UNIT_TEST') || + buildTargets.has('INTEGRATION_TEST')) { + timedExecOrDie('gulp check-types'); timedExecOrDie('gulp caches-json'); timedExecOrDie('gulp json-syntax'); diff --git a/build-system/pr-check/dist-test.js b/build-system/pr-check/dist-test.js index 33cf1562396c1..08027d8214c94 100644 --- a/build-system/pr-check/dist-test.js +++ b/build-system/pr-check/dist-test.js @@ -21,17 +21,23 @@ * This is run during the CI stage = test; job = dist tests. */ +const { + startTimer, + stopTimer, + timedExecOrDie: timedExecOrDieBase, + unzipBuildOutput} = require('./utils'); const {determineBuildTargets} = require('./build-target'); const {isTravisPushBuild} = require('../travis'); -const {startTimer, stopTimer, timedExecOrDie, unzipBuildOutput} = require('./utils'); const FILENAME = 'dist-test.js'; +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); function runSinglePassTest_() { timedExecOrDie('rm -R dist'); timedExecOrDie('gulp dist --fortesting --single_pass --psuedonames'); timedExecOrDie('gulp test --integration' + - '--nobuild --compiled --single_pass'); + '--nobuild --compiled --single_pass --headless'); timedExecOrDie('rm -R dist'); } @@ -48,6 +54,7 @@ function main() { let ranTests = false; if (buildTargets.has('RUNTIME')) { + unzipBuildOutput(); //TODO(estherkim): does this belong here? timedExecOrDie('gulp dist --fortesting --noextensions'); timedExecOrDie('gulp bundle-size --on_pr_build'); @@ -57,15 +64,16 @@ function main() { } if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM') || - buildTargets.has('INTEGRATION_TEST')) { + buildTargets.has('BUILD_SYSTEM') || + buildTargets.has('INTEGRATION_TEST')) { + runSinglePassTest_(); ranTests = true; } if (!ranTests) { console.log('Skipping dist tests because this commit does ' + - 'not affect the runtime, build system, or integration test files.'); + 'not affect the runtime, build system, or integration test files.'); } } diff --git a/build-system/pr-check/local-test.js b/build-system/pr-check/local-test.js index 5a4bce323fe72..53e0c292a02b2 100644 --- a/build-system/pr-check/local-test.js +++ b/build-system/pr-check/local-test.js @@ -21,11 +21,17 @@ * This is run during the CI stage = test; job = local tests. */ +const { + startTimer, + stopTimer, + timedExecOrDie: timedExecOrDieBase, + unzipBuildOutput} = require('./utils'); const {determineBuildTargets} = require('./build-target'); const {isTravisPushBuild} = require('../travis'); -const {startTimer, stopTimer, timedExecOrDie, unzipBuildOutput} = require('./utils'); const FILENAME = 'local-test.js'; +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); function main() { const startTime = startTimer(FILENAME); @@ -42,21 +48,24 @@ function main() { let ranTests = false; if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM') || - buildTargets.has('UNIT_TEST')) { + buildTargets.has('BUILD_SYSTEM') || + buildTargets.has('UNIT_TEST')) { + timedExecOrDie('gulp test --nobuild --headless --local-changes'); ranTests = true; } if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM') || - buildTargets.has('INTEGRATION_TEST')) { + buildTargets.has('BUILD_SYSTEM') || + buildTargets.has('INTEGRATION_TEST')) { + timedExecOrDie('gulp test --integraton --nobuild --headless --coverage'); ranTests = true; } if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM')) { + buildTargets.has('BUILD_SYSTEM')) { + timedExecOrDie('gulp test --unit --nobuild --headless --coverage'); //TODO(estherkim): turn on when stabilized :) //timedExecOrDie('gulp e2e --nobuild'); @@ -70,8 +79,8 @@ function main() { if (!ranTests) { console.log('Skipping unit and integration tests because ' + - 'this commit not affect the runtime, build system, ' + - 'unit test files, integration test files, or the dev dashboard.'); + 'this commit not affect the runtime, build system, ' + + 'unit test files, integration test files, or the dev dashboard.'); } } diff --git a/build-system/pr-check/remote-test.js b/build-system/pr-check/remote-test.js index 4730ebaf6a076..58965a9138d74 100644 --- a/build-system/pr-check/remote-test.js +++ b/build-system/pr-check/remote-test.js @@ -21,16 +21,19 @@ * This is run during the CI stage = test; job = remote tests. */ -const {determineBuildTargets} = require('./build-target'); -const {isTravisPushBuild} = require('../travis'); -const {startTimer, +const { + startTimer, stopTimer, startSauceConnect, stopSauceConnect, - timedExecOrDie, + timedExecOrDie: timedExecOrDieBase, unzipBuildOutput} = require('./utils'); +const {determineBuildTargets} = require('./build-target'); +const {isTravisPushBuild} = require('../travis'); const FILENAME = 'remote-test.js'; +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); function main() { const startTime = startTimer(FILENAME); diff --git a/build-system/pr-check/utils.js b/build-system/pr-check/utils.js index a6d6dc0167538..05244ca344d17 100644 --- a/build-system/pr-check/utils.js +++ b/build-system/pr-check/utils.js @@ -95,11 +95,12 @@ function timedExec(cmd) { * Executes the provided command and times it. The program terminates in case of * failure. * @param {string} cmd + * @param {string} functionName */ -function timedExecOrDie(cmd) { - const startTime = startTimer(cmd); +function timedExecOrDie(cmd, functionName = 'utils.js') { + const startTime = startTimer(functionName); execOrDie(cmd); - stopTimer(cmd, startTime); + stopTimer(functionName, startTime); } @@ -118,7 +119,6 @@ function unzipBuildOutput() { } function zipBuildOutput() { - timedExecOrDie(`rm -f ${BUILD_OUTPUT_FILE}`); timedExecOrDie( `log="$(zip -r ${BUILD_OUTPUT_FILE} ${BUILD_OUTPUT_DIRS})" && ` + 'echo travis_fold:start:zip_results && echo ${log} && ' + diff --git a/build-system/pr-check/validator.js b/build-system/pr-check/validator.js index a6f8699c8121d..ff17aa793c2ff 100644 --- a/build-system/pr-check/validator.js +++ b/build-system/pr-check/validator.js @@ -21,11 +21,16 @@ * This is run during the CI stage = build; job = validator. */ +const { + startTimer, + stopTimer, + timedExecOrDie: timedExecOrDieBase} = require('./utils'); const {determineBuildTargets} = require('./build-target'); const {isTravisPushBuild} = require('../travis'); -const {startTimer, stopTimer, timedExecOrDie} = require('./utils'); const FILENAME = 'validator.js'; +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); function main() { const startTime = startTimer(FILENAME); @@ -43,7 +48,7 @@ function main() { } else { console.log('Skipping validator job because this commit does ' + - 'not affect the validator or validator web UI.'); + 'not affect the validator or validator web UI.'); } stopTimer(FILENAME, startTime); diff --git a/build-system/pr-check/visual-diff-test.js b/build-system/pr-check/visual-diff-test.js index 6dc57ddea4f0b..59732b6ac1e38 100644 --- a/build-system/pr-check/visual-diff-test.js +++ b/build-system/pr-check/visual-diff-test.js @@ -22,11 +22,17 @@ */ const atob = require('atob'); +const { + startTimer, + stopTimer, + timedExecOrDie: timedExecOrDieBase, + unzipBuildOutput} = require('./utils'); const {determineBuildTargets} = require('./build-target'); const {isTravisPushBuild} = require('../travis'); -const {startTimer, stopTimer, timedExecOrDie, unzipBuildOutput} = require('./utils'); const FILENAME = 'visual-diff-test.js'; +const timedExecOrDie = + (cmd, unusedFunctionName) => timedExecOrDieBase(cmd, FILENAME); function main() { const startTime = startTimer(FILENAME); @@ -39,10 +45,11 @@ function main() { } else { if (buildTargets.has('RUNTIME') || - buildTargets.has('BUILD_SYSTEM') || - buildTargets.has('INTEGRATION_TEST') || - buildTargets.has('VISUAL_DIFF') || - buildTargets.has('FLAG_CONFIG')) { + buildTargets.has('BUILD_SYSTEM') || + buildTargets.has('INTEGRATION_TEST') || + buildTargets.has('VISUAL_DIFF') || + buildTargets.has('FLAG_CONFIG')) { + unzipBuildOutput(); process.env['PERCY_TOKEN'] = atob(process.env.PERCY_TOKEN_ENCODED); timedExecOrDie('gulp visual-diff --nobuild'); @@ -50,8 +57,8 @@ function main() { else { timedExecOrDie('gulp visual-diff --nobuild --empty'); console.log('Skipping visual diff tests because this commit does ' + - 'not affect the runtime, build system, integration test files, ' + - 'visual diff test files, or flag config files.'); + 'not affect the runtime, build system, integration test files, ' + + 'visual diff test files, or flag config files.'); } }