Repository: paularmstrong/build-tracker
Pull request #145 (2020-02-24 00:02:46)

[formatting] reduce zeroes in %-value formatting
<!--
Thank you so much for contributing to open source and the Build Tracker project!
-->

# Problem

Reading percent deltas like `10.000%` is silly

# Solution

Since this is for visual inspection and actual values tend to be more important if you want to run real reports, simply do a ceiling at 1 decimal place.

# TODO

- [x] 🤓 Add & update tests (always try to _increase_ test coverage)
- [x] 🔬 Ensure CI is passing (`yarn lint:ci`, `yarn test`, `yarn tsc:ci`)
- [ ] 📖 Update relevant documentation



Patch:
diff --git a/config/fixtures-large.js b/config/fixtures-large.js
index be4a1c2..21ee9ec 100644
--- a/config/fixtures-large.js
+++ b/config/fixtures-large.js
@@ -3,16 +3,10 @@ const { BudgetLevel, BudgetType } = require('@build-tracker/types');
 
 module.exports = makeFixtureConfig('large', {
   artifacts: {
-    groups: [
-      {
-        artifactMatch: /^web\//,
-        name: 'Web'
-      },
-      {
-        artifactMatch: /^serviceworker\//,
-        name: 'Service Worker'
-      }
-    ],
+    budgets: {
+      '*': [{ level: BudgetLevel.WARN, sizeKey: 'gzip', type: BudgetType.PERCENT_DELTA, maximum: 0.5 }]
+    },
+    groups: [{ artifactMatch: /^web\//, name: 'Web' }, { artifactMatch: /^serviceworker\//, name: 'Service Worker' }],
     filters: [
       // Ignore all but english
       /web\/i18n-\w+\/(?!en\.).+/,
@@ -21,10 +15,7 @@ module.exports = makeFixtureConfig('large', {
       /web\/ondemand\.countries-(?!en\.).+/
     ]
   },
-  budgets: [
-    { level: BudgetLevel.WARN, sizeKey: 'gzip', type: BudgetType.PERCENT_DELTA, maximum: 0.5 },
-    { level: BudgetLevel.WARN, sizeKey: 'gzip', type: BudgetType.DELTA, maximum: 1000 }
-  ],
+  budgets: [{ level: BudgetLevel.WARN, sizeKey: 'gzip', type: BudgetType.DELTA, maximum: 1000 }],
   defaultSizeKey: 'gzip',
   name: 'Static Fixtures - Large Set'
 });
diff --git a/docs/docs/packages/server.md b/docs/docs/packages/server.md
index 25f1fb0..194fec7 100644
--- a/docs/docs/packages/server.md
+++ b/docs/docs/packages/server.md
@@ -64,6 +64,20 @@ module.exports = {
 };
 ```
 
+Artifact budgets can also be given a special key, `'*'`, that will apply to all artifacts in every build and be merged with any specific budgets:
+
+```js
+module.exports = {
+  artifacts: {
+    budgets: {
+      main: [{ level: 'error', sizeKey: 'gzip', type: 'size', maximum: 150000 }],
+      // Warn on any artifact that grows more than 50%
+      '*': [{ level: 'warn', sizeKey: 'gzip', type: 'percent_delta', maximum: 0.5 }]
+    }
+  }
+};
+```
+
 ##### `artifacts.filters?: Array<RegExp>`
 
 An array of regular expressions used to hide artifacts from display. This is typically not needed, unless you have artifacts generated by systems that really don't need to be checked.
diff --git a/src/comparator/src/BuildDelta.ts b/src/comparator/src/BuildDelta.ts
index 6c0987d..969b346 100644
--- a/src/comparator/src/BuildDelta.ts
+++ b/src/comparator/src/BuildDelta.ts
@@ -107,10 +107,11 @@ export default class BuildDelta<M extends BuildMeta = BuildMeta> {
 
         const sizes = baseArtifact ? baseArtifact.sizes : this._fauxArtifactSizes;
         const prevSizes = prevArtifact ? prevArtifact.sizes : this._fauxArtifactSizes;
+        const budgets = [...(this._artifactBudgets[artifactName] || []), ...(this._artifactBudgets['*'] || [])];
 
         const delta = new ArtifactDelta(
           artifactName,
-          this._artifactBudgets[artifactName] || [],
+          budgets,
           sizes,
           prevSizes,
           !baseArtifact || !prevArtifact || baseArtifact.hash !== prevArtifact.hash
diff --git a/src/formatting/src/index.ts b/src/formatting/src/index.ts
index 637b760..fd231b8 100644
--- a/src/formatting/src/index.ts
+++ b/src/formatting/src/index.ts
@@ -29,7 +29,8 @@ export function formatSha(sha: string): string {
 }
 
 function formatPercent(value: number): string {
-  return `${(value * 100).toFixed(3)}%`;
+  const val = Math.ceil(value * 1000) / 10;
+  return `${val}%`;
 }
 
 const levelToString = {
@@ -55,7 +56,7 @@ export function formatBudgetResult(budgetResult: BudgetResult, itemName: string,
     case BudgetType.DELTA:
       return `${prefix} failed the ${sizeKey} budget delta limit. Expected to increase no more than ${expectedFormatted}, but increased by ${actualFormatted}`;
     case BudgetType.PERCENT_DELTA:
-      return `${prefix} failed the ${sizeKey} budget percent change limit. Expected no increase by no more than ${expectedFormatted}, but increased by ${actualFormatted}`;
+      return `${prefix} failed the ${sizeKey} budget percent change limit. Expected to not increase by more than ${expectedFormatted}, but increased by ${actualFormatted}`;
     case BudgetType.SIZE:
       return `${prefix} failed the ${sizeKey} budget size limit of ${expectedFormatted} by ${diffFormatted}`;
   }


Test patch:
diff --git a/src/comparator/src/__tests__/BuildDelta.test.ts b/src/comparator/src/__tests__/BuildDelta.test.ts
index 51bc44b..6566067 100644
--- a/src/comparator/src/__tests__/BuildDelta.test.ts
+++ b/src/comparator/src/__tests__/BuildDelta.test.ts
@@ -4,6 +4,7 @@
 import ArtifactDelta from '../ArtifactDelta';
 import Build from '@build-tracker/build';
 import BuildDelta from '../BuildDelta';
+import { BudgetLevel, BudgetType } from '@build-tracker/types';
 
 describe('BuildDelta', () => {
   let buildA, buildB;
@@ -111,6 +112,33 @@ describe('BuildDelta', () => {
         new ArtifactDelta('tacos', [], { gzip: 1, stat: 2 }, { gzip: 1, stat: 1 }, false)
       ]);
     });
+
+    test('includes budgets for each artifact', () => {
+      const budgets = [{ level: BudgetLevel.WARN, sizeKey: 'gzip', type: BudgetType.DELTA, maximum: 1 }];
+      const bd = new BuildDelta(buildA, buildB, {
+        artifactBudgets: { tacos: budgets }
+      });
+      expect(bd.artifactDeltas).toEqual(
+        expect.arrayContaining([new ArtifactDelta('tacos', budgets, { gzip: 1, stat: 2 }, { gzip: 1, stat: 1 }, false)])
+      );
+    });
+
+    test('includes * budgets on every artifact', () => {
+      const budget = { level: BudgetLevel.WARN, sizeKey: 'gzip', type: BudgetType.DELTA, maximum: 1 };
+      const burritoBudget = { level: BudgetLevel.ERROR, sizeKey: 'stat', type: BudgetType.DELTA, maximum: 1 };
+      const bd = new BuildDelta(buildA, buildB, { artifactBudgets: { '*': [budget], burritos: [burritoBudget] } });
+      expect(bd.artifactDeltas).toEqual([
+        new ArtifactDelta('tacos', [budget], { gzip: 1, stat: 2 }, { gzip: 1, stat: 1 }, false),
+        new ArtifactDelta(
+          'burritos',
+          expect.arrayContaining([budget, burritoBudget]),
+          { gzip: 2, stat: 3 },
+          { gzip: 4, stat: 6 },
+          true
+        ),
+        new ArtifactDelta('churros', [budget], { gzip: 0, stat: 0 }, { gzip: 4, stat: 6 }, true)
+      ]);
+    });
   });
 
   describe('getArtifactDelta', () => {
diff --git a/src/formatting/src/__tests__/index.test.ts b/src/formatting/src/__tests__/index.test.ts
index cd71103..3a85527 100644
--- a/src/formatting/src/__tests__/index.test.ts
+++ b/src/formatting/src/__tests__/index.test.ts
@@ -104,14 +104,14 @@ describe('formatBudgetResult', () => {
             sizeKey: 'stat',
             passing: false,
             expected: 0.1,
-            actual: 0.2,
+            actual: 0.22345,
             type: BudgetType.PERCENT_DELTA,
             level: BudgetLevel.ERROR
           },
           'tacos'
         )
       ).toEqual(
-        'Error: `tacos` failed the stat budget percent change limit. Expected no increase by no more than 10.000%, but increased by 20.000%'
+        'Error: `tacos` failed the stat budget percent change limit. Expected to not increase by more than 10%, but increased by 22.4%'
       );
     });
   });
@@ -175,14 +175,14 @@ describe('formatBudgetResult', () => {
             sizeKey: 'stat',
             passing: false,
             expected: 0.1,
-            actual: 0.2,
+            actual: 1.6354908725,
             type: BudgetType.PERCENT_DELTA,
             level: BudgetLevel.WARN
           },
           'tacos'
         )
       ).toEqual(
-        'Warning: `tacos` failed the stat budget percent change limit. Expected no increase by no more than 10.000%, but increased by 20.000%'
+        'Warning: `tacos` failed the stat budget percent change limit. Expected to not increase by more than 10%, but increased by 163.6%'
       );
     });
   });