ci: fix the payload-size checking scripts (#20683)

The scripts were accidentally broken in #20524. More specifically, when a limit
was exceeded the script would break while trying to log an error message due to
a missing `commit` variable.
This commit fixes it and also does some minor clean-up (improve docs, use more
descriptive variable names, remove dead code, etc).

PR Close #20683

aio/scripts/_payload-limits.json

@@ -1,3 +1,21 @@
 {
-"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37068}}}
+  "aio": {
+    "master": {
+      "gzip7": {
+        "inline": 925,
+        "main": 119519,
+        "polyfills": 11863
+      },
+      "gzip9": {
+        "inline": 925,
+        "main": 119301,
+        "polyfills": 11861
+      },
+      "uncompressed": {
+        "inline": 1533,
+        "main": 486493,
+        "polyfills": 37068
+      }
+    }
+  }
 }

aio/scripts/payload.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 set -eu -o pipefail
 

scripts/ci/payload-size.js

@@ -1,30 +1,32 @@
+'use strict';
 
+// Imports
 const fs = require('fs');
-// Get branch and project name from command line arguments
-const [, , limitFile, project, branch] = process.argv;
 
-const limit = JSON.parse(fs.readFileSync(limitFile, 'utf8'));
+// Get branch and project name from command line arguments.
-const current = JSON.parse(fs.readFileSync('/tmp/current.log', 'utf8'));
+const [, , limitFile, project, branch, commit] = process.argv;
 
+// Load sizes.
+const currentSizes = JSON.parse(fs.readFileSync('/tmp/current.log', 'utf8'));
+const allLimitSizes = JSON.parse(fs.readFileSync(limitFile, 'utf8'));
+const limitSizes = allLimitSizes[project][branch] || allLimitSizes[project]['master'];
 
-const limitData = limit[project][branch] || limit[project]["master"];
+// Check current sizes against limits.
-  
-if (!limitData) {
-  console.error(`No limit data found.`);
-  // If no payload limit file found, we don't need to check the size
-  exit(0);
-}
-
 let failed = false;
-for (let compressionType in limitData) {
+for (const compressionType in limitSizes) {
-  if (typeof limitData[compressionType] === 'object') {
+  if (typeof limitSizes[compressionType] === 'object') {
-    for (let file in limitData[compressionType]) {
+    const limitPerFile = limitSizes[compressionType];
-      const name = `${compressionType}/${file}`;
+
-      const number = limitData[compressionType][file];
+    for (const filename in limitPerFile) {
-      if (Math.abs(current[name] - number) > number / 100) {
+      const expectedSize = limitPerFile[filename];
-        console.log(`Commit ${commit} ${compressionType} ${file} changed from ${number} to ${current[name]}.
+      const actualSize = currentSizes[`${compressionType}/${filename}`];
-            If this is a desired change, please update the payload size limits in file ${limitFile}`);
+
+      if (Math.abs(actualSize - expectedSize) > expectedSize / 100) {
         failed = true;
+        console.log(
+            `Commit ${commit} ${compressionType} ${filename} exceeded expected size by >1% ` +
+            `(expected: ${expectedSize}, actual: ${actualSize}).\n` +
+            `If this is a desired change, please update the size limits in file '${limitFile}'.`);
       }
     }
   }
@@ -33,5 +35,5 @@ for (let compressionType in limitData) {
 if (failed) {
   process.exit(1);
 } else {
-  console.log(`Payload size 1% check okay`);
+  console.log('Payload size <1% change check passed.');
 }

scripts/ci/payload-size.sh

@@ -1,5 +1,7 @@
 #!/usr/bin/env bash
 
+set -eu -o pipefail
+
 readonly PROJECT_NAME="angular-payload-size"
 
 # Calculate the size of target file uncompressed size, gzip7 size, gzip9 size
@@ -18,23 +20,24 @@ calculateSize() {
   payloadData="$payloadData\"gzip9/$label\": ${size["gzip9"]}, "
 }
 
-# Check whether the file size is under limit
+# Check whether the file size is under limit.
-# Write to global variable $failed
+# Exit with an error if limit is exceeded.
-# Read from global variables $size, $size7, $size9, $label, $limitUncompressed
+#   $1: string - The name in database.
+#   $2: string - The payload size limit file.
 checkSize() {
   name="$1"
   limitFile="$2"
-  node ${PROJECT_ROOT}/scripts/ci/payload-size.js $limitFile $name $TRAVIS_BRANCH
+  node ${PROJECT_ROOT}/scripts/ci/payload-size.js $limitFile $name $TRAVIS_BRANCH $TRAVIS_COMMIT
 }
 
-# Write timestamp to global variable $payloadData
+# Write timestamp to global variable `$payloadData`.
 addTimestamp() {
   # Add Timestamp
   timestamp=$(date +%s)
   payloadData="$payloadData\"timestamp\": $timestamp, "
 }
 
-# Write travis commit message to global variable $payloadData
+# Write travis commit message to global variable `$payloadData`.
 addMessage() {
   # Grab the set of SHAs for the message. This can fail when you force push or do initial build
   # because $TRAVIS_COMMIT_RANGE will contain the previous SHA which will not be in the
@@ -44,9 +47,9 @@ addMessage() {
   payloadData="$payloadData\"message\": \"$message\""
 }
 
-# Add change source: application, dependencies, or 'application+dependencies'
+# Add change source: `application`, `dependencies`, or `application+dependencies`
-# Read from global variables $parentDir
+# Read from global variable `$parentDir`.
-# Update the change source to global variable $payloadData
+# Update the change source in global variable `$payloadData`.
 addChange() {
   yarnChanged=false
   allChangedFiles=$(git diff --name-only $TRAVIS_COMMIT_RANGE $parentDir | wc -l)
@@ -70,8 +73,8 @@ addChange() {
   payloadData="$payloadData\"change\": \"$change\", "
 }
 
-# Upload data to firebase database if it's commit, print out data for pull
+# Upload data to firebase database if it's commit, print out data for pull requests.
-# requests
+#   $1: string - The name in database.
 uploadData() {
   name="$1"
   payloadData="{${payloadData}}"
@@ -91,18 +94,21 @@ uploadData() {
   fi
 }
 
-# Track payload size, $1 is the name in database, $2 is the file path
+# Track payload size.
-# $3 is whether we check the payload size and fail the test if the size exceeds
+#   $1: string       - The name in database.
-# limit, $4 is whether record the type of changes: true | false
+#   $2: string       - The file path.
-# $5 is the payload size limit file
+#   $3: true | false - Whether to check the payload size and fail the test if it exceeds limit.
+#   $4: true | false - Whether to record the type of changes.
+#   $5: [string]     - The payload size limit file. Only necessary if `$3` is `true`.
 trackPayloadSize() {
   name="$1"
   path="$2"
   checkSize="$3"
-  trackChange=$4
+  trackChange="$4"
+  limitFile="${5:-}"
 
   payloadData=""
-  failed=false
+
   for filename in $path; do
     declare -A size
     calculateSize
@@ -114,10 +120,6 @@ trackPayloadSize() {
   addMessage
   uploadData $name
   if [[ $checkSize = true ]]; then
-    checkSize $name "$5"
+    checkSize $name $limitFile
-  fi
-  if [[ $failed = true ]]; then
-    echo exit 1
-    exit 1
   fi
 }