diff --git a/packages/core/test/bundling/core_all/BUILD.bazel b/packages/core/test/bundling/core_all/BUILD.bazel index 13b0f07a64..0fd8a712e1 100644 --- a/packages/core/test/bundling/core_all/BUILD.bazel +++ b/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", diff --git a/tools/size-tracking/file_size_compare.ts b/tools/size-tracking/file_size_compare.ts index 6ff91be7f6..83f42170c2 100644 --- a/tools/size-tracking/file_size_compare.ts +++ b/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) { - const diffs: SizeDifference[] = compareSizeEntry(actual.files, expected.files, '/', threshold); - const unmappedBytesDiff = getDifferencePercentage(actual.unmapped, expected.unmapped); - if (unmappedBytesDiff > threshold) { - diffs.push({ - message: `Unmapped bytes differ by ${unmappedBytesDiff.toFixed(2)}% from ` + - `the expected size (actual = ${actual.unmapped}, expected = ${expected.unmapped})` - }); - } - return diffs; + actual: FileSizeData, expected: FileSizeData, threshold: Threshold) { + return [ + ...compareSizeEntry(actual.files, expected.files, '/', threshold), + ...compareActualSizeToExpected(actual.unmapped, expected.unmapped, '', threshold) + ]; } /** 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( actual, expected, filePath, threshold); @@ -40,21 +51,31 @@ function compareSizeEntry( } /** - * Compares two size numbers and returns a size difference when the percentage difference - * exceeds the specified threshold. + * Compares two size numbers and returns a size difference if the difference + * 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) { - return [{ + const byteDiff = Math.abs(expectedSize - actualSize); + 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)]; diff --git a/tools/size-tracking/file_size_compare_spec.ts b/tools/size-tracking/file_size_compare_spec.ts index ac8045bb93..90282b66e3 100644 --- a/tools/size-tracking/file_size_compare_spec.ts +++ b/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(''); + 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'); diff --git a/tools/size-tracking/index.bzl b/tools/size-tracking/index.bzl index c2263f86f8..280799a953 100644 --- a/tools/size-tracking/index.bzl +++ b/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 ) diff --git a/tools/size-tracking/index.ts b/tools/size-tracking/index.ts index 07035ffaed..9857518ee5 100644 --- a/tools/size-tracking/index.ts +++ b/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 = JSON.parse(readFileSync(goldenSizeMapPath, 'utf8')); - const differences = compareFileSizeData(sizeResult, expectedSizeData, diffThreshold); + const differences = + compareFileSizeData(sizeResult, expectedSizeData, {maxByteDiff, maxPercentageDiff}); if (!differences.length) { return true;