Review changes

.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

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');

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');

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.');
     }
   }
 

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.');
     }
   }
 

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);

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} && ' +

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);

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,19 +45,20 @@ 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');
     }
     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.');
     }
   }