build: size-tracking test should support max-byte threshold (#30257)

Based on discussion that happened on the PR that introduced
the size-tracking tool, we want to have another threshold for
the raw byte difference. This allows us to better control for
which changes the size-tracking tool should report a difference.

See: https://github.com/angular/angular/pull/30070#discussion_r278332315

PR Close #30257

packages/core/test/bundling/core_all/BUILD.bazel

@@ -32,8 +32,9 @@ js_size_tracking_test(
         ":bundle",
         ":bundle.golden_size_map.json",
     ],
-    diffThreshold = 3,
     goldenFile = "angular/packages/core/test/bundling/core_all/bundle.golden_size_map.json",
+    maxByteDiff = 250,
+    maxPercentageDiff = 15,
     sourceMap = "angular/packages/core/test/bundling/core_all/bundle.min.js.map",
     tags = [
         "ivy-only",

tools/size-tracking/file_size_compare.ts

@@ -13,24 +13,35 @@ export interface SizeDifference {
   message: string;
 }
 
+export interface Threshold {
+  /**
+   * Maximum difference percentage. Exceeding this causes a reported size
+   * difference. Percentage difference is helpful for small files where
+   * the byte threshold is not exceeded but the change is relatively large
+   * for the small file and should be reported.
+   */
+  maxPercentageDiff: number;
+  /**
+   * Maximum byte difference. Exceeding this causes a reported size difference.
+   * The max byte threshold works good for large files where change is relatively
+   * small but still needs to reported as it causes an overall size regression.
+   */
+  maxByteDiff: number;
+}
+
 /** Compares two file size data objects and returns an array of size differences. */
 export function compareFileSizeData(
-    actual: FileSizeData, expected: FileSizeData, threshold: number) {
+    actual: FileSizeData, expected: FileSizeData, threshold: Threshold) {
-  const diffs: SizeDifference[] = compareSizeEntry(actual.files, expected.files, '/', threshold);
+  return [
-  const unmappedBytesDiff = getDifferencePercentage(actual.unmapped, expected.unmapped);
+    ...compareSizeEntry(actual.files, expected.files, '/', threshold),
-  if (unmappedBytesDiff > threshold) {
+    ...compareActualSizeToExpected(actual.unmapped, expected.unmapped, '<unmapped>', threshold)
-    diffs.push({
+  ];
-      message: `Unmapped bytes differ by ${unmappedBytesDiff.toFixed(2)}% from ` +
-          `the expected size (actual = ${actual.unmapped}, expected = ${expected.unmapped})`
-    });
-  }
-  return diffs;
 }
 
 /** Compares two file size entries and returns an array of size differences. */
 function compareSizeEntry(
     actual: DirectorySizeEntry | number, expected: DirectorySizeEntry | number, filePath: string,
-    threshold: number) {
+    threshold: Threshold) {
   if (typeof actual !== 'number' && typeof expected !== 'number') {
     return compareDirectorySizeEntry(
         <DirectorySizeEntry>actual, <DirectorySizeEntry>expected, filePath, threshold);
@@ -40,21 +51,31 @@ function compareSizeEntry(
 }
 
 /**
- * Compares two size numbers and returns a size difference when the percentage difference
+ * Compares two size numbers and returns a size difference if the difference
- * exceeds the specified threshold.
+ * percentage exceeds the specified maximum percentage or the byte size
+ * difference exceeds the maximum byte difference.
  */
 function compareActualSizeToExpected(
     actualSize: number, expectedSize: number, filePath: string,
-    threshold: number): SizeDifference[] {
+    threshold: Threshold): SizeDifference[] {
   const diffPercentage = getDifferencePercentage(actualSize, expectedSize);
-  if (diffPercentage > threshold) {
+  const byteDiff = Math.abs(expectedSize - actualSize);
-    return [{
+  const diffs: SizeDifference[] = [];
+  if (diffPercentage > threshold.maxPercentageDiff) {
+    diffs.push({
       filePath: filePath,
       message: `Differs by ${diffPercentage.toFixed(2)}% from the expected size ` +
           `(actual = ${actualSize}, expected = ${expectedSize})`
-    }];
+    });
   }
-  return [];
+  if (byteDiff > threshold.maxByteDiff) {
+    diffs.push({
+      filePath: filePath,
+      message: `Differs by ${byteDiff}B from the expected size ` +
+          `(actual = ${actualSize}, expected = ${expectedSize})`
+    });
+  }
+  return diffs;
 }
 
 /**
@@ -63,7 +84,7 @@ function compareActualSizeToExpected(
  */
 function compareDirectorySizeEntry(
     actual: DirectorySizeEntry, expected: DirectorySizeEntry, filePath: string,
-    threshold: number): SizeDifference[] {
+    threshold: Threshold): SizeDifference[] {
   const diffs: SizeDifference[] =
       [...compareActualSizeToExpected(actual.size, expected.size, filePath, threshold)];
 

tools/size-tracking/file_size_compare_spec.ts

@@ -10,7 +10,7 @@ import {compareFileSizeData} from './file_size_compare';
 
 describe('file size compare', () => {
 
-  it('should report if size entry differ by more than the specified threshold', () => {
+  it('should report if size entry differ by more than the specified max percentage diff', () => {
     const diffs = compareFileSizeData(
         {
           unmapped: 0,
@@ -26,7 +26,7 @@ describe('file size compare', () => {
             'a.ts': 75,
           }
         },
-        0);
+        {maxPercentageDiff: 0, maxByteDiff: 25});
 
     expect(diffs.length).toBe(2);
     expect(diffs[0].filePath).toBe('/');
@@ -35,6 +35,41 @@ describe('file size compare', () => {
     expect(diffs[1].message).toMatch(/40.00% from the expected size/);
   });
 
+  it('should report if size entry differ by more than the specified max byte diff', () => {
+    const diffs = compareFileSizeData(
+        {
+          unmapped: 0,
+          files: {
+            size: 1000,
+            'a.ts': 1000,
+          }
+        },
+        {
+          unmapped: 0,
+          files: {
+            size: 1055,
+            'a.ts': 1055,
+          }
+        },
+        {maxPercentageDiff: 6, maxByteDiff: 50});
+
+    expect(diffs.length).toBe(2);
+    expect(diffs[0].filePath).toBe('/');
+    expect(diffs[0].message).toMatch(/55B from the expected size/);
+    expect(diffs[1].filePath).toBe('/a.ts');
+    expect(diffs[1].message).toMatch(/55B from the expected size/);
+  });
+
+  it('should report if unmapped bytes differ by more than specified threshold', () => {
+    const diffs = compareFileSizeData(
+        {unmapped: 1000, files: {size: 0}}, {unmapped: 1055, files: {size: 0}},
+        {maxPercentageDiff: 6, maxByteDiff: 50});
+
+    expect(diffs.length).toBe(1);
+    expect(diffs[0].filePath).toBe('<unmapped>');
+    expect(diffs[0].message).toMatch(/55B from the expected size/);
+  });
+
   it('should not report if size percentage difference does not exceed threshold', () => {
     const diffs = compareFileSizeData(
         {
@@ -51,7 +86,7 @@ describe('file size compare', () => {
             'a.ts': 75,
           }
         },
-        40);
+        {maxPercentageDiff: 40, maxByteDiff: 25});
 
     expect(diffs.length).toBe(0);
   });
@@ -67,7 +102,7 @@ describe('file size compare', () => {
             'b.ts': 1,
           }
         },
-        {unmapped: 0, files: {size: 100, 'a.ts': 100}}, 1);
+        {unmapped: 0, files: {size: 100, 'a.ts': 100}}, {maxByteDiff: 10, maxPercentageDiff: 1});
 
     expect(diffs.length).toBe(1);
     expect(diffs[0].filePath).toBe('/b.ts');
@@ -83,7 +118,8 @@ describe('file size compare', () => {
             'a.ts': 100,
           }
         },
-        {unmapped: 0, files: {size: 101, 'a.ts': 100, 'b.ts': 1}}, 1);
+        {unmapped: 0, files: {size: 101, 'a.ts': 100, 'b.ts': 1}},
+        {maxByteDiff: 10, maxPercentageDiff: 1});
 
     expect(diffs.length).toBe(1);
     expect(diffs[0].filePath).toBe('/b.ts');

tools/size-tracking/index.bzl

@@ -11,7 +11,15 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "nodejs_test")
   file size data against previously approved file size data
 """
 
-def js_size_tracking_test(name, src, sourceMap, goldenFile, diffThreshold, data = [], **kwargs):
+def js_size_tracking_test(
+        name,
+        src,
+        sourceMap,
+        goldenFile,
+        maxPercentageDiff,
+        maxByteDiff,
+        data = [],
+        **kwargs):
     all_data = data + [
         "//tools/size-tracking",
         "@npm//source-map",
@@ -24,7 +32,7 @@ def js_size_tracking_test(name, src, sourceMap, goldenFile, diffThreshold, data
         data = all_data,
         entry_point = entry_point,
         configuration_env_vars = ["compile"],
-        templated_args = [src, sourceMap, goldenFile, "%d" % diffThreshold, "false"],
+        templated_args = [src, sourceMap, goldenFile, "%d" % maxPercentageDiff, "%d" % maxByteDiff, "false"],
         **kwargs
     )
 
@@ -34,6 +42,6 @@ def js_size_tracking_test(name, src, sourceMap, goldenFile, diffThreshold, data
         data = all_data,
         entry_point = entry_point,
         configuration_env_vars = ["compile"],
-        templated_args = [src, sourceMap, goldenFile, "%d" % diffThreshold, "true"],
+        templated_args = [src, sourceMap, goldenFile, "0", "0", "true"],
         **kwargs
     )

tools/size-tracking/index.ts

@@ -13,17 +13,19 @@ import {compareFileSizeData} from './file_size_compare';
 import {FileSizeData} from './file_size_data';
 
 if (require.main === module) {
-  const [filePath, sourceMapPath, goldenPath, thresholdArg, writeGoldenArg] = process.argv.slice(2);
+  const
+      [filePath, sourceMapPath, goldenPath, maxPercentageDiffArg, maxSizeDiffArg, writeGoldenArg] =
+          process.argv.slice(2);
   const status = main(
       require.resolve(filePath), require.resolve(sourceMapPath), require.resolve(goldenPath),
-      writeGoldenArg === 'true', parseInt(thresholdArg));
+      writeGoldenArg === 'true', parseInt(maxPercentageDiffArg), parseInt(maxSizeDiffArg));
 
   process.exit(status ? 0 : 1);
 }
 
 export function main(
     filePath: string, sourceMapPath: string, goldenSizeMapPath: string, writeGolden: boolean,
-    diffThreshold: number): boolean {
+    maxPercentageDiff: number, maxByteDiff: number): boolean {
   const {sizeResult} = new SizeTracker(filePath, sourceMapPath);
 
   if (writeGolden) {
@@ -33,7 +35,8 @@ export function main(
   }
 
   const expectedSizeData = <FileSizeData>JSON.parse(readFileSync(goldenSizeMapPath, 'utf8'));
-  const differences = compareFileSizeData(sizeResult, expectedSizeData, diffThreshold);
+  const differences =
+      compareFileSizeData(sizeResult, expectedSizeData, {maxByteDiff, maxPercentageDiff});
 
   if (!differences.length) {
     return true;