refactor(aggregations): switch usage of aggregations to categories (#…

…1935)

* refactor(aggregations): switch usage of aggregations to categories
* github conflicts
* update perf config
* feedback

lighthouse-core/config/config.js

@@ -313,20 +313,20 @@ class Config {
     return merge(baseJSON, extendJSON);
   }
 
- /**
-  * Filter out any unrequested items from the config, based on requested top-level aggregations.
-  * @param {!Object} oldConfig Lighthouse config object
-  * @param {!Array<string>} aggregationIDs Id values of aggregations to include
-  * @return {Object} new config
-  */
-  static generateNewConfigOfAggregations(oldConfig, aggregationIDs) {
+  /**
+   * Filter out any unrequested items from the config, based on requested top-level categories.
+   * @param {!Object} oldConfig Lighthouse config object
+   * @param {!Array<string>} categoryIds ID values of categories to include
+   * @return {!Object} A new config
+   */
+  static generateNewConfigOfCategories(oldConfig, categoryIds) {
     // 0. Clone config to avoid mutating it
     const config = JSON.parse(JSON.stringify(oldConfig));
-    // 1. Filter to just the chosen aggregations
-    config.aggregations = config.aggregations.filter(agg => aggregationIDs.includes(agg.id));
+    // 1. Filter to just the chosen categories
+    config.categories = Config.filterCategories(config.categories, categoryIds);
 
     // 2. Resolve which audits will need to run
-    const requestedAuditNames = Config.getAuditsNeededByAggregations(config.aggregations);
+    const requestedAuditNames = Config.getAuditIdsInCategories(config.categories);
     const auditPathToNameMap = Config.getMapOfAuditPathToName(config);
     config.audits = config.audits.filter(auditPath =>
         requestedAuditNames.has(auditPathToNameMap.get(auditPath)));
@@ -340,27 +340,43 @@ class Config {
     return config;
   }
 
- /**
-  * Return IDs of top-level aggregations from a config. Used by tools driving lighthouse-core
-  * @param {{aggregations: !Array<{name: string}>}} Lighthouse config object.
-  * @return {!Array<string>}  Name values of aggregations within
-  */
-  static getAggregations(config) {
-    return config.aggregations.map(agg => ({
-      name: agg.name,
-      id: agg.id
-    }));
+  /**
+   * Filter out any unrequested categories from the categories object.
+   * @param {!Object<string, {audits: !Array<{id: string}>}>} categories
+   * @param {Array<string>=} categoryIds
+   * @return {!Object<string, {audits: !Array<{id: string}>}>}
+   */
+  static filterCategories(oldCategories, categoryIds = []) {
+    const categories = {};
+
+    Object.keys(oldCategories).forEach(categoryId => {
+      if (categoryIds.includes(categoryId)) {
+        categories[categoryId] = oldCategories[categoryId];
+      }
+    });
+
+    return categories;
   }
 
   /**
-   * Find audits required for remaining aggregations.
-   * @param {!Object} aggregations
+   * Finds the unique set of audit IDs used by the categories object.
+   * @param {!Object<string, {audits: !Array<{id: string}>}>} categories
    * @return {!Set<string>}
    */
-  static getAuditsNeededByAggregations(aggregations) {
-    const requestedItems = _flatten(aggregations.map(aggregation => aggregation.items));
-    const requestedAudits = _flatten(requestedItems.map(item => Object.keys(item.audits)));
-    return new Set(requestedAudits);
+  static getAuditIdsInCategories(categories) {
+    const audits = _flatten(Object.keys(categories).map(id => categories[id].audits));
+    return new Set(audits.map(audit => audit.id));
+  }
+
+ /**
+  * @param {{categories: !Object<string, {name: string}>}} config
+  * @return {!Array<{id: string, name: string}>}
+  */
+  static getCategories(config) {
+    return Object.keys(config.categories).map(id => {
+      const name = config.categories[id].name;
+      return {id, name};
+    });
   }
 
   /**

lighthouse-core/config/perf.json

@@ -28,6 +28,26 @@
     "dobetterweb/script-blocking-first-paint"
   ],
 
+  "categories": {
+    "performance": {
+      "name": "Performance",
+      "description": "These encapsulate your app's performance.",
+      "audits": [
+        {"id": "first-meaningful-paint", "weight": 5},
+        {"id": "speed-index-metric", "weight": 1},
+        {"id": "estimated-input-latency", "weight": 1},
+        {"id": "time-to-interactive", "weight": 5},
+        {"id": "link-blocking-first-paint", "weight": 0},
+        {"id": "script-blocking-first-paint", "weight": 0},
+        {"id": "uses-optimized-images", "weight": 0},
+        {"id": "uses-responsive-images", "weight": 0},
+        {"id": "dom-size", "weight": 0},
+        {"id": "critical-request-chains", "weight": 0},
+        {"id": "user-timings", "weight": 0}
+      ]
+    }
+  },
+
   "aggregations": [{
     "name": "Performance metrics",
     "description": "",

lighthouse-core/report/v2/report-generator.js

@@ -61,6 +61,31 @@ class ReportGeneratorV2 {
         .join(firstReplacement.replacement);
   }
 
+  /**
+   * Convert categories into old-school aggregations for old HTML report compat.
+   * @param {!Array<{name: string, description: string, id: string, score: number,
+   *    audits: !Array<{result: Object}>}>} categories
+   * @return {!Array<!Aggregation>}
+   */
+  static _getAggregations(reportCategories) {
+    return reportCategories.map(category => {
+      const name = category.name;
+      const description = category.description;
+
+      return {
+        name, description,
+        categorizable: false,
+        scored: category.id === 'pwa',
+        total: category.score / 100,
+        score: [{
+          name, description,
+          overall: category.score / 100,
+          subItems: category.audits.map(audit => audit.result),
+        }],
+      };
+    });
+  }
+
   /**
    * Returns the report JSON object with computed scores.
    * @param {{categories: !Object<{audits: !Array}>}} config
@@ -70,6 +95,7 @@ class ReportGeneratorV2 {
   generateReportJson(config, resultsByAuditId) {
     const categories = Object.keys(config.categories).map(categoryId => {
       const category = config.categories[categoryId];
+      category.id = categoryId;
 
       const audits = category.audits.map(audit => {
         const result = resultsByAuditId[audit.id];
@@ -85,8 +111,11 @@ class ReportGeneratorV2 {
       const categoryScore = ReportGeneratorV2.arithmeticMean(audits);
       return Object.assign({}, category, {audits, score: categoryScore});
     });
+
     const overallScore = ReportGeneratorV2.arithmeticMean(categories);
-    return {score: overallScore, categories};
+    // TODO: remove aggregations when old report is fully replaced
+    const aggregations = ReportGeneratorV2._getAggregations(categories);
+    return {score: overallScore, categories, aggregations};
   }
 
   /**

lighthouse-core/runner.js

@@ -18,7 +18,6 @@
 
 const Driver = require('./gather/driver.js');
 const GatherRunner = require('./gather/gather-runner');
-const Aggregate = require('./aggregator/aggregate');
 const ReportGeneratorV2 = require('./report/v2/report-generator');
 const Audit = require('./audits/audit');
 const emulation = require('./lib/emulation');
@@ -128,19 +127,14 @@ class Runner {
           return results;
         }, {});
 
-        // Only run aggregations if needed.
         let aggregations = [];
-        if (config.aggregations) {
-          aggregations = config.aggregations.map(
-            a => Aggregate.aggregate(a, runResults.auditResults));
-        }
-
         let reportCategories = [];
         let score = 0;
         if (config.categories) {
           const reportGenerator = new ReportGeneratorV2();
           const report = reportGenerator.generateReportJson(config, resultsById);
           reportCategories = report.categories;
+          aggregations = report.aggregations;
           score = report.score;
         }
 

lighthouse-core/test/config/config-test.js

@@ -26,6 +26,11 @@ const Audit = require('../../audits/audit');
 /* eslint-env mocha */
 
 describe('Config', () => {
+  let origConfig;
+  beforeEach(() => {
+    origConfig = JSON.parse(JSON.stringify(defaultConfig));
+  });
+
   it('returns new object', () => {
     const config = {
       audits: ['is-on-https']
@@ -60,8 +65,8 @@ describe('Config', () => {
 
   it('uses the default config when no config is provided', () => {
     const config = new Config();
-    assert.deepStrictEqual(defaultConfig.aggregations, config.aggregations);
-    assert.equal(defaultConfig.audits.length, config.audits.length);
+    assert.deepStrictEqual(origConfig.aggregations, config.aggregations);
+    assert.equal(origConfig.audits.length, config.audits.length);
   });
 
   it('warns when a passName is used twice', () => {
@@ -149,7 +154,7 @@ describe('Config', () => {
   });
 
   it('contains new copies of auditResults and aggregations', () => {
-    const configJSON = defaultConfig;
+    const configJSON = origConfig;
     configJSON.auditResults = [{
       value: 1,
       rawValue: 1.0,
@@ -365,53 +370,51 @@ describe('Config', () => {
     });
   });
 
-  describe('getAggregations', () => {
+  describe('getCategories', () => {
     it('returns the IDs & names of the aggregations', () => {
-      const runConfig = JSON.parse(JSON.stringify(defaultConfig));
-      const aggs = Config.getAggregations(runConfig);
-      assert.equal(Array.isArray(aggs), true);
-      assert.equal(aggs.length, 5, 'Found the correct number of aggregations');
-      const haveName = aggs.every(agg => agg.name.length);
-      const haveID = aggs.every(agg => agg.id.length);
-      assert.equal(haveName === haveID === true, true, 'they dont have IDs and names');
+      const categories = Config.getCategories(origConfig);
+      assert.equal(Array.isArray(categories), true);
+      assert.equal(categories.length, 4, 'Found the correct number of categories');
+      const haveName = categories.every(agg => agg.name.length);
+      const haveID = categories.every(agg => agg.id.length);
+      assert.equal(haveName === haveID === true, true, 'they have IDs and names');
     });
   });
 
-  describe('generateConfigOfAggregations', () => {
-    const aggregationIDs = ['perf'];
-    const totalAuditCount = defaultConfig.audits.length;
-
+  describe('generateConfigOfCategories', () => {
     it('should not mutate the original config', () => {
-      const origConfig = JSON.parse(JSON.stringify(defaultConfig));
-      Config.generateNewConfigOfAggregations(defaultConfig, aggregationIDs);
-      assert.deepStrictEqual(origConfig, defaultConfig, 'Original config mutated');
+      const configCopy = JSON.parse(JSON.stringify(origConfig));
+      Config.generateNewConfigOfCategories(configCopy, ['performance']);
+      assert.deepStrictEqual(configCopy, origConfig, 'no mutations');
     });
 
     it('should filter out other passes if passed Performance', () => {
-      const config = Config.generateNewConfigOfAggregations(defaultConfig, aggregationIDs);
-      assert.equal(config.aggregations.length, 1, 'other aggregations are present');
+      const totalAuditCount = origConfig.audits.length;
+      const config = Config.generateNewConfigOfCategories(origConfig, ['performance']);
+      assert.equal(Object.keys(config.categories).length, 1, 'other categories are present');
       assert.equal(config.passes.length, 2, 'incorrect # of passes');
       assert.ok(config.audits.length < totalAuditCount, 'audit filtering probably failed');
     });
 
     it('should filter out other passes if passed PWA', () => {
-      const config = Config.generateNewConfigOfAggregations(defaultConfig, ['pwa']);
-      assert.equal(config.aggregations.length, 1, 'other aggregations are present');
+      const totalAuditCount = origConfig.audits.length;
+      const config = Config.generateNewConfigOfCategories(origConfig, ['pwa']);
+      assert.equal(Object.keys(config.categories).length, 1, 'other categories are present');
       assert.ok(config.audits.length < totalAuditCount, 'audit filtering probably failed');
     });
 
     it('should filter out other passes if passed Best Practices', () => {
-      const config = Config.generateNewConfigOfAggregations(defaultConfig, ['bp']);
-      assert.equal(config.aggregations.length, 1, 'other aggregations are present');
+      const totalAuditCount = origConfig.audits.length;
+      const config = Config.generateNewConfigOfCategories(origConfig, ['best-practices']);
+      assert.equal(Object.keys(config.categories).length, 1, 'other categories are present');
       assert.equal(config.passes.length, 2, 'incorrect # of passes');
       assert.ok(config.audits.length < totalAuditCount, 'audit filtering probably failed');
     });
 
-    it('should only run audits for ones named by the aggregation', () => {
-      const config = Config.generateNewConfigOfAggregations(defaultConfig, aggregationIDs);
-      const selectedAggregation = defaultConfig.aggregations
-          .find(agg => aggregationIDs.includes(agg.id));
-      const auditCount = Object.keys(selectedAggregation.items[0].audits).length;
+    it('should only run audits for ones named by the category', () => {
+      const config = Config.generateNewConfigOfCategories(origConfig, ['performance']);
+      const selectedCategory = origConfig.categories.performance;
+      const auditCount = Object.keys(selectedCategory.audits).length;
 
       assert.equal(config.audits.length, auditCount, '# of audits match aggregation list');
     });

lighthouse-core/test/runner-test.js

@@ -322,6 +322,38 @@ describe('Runner', () => {
     });
   });
 
+  it('returns reportCategories', () => {
+    const url = 'https://example.com';
+    const config = new Config({
+      auditResults: [{
+        name: 'content-width',
+        rawValue: true,
+        score: true,
+        displayValue: 'display'
+      }],
+      categories: {
+        category: {
+          name: 'Category',
+          description: '',
+          audits: [
+            {id: 'content-width', weight: 1}
+          ]
+        }
+      }
+    });
+
+    return Runner.run(null, {url, config, driverMock}).then(results => {
+      assert.ok(results.lighthouseVersion);
+      assert.ok(results.generatedTime);
+      assert.equal(results.initialUrl, url);
+      assert.equal(results.audits['content-width'].name, 'content-width');
+      assert.equal(results.reportCategories[0].score, 100);
+      assert.equal(results.reportCategories[0].audits[0].id, 'content-width');
+      assert.equal(results.reportCategories[0].audits[0].score, 100);
+      assert.equal(results.reportCategories[0].audits[0].result.displayValue, 'display');
+    });
+  });
+
   it('returns an aggregation', () => {
     const url = 'https://example.com';
     const config = new Config({
@@ -331,23 +363,15 @@ describe('Runner', () => {
         score: true,
         displayValue: ''
       }],
-
-      aggregations: [{
-        name: 'Aggregation',
-        description: '',
-        scored: true,
-        categorizable: true,
-        items: [{
-          name: 'name',
-          description: 'description',
-          audits: {
-            'content-width': {
-              expectedValue: true,
-              weight: 1
-            }
-          }
-        }]
-      }]
+      categories: {
+        category: {
+          name: 'Category',
+          description: '',
+          audits: [
+            {id: 'content-width', weight: 1}
+          ]
+        }
+      }
     });
 
     return Runner.run(null, {url, config, driverMock}).then(results => {
@@ -356,7 +380,7 @@ describe('Runner', () => {
       assert.equal(results.initialUrl, url);
       assert.equal(results.audits['content-width'].name, 'content-width');
       assert.equal(results.aggregations[0].score[0].overall, 1);
-      assert.equal(results.aggregations[0].score[0].subItems[0], 'content-width');
+      assert.equal(results.aggregations[0].score[0].subItems[0].name, 'content-width');
     });
   });