core(lhr): make reportCategories shallow; move audit scores to AuditR…

…esult (#4711)

* `reportCategories` no longer contains real auditResults
  * Each audit's score moves from `reportCategories.performance['audit-name']` (which now just holds `weight` and `group`) over to `audits['audit-name']`
* Remove `LHR.score`, the overallScore

lighthouse-core/config/default.js

@@ -297,7 +297,6 @@ module.exports = {
     },
     'pwa': {
       name: 'Progressive Web App',
-      weight: 1,
       description: 'These checks validate the aspects of a Progressive Web App, as specified by the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).',
       audits: [
         {id: 'service-worker', weight: 1},

lighthouse-core/report/v2/renderer/category-renderer.js

@@ -57,7 +57,7 @@ class CategoryRenderer {
       scoreEl.classList.add('lh-score--manual');
     }
 
-    return this._populateScore(tmpl, audit.score, scoringMode, title, description);
+    return this._populateScore(tmpl, audit.result.score, scoringMode, title, description);
   }
 
   /**
@@ -294,7 +294,7 @@ class CategoryRenderer {
 
       if (audit.result.notApplicable) {
         group.notApplicable.push(audit);
-      } else if (audit.score === 100 && !audit.result.debugString) {
+      } else if (audit.result.score === 100 && !audit.result.debugString) {
         group.passed.push(audit);
       } else {
         group.failed.push(audit);

lighthouse-core/report/v2/renderer/performance-category-renderer.js

@@ -16,7 +16,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
   _renderTimelineMetricAudit(audit, scale) {
     const tmpl = this.dom.cloneTemplate('#tmpl-lh-timeline-metric', this.templateContext);
     const element = this.dom.find('.lh-timeline-metric', tmpl);
-    element.classList.add(`lh-timeline-metric--${Util.calculateRating(audit.score)}`);
+    element.classList.add(`lh-timeline-metric--${Util.calculateRating(audit.result.score)}`);
 
     const titleEl = this.dom.find('.lh-timeline-metric__title', tmpl);
     titleEl.textContent = audit.result.description;
@@ -49,7 +49,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
 
     const element = this.dom.createElement('details', [
       'lh-perf-hint',
-      `lh-perf-hint--${Util.calculateRating(audit.score)}`,
+      `lh-perf-hint--${Util.calculateRating(audit.result.score)}`,
       'lh-expandable-details',
     ].join(' '));
 
@@ -152,7 +152,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
     element.appendChild(metricAuditsEl);
 
     const hintAudits = category.audits
-        .filter(audit => audit.group === 'perf-hint' && audit.score < 100)
+        .filter(audit => audit.group === 'perf-hint' && audit.result.score < 100)
         .sort((auditA, auditB) => auditB.result.rawValue - auditA.result.rawValue);
     if (hintAudits.length) {
       const maxWaste = Math.max(...hintAudits.map(audit => audit.result.rawValue));
@@ -164,7 +164,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
     }
 
     const infoAudits = category.audits
-        .filter(audit => audit.group === 'perf-info' && audit.score < 100);
+        .filter(audit => audit.group === 'perf-info' && audit.result.score < 100);
     if (infoAudits.length) {
       const infoAuditsEl = this.renderAuditGroup(groups['perf-info'], {expandable: false});
       infoAudits.forEach(item => infoAuditsEl.appendChild(this.renderAudit(item)));
@@ -174,7 +174,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
 
     const passedElements = category.audits
         .filter(audit => (audit.group === 'perf-hint' || audit.group === 'perf-info') &&
-            audit.score === 100)
+            audit.result.score === 100)
         .map(audit => this.renderAudit(audit));
 
     if (!passedElements.length) return element;

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

@@ -30,9 +30,14 @@ class ReportRenderer {
    * @param {!Element} container Parent element to render the report into.
    */
   renderReport(report, container) {
-    container.textContent = ''; // Remove previous report.
-    const element = container.appendChild(this._renderReport(report));
+    // If any mutations happen to the report within the renderers, we want the original object untouched
+    const clone = /** @type {!ReportRenderer.ReportJSON} */ (JSON.parse(JSON.stringify(report)));
+
+    if (!Array.isArray(clone.reportCategories)) throw new Error('No reportCategories provided.');
+    ReportRenderer.smooshAuditResultsIntoCategories(clone.audits, clone.reportCategories);
 
+    container.textContent = ''; // Remove previous report.
+    const element = container.appendChild(this._renderReport(clone));
     return /** @type {!Element} **/ (element);
   }
 
@@ -155,24 +160,37 @@ class ReportRenderer {
     perfCategoryRenderer.setTemplateContext(this._templateContext);
 
     const categories = reportSection.appendChild(this._dom.createElement('div', 'lh-categories'));
+
     for (const category of report.reportCategories) {
       if (scoreHeader) {
         scoreHeader.appendChild(categoryRenderer.renderScoreGauge(category));
       }
 
       let renderer = categoryRenderer;
-
       if (category.id === 'performance') {
         renderer = perfCategoryRenderer;
       }
-
       categories.appendChild(renderer.render(category, report.reportGroups));
     }
 
     reportSection.appendChild(this._renderReportFooter(report));
 
     return container;
   }
+
+  /**
+   * Place the AuditResult into the auditDfn (which has just weight & group)
+   * @param {!Object<string, !ReportRenderer.AuditResultJSON>} audits
+   * @param {!Array<!ReportRenderer.CategoryJSON>} reportCategories
+   */
+  static smooshAuditResultsIntoCategories(audits, reportCategories) {
+    for (const category of reportCategories) {
+      category.audits.forEach(auditMeta => {
+        const result = audits[auditMeta.id];
+        auditMeta.result = result;
+      });
+    }
+  }
 }
 
 if (typeof module !== 'undefined' && module.exports) {
@@ -194,6 +212,7 @@ if (typeof module !== 'undefined' && module.exports) {
  *     scoringMode: string,
  *     extendedInfo: Object,
  *     error: boolean,
+ *     score: number,
  *     details: (!DetailsRenderer.DetailsJSON|undefined),
  * }}
  */
@@ -214,7 +233,6 @@ ReportRenderer.AuditJSON; // eslint-disable-line no-unused-expressions
  * @typedef {{
  *     name: string,
  *     id: string,
- *     weight: number,
  *     score: number,
  *     description: string,
  *     audits: !Array<!ReportRenderer.AuditJSON>

lighthouse-core/runner.js

@@ -93,9 +93,10 @@ class Runner {
       const resultsById = {};
       for (const audit of runResults.auditResults) resultsById[audit.name] = audit;
 
-      let report;
+      let categories;
       if (opts.config.categories) {
-        report = Runner._scoreAndCategorize(opts, resultsById);
+        Runner._scoreAndCategorize(opts, resultsById);
+        categories = Object.values(opts.config.categories);
       }
 
       return {
@@ -108,8 +109,7 @@ class Runner {
         audits: resultsById,
         artifacts: runResults.artifacts,
         runtimeConfig: Runner.getRuntimeConfig(opts.flags),
-        score: report ? report.score : 0,
-        reportCategories: report ? report.categories : [],
+        reportCategories: categories,
         reportGroups: opts.config.groups,
       };
     }).catch(err => {
@@ -192,7 +192,7 @@ class Runner {
    * @param {{}} resultsById
    */
   static _scoreAndCategorize(opts, resultsById) {
-    return ReportScoring.scoreAllCategories(opts.config, resultsById);
+    ReportScoring.scoreAllCategories(opts.config, resultsById);
   }
 
   /**

lighthouse-core/scoring.js

@@ -9,39 +9,53 @@
 class ReportScoring {
   /**
    * Computes the weighted-average of the score of the list of items.
-   * @param {!Array<{score: number|undefined, weight: number|undefined}>} items
+   * @param {!Array<{score: ?number|boolean|undefined, weight: number|undefined}>} items
    * @return {number}
    */
   static arithmeticMean(items) {
-    const results = items.reduce((result, item) => {
-      const score = Number(item.score) || 0;
-      const weight = Number(item.weight) || 0;
-      return {
-        weight: result.weight + weight,
-        sum: result.sum + score * weight,
-      };
-    }, {weight: 0, sum: 0});
+    const results = items.reduce(
+      (result, item) => {
+        // HACK. remove this in the next PR
+        // Srsly. The score inconsitency has been very bad.
+        let itemScore = item.score;
+        if (typeof item.score === 'boolean') {
+          itemScore = item.score ? 100 : 0;
+        }
+
+        const score = Number(itemScore) || 0;
+        const weight = Number(item.weight) || 0;
+        return {
+          weight: result.weight + weight,
+          sum: result.sum + score * weight,
+        };
+      },
+      {weight: 0, sum: 0}
+    );
 
-    return (results.sum / results.weight) || 0;
+    return results.sum / results.weight || 0;
   }
 
   /**
    * Returns the report JSON object with computed scores.
-   * @param {{categories: !Object<string, {id: string|undefined, weight: number|undefined, audits: !Array<{id: string, weight: number|undefined}>}>}} config
+   * @param {{categories: !Object<string, {id: string|undefined, weight: number|undefined, score: number|undefined, audits: !Array<{id: string, weight: number|undefined}>}>}} config
    * @param {!Object<string, {score: ?number|boolean|undefined, notApplicable: boolean, informative: boolean}>} resultsByAuditId
-   * @return {{score: number, categories: !Array<{audits: !Array<{score: number, result: !Object}>}>}}
    */
   static scoreAllCategories(config, resultsByAuditId) {
-    const categories = Object.keys(config.categories).map(categoryId => {
-      const category = config.categories[categoryId];
+    for (const [categoryId, category] of Object.entries(config.categories)) {
       category.id = categoryId;
-
-      const audits = category.audits.map(audit => {
+      category.audits.forEach(audit => {
         const result = resultsByAuditId[audit.id];
         // Cast to number to catch `null` and undefined when audits error
+        /** @type {number|boolean} */
         let auditScore = Number(result.score) || 0;
         if (typeof result.score === 'boolean') {
-          auditScore = result.score ? 100 : 0;
+          // HACK removed in the next PR
+          // While we'd like to do this boolean transformation happened on the auditDfn:
+          //     auditScore = result.score ? 100 : 0;
+          // …the original result.score is untouched which means the smokehouse expectations will fail
+          // We're officially rebaselining all those expectations in the next PR...
+          // So for now, we'll keep keep both auditDfn.score and result.score boolean
+          auditScore = result.score;
         }
         // If a result was not applicable, meaning its checks did not run against anything on
         // the page, force it's weight to 0. It will not count during the arithmeticMean() but
@@ -53,15 +67,17 @@ class ReportScoring {
           result.informative = true;
         }
 
-        return Object.assign({}, audit, {result, score: auditScore});
+        result.score = auditScore;
       });
 
-      const categoryScore = ReportScoring.arithmeticMean(audits);
-      return Object.assign({}, category, {audits, score: categoryScore});
-    });
-
-    const overallScore = ReportScoring.arithmeticMean(categories);
-    return {score: overallScore, categories};
+      const scores = category.audits.map(audit => ({
+        score: resultsByAuditId[audit.id].score,
+        weight: audit.weight,
+      }));
+      const categoryScore = ReportScoring.arithmeticMean(scores);
+      // mutate config.categories[].score
+      category.score = categoryScore;
+    }
   }
 }
 

lighthouse-core/test/report/v2/renderer/category-renderer-test.js

@@ -17,6 +17,7 @@ const DetailsRenderer = require('../../../../report/v2/renderer/details-renderer
 const CriticalRequestChainRenderer = require(
     '../../../../report/v2/renderer/crc-details-renderer.js');
 const CategoryRenderer = require('../../../../report/v2/renderer/category-renderer.js');
+const ReportRenderer = require('../../../../report/v2/renderer/report-renderer.js');
 const sampleResults = require('../../../results/sample_v2.json');
 
 const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../../report/v2/templates.html', 'utf8');
@@ -33,6 +34,9 @@ describe('CategoryRenderer', () => {
     const dom = new DOM(document);
     const detailsRenderer = new DetailsRenderer(dom);
     renderer = new CategoryRenderer(dom, detailsRenderer);
+
+    ReportRenderer.smooshAuditResultsIntoCategories(sampleResults.audits,
+      sampleResults.reportCategories);
   });
 
   after(() => {
@@ -178,7 +182,7 @@ describe('CategoryRenderer', () => {
     it.skip('renders the failed audits grouped by group', () => {
       const categoryDOM = renderer.render(category, sampleResults.reportGroups);
       const failedAudits = category.audits.filter(audit => {
-        return audit.score !== 100 && !audit.result.notApplicable;
+        return audit.result.score !== 100 && !audit.result.notApplicable;
       });
       const failedAuditTags = new Set(failedAudits.map(audit => audit.group));
 
@@ -188,10 +192,10 @@ describe('CategoryRenderer', () => {
 
     it('renders the passed audits grouped by group', () => {
       const categoryDOM = renderer.render(category, sampleResults.reportGroups);
-
       const passedAudits = category.audits.filter(audit =>
-        !audit.result.notApplicable && audit.score === 100);
+          !audit.result.notApplicable && audit.result.score === 100);
       const passedAuditTags = new Set(passedAudits.map(audit => audit.group));
+
       const passedAuditGroups = categoryDOM.querySelectorAll('.lh-passed-audits .lh-audit-group');
       assert.equal(passedAuditGroups.length, passedAuditTags.size);
     });
@@ -219,7 +223,7 @@ describe('CategoryRenderer', () => {
     it('doesnt create a passed section if there were 0 passed', () => {
       const origCategory = sampleResults.reportCategories.find(c => c.id === 'pwa');
       const category = JSON.parse(JSON.stringify(origCategory));
-      category.audits.forEach(audit => audit.score = 0);
+      category.audits.forEach(audit => audit.result.score = 0);
       const elem = renderer.render(category, sampleResults.reportGroups);
       const passedAudits = elem.querySelectorAll('.lh-passed-audits > .lh-audit');
       const failedAudits = elem.querySelectorAll('.lh-failed-audits > .lh-audit');

lighthouse-core/test/report/v2/renderer/performance-category-renderer-test.js

@@ -17,6 +17,7 @@ const DetailsRenderer = require('../../../../report/v2/renderer/details-renderer
 const CriticalRequestChainRenderer = require(
     '../../../../report/v2/renderer/crc-details-renderer.js');
 const CategoryRenderer = require('../../../../report/v2/renderer/category-renderer.js');
+const ReportRenderer = require('../../../../report/v2/renderer/report-renderer.js');
 const sampleResults = require('../../../results/sample_v2.json');
 
 const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../../report/v2/templates.html', 'utf8');
@@ -37,6 +38,8 @@ describe('PerfCategoryRenderer', () => {
     const dom = new DOM(document);
     const detailsRenderer = new DetailsRenderer(dom);
     renderer = new PerformanceCategoryRenderer(dom, detailsRenderer);
+    ReportRenderer.smooshAuditResultsIntoCategories(sampleResults.audits,
+      sampleResults.reportCategories);
   });
 
   after(() => {
@@ -81,7 +84,7 @@ describe('PerfCategoryRenderer', () => {
     const categoryDOM = renderer.render(category, sampleResults.reportGroups);
 
     const hintAudits = category.audits.filter(audit => audit.group === 'perf-hint' &&
-        audit.score !== 100);
+        audit.result.score !== 100);
     const hintElements = categoryDOM.querySelectorAll('.lh-perf-hint');
     assert.equal(hintElements.length, hintAudits.length);
 
@@ -99,7 +102,7 @@ describe('PerfCategoryRenderer', () => {
       group: 'perf-hint',
       result: {
         rawValue: 100, debugString: 'Yikes!', description: 'Bug',
-        helpText: '',
+        helpText: '', score: 32,
         details: {summary: {wastedMs: 3223}},
       },
     };
@@ -134,7 +137,8 @@ describe('PerfCategoryRenderer', () => {
       score: 0,
       group: 'perf-hint',
       result: {
-        rawValue: 100, description: 'Bug', helpText: '',
+        rawValue: 100, description: 'Bug',
+        helpText: '', score: 32,
       },
     };
 
@@ -149,7 +153,7 @@ describe('PerfCategoryRenderer', () => {
     const diagnosticSection = categoryDOM.querySelectorAll('.lh-category > .lh-audit-group')[2];
 
     const diagnosticAudits = category.audits.filter(audit => audit.group === 'perf-info' &&
-        audit.score !== 100);
+        audit.result.score !== 100);
     const diagnosticElements = diagnosticSection.querySelectorAll('.lh-audit');
     assert.equal(diagnosticElements.length, diagnosticAudits.length);
   });
@@ -159,7 +163,8 @@ describe('PerfCategoryRenderer', () => {
     const passedSection = categoryDOM.querySelector('.lh-category > .lh-passed-audits');
 
     const passedAudits = category.audits.filter(audit =>
-        audit.group && audit.group !== 'perf-metric' && audit.score === 100);
+        audit.group && audit.group !== 'perf-metric' &&
+        audit.result.score === 100);
     const passedElements = passedSection.querySelectorAll('.lh-audit');
     assert.equal(passedElements.length, passedAudits.length);
   });

lighthouse-core/test/report/v2/renderer/report-renderer-test.js

@@ -115,6 +115,13 @@ describe('ReportRenderer V2', () => {
       });
     });
 
+    it('should not mutate a report object', () => {
+      const container = renderer._dom._document.body;
+      const originalResults = JSON.parse(JSON.stringify(sampleResults));
+      renderer.renderReport(sampleResults, container);
+      assert.deepStrictEqual(sampleResults, originalResults);
+    }).timeout(2000);
+
     it('renders a left nav', () => {
       const header = renderer._renderReportNav(sampleResults);
       const categoryCount = sampleResults.reportCategories.length;