Address comments

Author: tinayuangao

tools/gulp/tasks/coverage.ts

@@ -2,15 +2,15 @@ import {task} from 'gulp';
 import {existsSync} from 'fs-extra';
 import {COVERAGE_RESULT_FILE} from '../constants';
 import {spawnSync} from 'child_process';
-import {isTravisPushBuild} from '../util/travis-ci';
+import {isTravisMasterBuild} from '../util/travis-ci';
 import {openFirebaseDashboardDatabase} from '../util/firebase';
 
 task('coverage:upload', () => {
   if (!existsSync(COVERAGE_RESULT_FILE)) {
     throw new Error('No coverage file has been found!');
   }
 
-  if (!isTravisPushBuild()) {
+  if (!isTravisMasterBuild()) {
     throw new Error('Coverage results will be only uploaded inside of Travis Push builds.');
   }
 

tools/gulp/tasks/payload.ts

@@ -3,7 +3,7 @@ import {join} from 'path';
 import {statSync} from 'fs';
 import {DIST_ROOT} from '../constants';
 import {spawnSync} from 'child_process';
-import {isTravisPushBuild} from '../util/travis-ci';
+import {isTravisMasterBuild} from '../util/travis-ci';
 import {openFirebaseDashboardDatabase} from '../util/firebase';
 
 const bundlesDir = join(DIST_ROOT, 'bundles');
@@ -23,7 +23,7 @@ task('payload', ['library:clean-build'], () => {
   console.log('Payload Results:', JSON.stringify(results, null, 2));
 
   // Publish the results to firebase when it runs on Travis and not as a PR.
-  if (isTravisPushBuild()) {
+  if (isTravisMasterBuild()) {
     return publishResults(results);
   }
 

tools/gulp/tasks/screenshots.ts

@@ -6,7 +6,7 @@ import * as firebase from 'firebase';
 import {
   openScreenshotsBucket,
   connectFirebaseScreenshots} from '../util/firebase';
-import {isTravisPushBuild} from '../util/travis-ci';
+import {isTravisMasterBuild} from '../util/travis-ci';
 
 const imageDiff = require('image-diff');
 
@@ -27,8 +27,8 @@ const FIREBASE_STORAGE_GOLDENS = 'goldens';
 task('screenshots', () => {
   let prNumber = process.env['TRAVIS_PULL_REQUEST'];
 
-  if (isTravisPushBuild()) {
-    // Only update goldens for build
+  if (isTravisMasterBuild()) {
+    // Only update goldens for master build
     return uploadScreenshots();
   } else if (prNumber) {
     let firebaseApp = connectFirebaseScreenshots();

tools/gulp/util/travis-ci.ts

@@ -1,4 +1,4 @@
 /** Whether gulp currently runs inside of Travis as a push. */
-export function isTravisPushBuild() {
+export function isTravisMasterBuild() {
   return process.env['TRAVIS_PULL_REQUEST'] === 'false';
 }

tools/screenshot-test/functions/data_image.ts renamed to tools/screenshot-test/functions/data-image.ts

@@ -1,6 +1,6 @@
 import * as firebaseFunctions from 'firebase-functions';
 import {writeFileSync} from 'fs';
-import {verifySecureToken} from './jwt_util';
+import {verifySecureToken} from './jwt-util';
 import {isCreateEvent} from './util/util';
 
 const gcs = require('@google-cloud/storage')();
@@ -9,11 +9,12 @@ const gcs = require('@google-cloud/storage')();
 const bucket = gcs.bucket(firebaseFunctions.config().firebase.storageBucket);
 
 /**
- * Convert data to images. Image data posted to database will be saved as png files
+ * Writes base-64 encoded test images to png files on the filesystem.
+ * Image data posted to database will be saved as png files
  * and uploaded to screenshot/$prNumber/dataType/$filename
  * Convert BufferArray to .png image file
  */
-export function convertTestImageDataToFiles(event: any) {
+export function writeTestImagesToFiles(event: any) {
   // Only edit data when it is first created. Exit when the data is deleted.
   if (!isCreateEvent(event)) {
     return;

tools/screenshot-test/functions/image_data.ts renamed to tools/screenshot-test/functions/image-data.ts

@@ -9,10 +9,10 @@ const FIREBASE_DATA_GOLDENS = 'screenshot/goldens';
 
 /**
  * Read golden files under /goldens/ and store the image data to
- * database /screenshot/goldens/$filename
+ * database /screenshot/goldens/$filename as base-64 encoded string
  * Convert png image files to BufferArray data
  */
-export function convertGoldenImagesToData(name: string, resourceState: string, fileBucket: any) {
+export function copyGoldImagesToDatabase(name: string, resourceState: string, fileBucket: any) {
   // The name should always look like "goldens/xxx.png"
   let parsedPath = path.parse(name);
   // Get the file name.

tools/screenshot-test/functions/index.ts

@@ -4,9 +4,9 @@ import * as firebaseFunctions from 'firebase-functions';
 import * as firebaseAdmin from 'firebase-admin';
 
 import {verifyJwtAndTransferResultToTrustedLocation} from './verify-and-copy-report';
-import {convertGoldenImagesToData} from './image_data';
-import {convertTestImageDataToFiles} from './data_image';
-import {copyTestImagesToGoldens} from './test_goldens';
+import {copyGoldImagesToDatabase} from './image-data';
+import {writeTestImagesToFiles} from './data-image';
+import {copyTestImagesToGoldens} from './test-goldens';
 import {updateGithubStatus} from './github';
 
 /**
@@ -99,15 +99,15 @@ export let testResults = firebaseFunctions.database.ref(testResultsPath)
  */
 const imageDataToFilePath = `${imagePath}/{dataType}/{filename}`;
 export let imageDataToFile = firebaseFunctions.database.ref(imageDataToFilePath)
-  .onWrite(convertTestImageDataToFiles);
+  .onWrite(writeTestImagesToFiles);
 
 /**
  * Copy valid goldens from storage /goldens/ to database /screenshot/goldens/
  * so we can read the goldens without credentials.
  */
 export let goldenImageToData = firebaseFunctions.storage.bucket(
     firebaseFunctions.config().firebase.storageBucket).object().onChange((event: any) => {
-  return convertGoldenImagesToData(event.data.name, event.data.resourceState, event.data.bucket);
+  return copyGoldImagesToDatabase(event.data.name, event.data.resourceState, event.data.bucket);
 });
 
 /**

tools/screenshot-test/functions/jwt_util.ts renamed to tools/screenshot-test/functions/jwt-util.ts

@@ -28,7 +28,7 @@ export function copyTestImagesToGoldens(prNumber: string) {
       });
       return failedFilenames;
     }).then((failedFilenames: string[]) => {
-      return bucket.getFiles({prefix: `screenshots/${prNumber}/test`}).then(function (data: any) {
+      return bucket.getFiles({prefix: `screenshots/${prNumber}/test`}).then((data: any) => {
         return Promise.all(data[0]
           .filter((file: any) => failedFilenames.includes(
             path.basename(file.name, '.screenshot.png')))

tools/screenshot-test/functions/test_goldens.ts renamed to tools/screenshot-test/functions/test-goldens.ts

@@ -1,5 +1,5 @@
 import * as firebaseAdmin from 'firebase-admin';
-import {verifySecureToken} from './jwt_util';
+import {verifySecureToken} from './jwt-util';
 import {isCreateEvent} from './util/util';
 
 /**