Skip to content

Commit

Permalink
extract reportCategories/score changes
Browse files Browse the repository at this point in the history
  • Loading branch information
paulirish committed Mar 7, 2018
1 parent a594d44 commit 72c8a6e
Show file tree
Hide file tree
Showing 17 changed files with 5,364 additions and 357 deletions.
7 changes: 0 additions & 7 deletions lighthouse-cli/test/smokehouse/smokehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,6 @@ function collateResults(actual, expected) {
throw new Error(`Config did not trigger run of expected audit ${auditName}`);
}

// TODO: TERRIBLE HACK, remove asap
// Smokehouse has been asserting the AuditResult.score, whereas report cares about AuditDfn score (within reportCategories)
// A subsequent PR will change scores completely which means rebaselining all our smokehouse expectations
if (actualResult.scoringMode === 'binary') {
actualResult.score = Boolean(actualResult.score);
}

const expectedResult = expected.audits[auditName];
const diff = findDifference(auditName, actualResult, expectedResult);

Expand Down
17 changes: 1 addition & 16 deletions lighthouse-core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,35 +114,20 @@ class Audit {
if (displayValue === score) {
displayValue = '';
}

// TODO: restore after initial 3.0 branching
// if (typeof score === 'boolean' || score === null) {
// score = score ? 100 : 0;
// }

// if (!Number.isFinite(score)) {
// throw new Error(`Invalid score: ${score}`);
// }

// TODO, don't consider an auditResult's scoringMode (currently applied to all ByteEfficiency)
const scoringMode = result.scoringMode || audit.meta.scoringMode || Audit.SCORING_MODES.BINARY;
delete result.scoringMode;

let auditDescription = audit.meta.description;
if (audit.meta.failureDescription) {
if (!score || (typeof score === 'number' && score < 100)) {
auditDescription = audit.meta.failureDescription;
}
}

return {
score,
displayValue: `${displayValue}`,
rawValue: result.rawValue,
error: result.error,
debugString: result.debugString,
extendedInfo: result.extendedInfo,
scoringMode,
scoringMode: audit.meta.scoringMode || Audit.SCORING_MODES.BINARY,
informative: audit.meta.informative,
manual: audit.meta.manual,
notApplicable: result.notApplicable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ class UnusedBytes extends Audit {
displayValue,
rawValue: wastedMs,
score: UnusedBytes.scoreForWastedMs(wastedMs),
scoringMode: Audit.SCORING_MODES.NUMERIC,
extendedInfo: {
value: {
wastedMs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class LinkBlockingFirstPaintAudit extends Audit {
name: 'link-blocking-first-paint',
description: 'Reduce render-blocking stylesheets',
informative: true,
scoringMode: Audit.SCORING_MODES.NUMERIC,
helpText: 'External stylesheets are blocking the first paint of your page. Consider ' +
'delivering critical CSS via `<style>` tags and deferring non-critical ' +
'styles. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/blocking-resources).',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class ScriptBlockingFirstPaint extends Audit {
name: 'script-blocking-first-paint',
description: 'Reduce render-blocking scripts',
informative: true,
scoringMode: Audit.SCORING_MODES.NUMERIC,
helpText: 'Script elements are blocking the first paint of your page. Consider inlining ' +
'critical scripts and deferring non-critical ones. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/blocking-resources).',
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/audits/redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class Redirects extends Audit {
name: 'redirects',
description: 'Avoids page redirects',
failureDescription: 'Has multiple page redirects',
scoringMode: Audit.SCORING_MODES.NUMERIC,
helpText: 'Redirects introduce additional delays before the page can be loaded. [Learn more](https://developers.google.com/speed/docs/insights/AvoidRedirects).',
requiredArtifacts: ['URL', 'devtoolsLogs'],
};
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ 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},
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/report/v2/renderer/category-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class CategoryRenderer {
scoreEl.classList.add('lh-score--manual');
}

return this._populateScore(tmpl, audit.result.score, scoringMode, title, description);
return this._populateScore(tmpl, audit.score, scoringMode, title, description);
}

/**
Expand Down Expand Up @@ -296,7 +296,7 @@ class CategoryRenderer {

if (audit.result.notApplicable) {
group.notApplicable.push(audit);
} else if (audit.result.score === 100 && !audit.result.debugString) {
} else if (audit.score === 100 && !audit.result.debugString) {
group.passed.push(audit);
} else {
group.failed.push(audit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.result.score)}`);
element.classList.add(`lh-timeline-metric--${Util.calculateRating(audit.score)}`);

const titleEl = this.dom.find('.lh-timeline-metric__title', tmpl);
titleEl.textContent = audit.result.description;
Expand Down Expand Up @@ -49,7 +49,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {

const element = this.dom.createElement('details', [
'lh-perf-hint',
`lh-perf-hint--${Util.calculateRating(audit.result.score)}`,
`lh-perf-hint--${Util.calculateRating(audit.score)}`,
'lh-expandable-details',
].join(' '));

Expand Down Expand Up @@ -152,7 +152,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
element.appendChild(metricAuditsEl);

const hintAudits = category.audits
.filter(audit => audit.group === 'perf-hint' && audit.result.score < 100)
.filter(audit => audit.group === 'perf-hint' && audit.score < 100)
.sort((auditA, auditB) => auditB.result.rawValue - auditA.result.rawValue);
if (hintAudits.length) {
const maxWaste = Math.max(...hintAudits.map(audit => audit.result.rawValue));
Expand All @@ -164,7 +164,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
}

const infoAudits = category.audits
.filter(audit => audit.group === 'perf-info' && audit.result.score < 100);
.filter(audit => audit.group === 'perf-info' && audit.score < 100);
if (infoAudits.length) {
const infoAuditsEl = this.renderAuditGroup(groups['perf-info'], {expandable: false});
infoAudits.forEach(item => infoAuditsEl.appendChild(this.renderAudit(item)));
Expand All @@ -174,7 +174,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {

const passedElements = category.audits
.filter(audit => (audit.group === 'perf-hint' || audit.group === 'perf-info') &&
audit.result.score === 100)
audit.score === 100)
.map(audit => this.renderAudit(audit));

if (!passedElements.length) return element;
Expand Down
22 changes: 4 additions & 18 deletions lighthouse-core/report/v2/renderer/report-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,39 +155,24 @@ class ReportRenderer {
perfCategoryRenderer.setTemplateContext(this._templateContext);

const categories = reportSection.appendChild(this._dom.createElement('div', 'lh-categories'));

ReportRenderer.assignAuditResultsIntoCategories(report.audits, report.reportCategories);

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 assignAuditResultsIntoCategories(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) {
Expand All @@ -199,6 +184,7 @@ if (typeof module !== 'undefined' && module.exports) {
/**
* @typedef {{
* id: string,
* score: number,
* weight: number,
* group: (string|undefined),
* result: (ReportRenderer.AuditResultJSON|undefined)
Expand All @@ -219,7 +205,6 @@ ReportRenderer.AuditJSON; // eslint-disable-line no-unused-expressions
* scoringMode: string,
* extendedInfo: Object,
* error: boolean,
* score: number,
* details: (!DetailsRenderer.DetailsJSON|undefined),
* }}
*/
Expand All @@ -229,6 +214,7 @@ ReportRenderer.AuditResultJSON; // eslint-disable-line no-unused-expressions
* @typedef {{
* name: string,
* id: string,
* weight: number,
* score: number,
* description: string,
* audits: !Array<!ReportRenderer.AuditJSON>
Expand Down
11 changes: 5 additions & 6 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,9 @@ class Runner {
const resultsById = {};
for (const audit of runResults.auditResults) resultsById[audit.name] = audit;

let categories;
let report;
if (opts.config.categories) {
Runner._scoreAndCategorize(opts, resultsById);
// COMPAT: after dropping Node 6, upgrade to use Object.values
categories = Object.keys(opts.config.categories).map(key => opts.config.categories[key]);
report = Runner._scoreAndCategorize(opts, resultsById);
}

return {
Expand All @@ -110,7 +108,8 @@ class Runner {
audits: resultsById,
artifacts: runResults.artifacts,
runtimeConfig: Runner.getRuntimeConfig(opts.flags),
reportCategories: categories,
score: report ? report.score : 0,
reportCategories: report ? report.categories : [],
reportGroups: opts.config.groups,
};
}).catch(err => {
Expand Down Expand Up @@ -193,7 +192,7 @@ class Runner {
* @param {{}} resultsById
*/
static _scoreAndCategorize(opts, resultsById) {
ReportScoring.scoreAllCategories(opts.config, resultsById);
return ReportScoring.scoreAllCategories(opts.config, resultsById);
}

/**
Expand Down
43 changes: 19 additions & 24 deletions lighthouse-core/scoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,30 @@ class ReportScoring {
* @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) => {
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});

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, score: number|undefined, audits: !Array<{id: string, weight: number|undefined}>}>}} config
* @param {{categories: !Object<string, {id: string|undefined, weight: number|undefined, audits: !Array<{id: string, weight: number|undefined}>}>}} config
* @param {!Object<{score: ?number|boolean|undefined}>} resultsByAuditId
* @void
* @return {{score: number, categories: !Array<{audits: !Array<{score: number, result: !Object}>}>}}
*/
static scoreAllCategories(config, resultsByAuditId) {
Object.keys(config.categories).forEach(categoryId => {
const categories = Object.keys(config.categories).map(categoryId => {
const category = config.categories[categoryId];
category.id = categoryId;

category.audits.forEach(audit => {
const audits = category.audits.map(audit => {
const result = resultsByAuditId[audit.id];
// Cast to number to catch `null` and undefined when audits error
let auditScore = Number(result.score) || 0;
Expand All @@ -56,17 +53,15 @@ class ReportScoring {
result.informative = true;
}

result.score = auditScore;
return Object.assign({}, audit, {result, score: auditScore});
});

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;
const categoryScore = ReportScoring.arithmeticMean(audits);
return Object.assign({}, category, {audits, score: categoryScore});
});

const overallScore = ReportScoring.arithmeticMean(categories);
return {score: overallScore, categories};
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ 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');
Expand All @@ -34,9 +33,6 @@ describe('CategoryRenderer', () => {
const dom = new DOM(document);
const detailsRenderer = new DetailsRenderer(dom);
renderer = new CategoryRenderer(dom, detailsRenderer);

ReportRenderer.assignAuditResultsIntoCategories(sampleResults.audits,
sampleResults.reportCategories);
});

after(() => {
Expand Down Expand Up @@ -182,7 +178,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.result.score !== 100 && !audit.result.notApplicable;
return audit.score !== 100 && !audit.result.notApplicable;
});
const failedAuditTags = new Set(failedAudits.map(audit => audit.group));

Expand All @@ -192,10 +188,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.result.score === 100);
!audit.result.notApplicable && audit.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);
});
Expand Down Expand Up @@ -223,7 +219,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.result.score = 0);
category.audits.forEach(audit => audit.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');
Expand Down
Loading

0 comments on commit 72c8a6e

Please sign in to comment.