From d752e26eb2a50d0dff599f920b821121737551b5 Mon Sep 17 00:00:00 2001
From: Kara Erickson <karakara@google.com>
Date: Thu, 21 Nov 2019 11:42:11 -0800
Subject: [PATCH] ci: tighten size threshold to 1% or 500 bytes (#33969)

The size diff threshold of 1% has proven to be too lenient for us
to catch size regressions in AIO. Since the AIO main bundle is
between 400-500 KB, a size regression must be between 4-5 KB before
it will cause the tests to fail. As a result, we may merge many
changes with smaller regressions of a few KB before the size test
eventually lets us know that the number has increased. The hope is
that lowering the threshold will help us catch the smaller
regressions during code review and prevent the size tests failing at
a random later time when someone catches the size "hot potato".

PR Close #33969
---
 aio/scripts/_payload-limits.json |  2 +-
 integration/_payload-limits.json |  8 ++++----
 scripts/ci/payload-size.js       | 35 ++++++++++++++++++++++----------
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json
index a71bad77f845c..a6e33d1f100ed 100755
--- a/aio/scripts/_payload-limits.json
+++ b/aio/scripts/_payload-limits.json
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 3097,
-        "main-es2015": 436779,
+        "main-es2015": 439352,
         "polyfills-es2015": 52503
       }
     }
diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json
index 5673a9c161f37..c2f9b2f00688c 100644
--- a/integration/_payload-limits.json
+++ b/integration/_payload-limits.json
@@ -3,7 +3,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 141575,
+        "main-es2015": 142186,
         "polyfills-es2015": 36808
       }
     }
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 147719,
+        "main-es2015": 148320,
         "polyfills-es2015": 36808
       }
     }
@@ -49,7 +49,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2289,
-        "main-es2015": 227998,
+        "main-es2015": 227232,
         "polyfills-es2015": 36808,
         "5-es2015": 779
       }
@@ -60,7 +60,7 @@
       "uncompressed": {
         "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated",
         "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.",
-        "bundle": 177447
+        "bundle": 176433
       }
     }
   }
diff --git a/scripts/ci/payload-size.js b/scripts/ci/payload-size.js
index 36193d0707117..446519ff6934d 100644
--- a/scripts/ci/payload-size.js
+++ b/scripts/ci/payload-size.js
@@ -13,6 +13,8 @@ const limitSizes = allLimitSizes[project][branch] || allLimitSizes[project]['mas
 
 // Check current sizes against limits.
 let failed = false;
+const successMessages = [];
+const failureMessages = [];
 for (const compressionType in limitSizes) {
   if (typeof limitSizes[compressionType] === 'object') {
     const limitPerFile = limitSizes[compressionType];
@@ -26,26 +28,37 @@ for (const compressionType in limitSizes) {
         // An expected compression type/file combination is missing. Maybe the file was renamed or
         // removed. Report it as an error, so the user updates the corresponding limit file.
         console.log(
-            `Commit ${commit} ${compressionType} ${filename} meassurement is missing. ` +
+            `ERROR: Commit ${commit} ${compressionType} ${filename} measurement is missing. ` +
             'Maybe the file was renamed or removed.');
-      } else if (Math.abs(actualSize - expectedSize) > expectedSize / 100) {
-        failed = true;
-        // We must also catch when the size is significantly lower than the payload limit, so
-        // we are forced to update the expected payload number when the payload size reduces.
-        // Otherwise, we won't be able to catch future regressions that happen to be below
-        // the artificially inflated limit.
-        const operator = actualSize > expectedSize ? 'exceeded' : 'fell below';
-        console.log(
-            `Commit ${commit} ${compressionType} ${filename} ${operator} expected size by >1% ` +
+      } else {
+        const absoluteSizeDiff = Math.abs(actualSize - expectedSize);
+        // If size diff is larger than 1% or 500 bytes...
+        if (absoluteSizeDiff > 500 || absoluteSizeDiff > expectedSize / 100) {
+          failed = true;
+          // We must also catch when the size is significantly lower than the payload limit, so
+          // we are forced to update the expected payload number when the payload size reduces.
+          // Otherwise, we won't be able to catch future regressions that happen to be below
+          // the artificially inflated limit.
+          const operator = actualSize > expectedSize ? 'exceeded' : 'fell below';
+
+          failureMessages.push(
+            `FAIL: Commit ${commit} ${compressionType} ${filename} ${operator} expected size by 500 bytes or >1% ` +
             `(expected: ${expectedSize}, actual: ${actualSize}).`);
+        } else {
+          successMessages.push(`SUCCESS: Commit ${commit} ${compressionType} ${filename} did NOT cross size threshold of 500 bytes or >1% ` +
+            `(expected: ${expectedSize}, actual: ${actualSize}).`);
+        }
       }
     }
   }
 }
 
+// Group failure messages separately from success messages so they are easier to find.
+successMessages.concat(failureMessages).forEach(message => console.log(message));
+
 if (failed) {
   console.log(`If this is a desired change, please update the size limits in file '${limitFile}'.`);
   process.exit(1);
 } else {
-  console.log('Payload size <1% change check passed.');
+  console.log(`Payload size check passed. All diffs are less than 1% or 500 bytes.`);
 }