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

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;