Skip to content

Commit

Permalink
core(metrics): do not round CLS in metrics output
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce committed May 7, 2020
1 parent 24c0c73 commit 7aeb4d2
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 14 deletions.
8 changes: 7 additions & 1 deletion lighthouse-core/audits/metrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
const Audit = require('./audit.js');
const ComputedTimingSummary = require('../computed/metrics/timing-summary.js');

/** @type {Set<keyof LH.Artifacts.TimingSummary>} */
const NON_ROUNDED_METRICS = new Set([
'cumulativeLayoutShift',
'observedCumulativeLayoutShift',
]);

class Metrics extends Audit {
/**
* @return {LH.Audit.Meta}
Expand Down Expand Up @@ -37,7 +43,7 @@ class Metrics extends Audit {

for (const [name, value] of Object.entries(metrics)) {
const key = /** @type {keyof LH.Artifacts.TimingSummary} */ (name);
if (typeof value === 'number') {
if (typeof value === 'number' && !NON_ROUNDED_METRICS.has(key)) {
metrics[key] = Math.round(value);
}
}
Expand Down
10 changes: 6 additions & 4 deletions lighthouse-core/computed/metrics/timing-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class TimingSummary {
const estimatedInputLatency = await EstimatedInputLatency.request(metricComputationData, context); // eslint-disable-line max-len
const totalBlockingTime = await TotalBlockingTime.request(metricComputationData, context); // eslint-disable-line max-len

const cumulativeLayoutShiftValue = cumulativeLayoutShift &&
cumulativeLayoutShift.value !== null ?
cumulativeLayoutShift.value : undefined;

/** @type {LH.Artifacts.TimingSummary} */
const metrics = {
// Include the simulated/observed performance metrics
Expand All @@ -71,8 +75,7 @@ class TimingSummary {
estimatedInputLatencyTs: estimatedInputLatency.timestamp,
totalBlockingTime: totalBlockingTime.timing,
maxPotentialFID: maxPotentialFID && maxPotentialFID.timing,
cumulativeLayoutShift: cumulativeLayoutShift && cumulativeLayoutShift.value !== null ?
cumulativeLayoutShift.value : undefined,
cumulativeLayoutShift: cumulativeLayoutShiftValue,

// Include all timestamps of interest from trace of tab
observedNavigationStart: traceOfTab.timings.navigationStart,
Expand All @@ -91,8 +94,7 @@ class TimingSummary {
observedLoadTs: traceOfTab.timestamps.load,
observedDomContentLoaded: traceOfTab.timings.domContentLoaded,
observedDomContentLoadedTs: traceOfTab.timestamps.domContentLoaded,
observedCumulativeLayoutShift: traceOfTab.timings.cumulativeLayoutShift,
observedCumulativeLayoutShiftTs: traceOfTab.timestamps.cumulativeLayoutShift,
observedCumulativeLayoutShift: cumulativeLayoutShiftValue,

// Include some visual metrics from speedline
observedFirstVisualChange: speedline.first,
Expand Down
51 changes: 47 additions & 4 deletions lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ Object {
"largestContentfulPaint": 2758,
"largestContentfulPaintTs": undefined,
"maxPotentialFID": 1336,
"observedCumulativeLayoutShift": undefined,
"observedCumulativeLayoutShiftTs": undefined,
"observedCumulativeLayoutShift": 0,
"observedDomContentLoaded": 1513,
"observedDomContentLoadedTs": 713038536140,
"observedFirstContentfulPaint": 1122,
Expand Down Expand Up @@ -46,6 +45,51 @@ Object {
}
`;

exports[`Performance: metrics evaluates valid input correctly (throttlingMethod=provided) 1`] = `
Object {
"cumulativeLayoutShift": 0,
"estimatedInputLatency": 17,
"estimatedInputLatencyTs": undefined,
"firstCPUIdle": 1582,
"firstCPUIdleTs": 225415754204,
"firstContentfulPaint": 499,
"firstContentfulPaintTs": 225414670885,
"firstMeaningfulPaint": 783,
"firstMeaningfulPaintTs": 225414955343,
"interactive": 1582,
"interactiveTs": 225415754204,
"largestContentfulPaint": undefined,
"largestContentfulPaintTs": undefined,
"maxPotentialFID": 198,
"observedCumulativeLayoutShift": 0,
"observedDomContentLoaded": 560,
"observedDomContentLoadedTs": 225414732309,
"observedFirstContentfulPaint": 499,
"observedFirstContentfulPaintTs": 225414670885,
"observedFirstMeaningfulPaint": 783,
"observedFirstMeaningfulPaintTs": 225414955343,
"observedFirstPaint": 499,
"observedFirstPaintTs": 225414670868,
"observedFirstVisualChange": 520,
"observedFirstVisualChangeTs": 225414692015,
"observedLargestContentfulPaint": undefined,
"observedLargestContentfulPaintTs": undefined,
"observedLastVisualChange": 818,
"observedLastVisualChangeTs": 225414990015,
"observedLoad": 2199,
"observedLoadTs": 225416370913,
"observedNavigationStart": 0,
"observedNavigationStartTs": 225414172015,
"observedSpeedIndex": 605,
"observedSpeedIndexTs": 225414776724,
"observedTraceEnd": 12540,
"observedTraceEndTs": 225426711887,
"speedIndex": 605,
"speedIndexTs": 225414777015,
"totalBlockingTime": 48,
}
`;

exports[`Performance: metrics evaluates valid input correctly 1`] = `
Object {
"cumulativeLayoutShift": 0,
Expand All @@ -62,8 +106,7 @@ Object {
"largestContentfulPaint": undefined,
"largestContentfulPaintTs": undefined,
"maxPotentialFID": 396,
"observedCumulativeLayoutShift": undefined,
"observedCumulativeLayoutShiftTs": undefined,
"observedCumulativeLayoutShift": 0,
"observedDomContentLoaded": 560,
"observedDomContentLoadedTs": 225414732309,
"observedFirstContentfulPaint": 499,
Expand Down
34 changes: 34 additions & 0 deletions lighthouse-core/test/audits/metrics-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ const pwaDevtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.
const lcpTrace = require('../fixtures/traces/lcp-m78.json');
const lcpDevtoolsLog = require('../fixtures/traces/lcp-m78.devtools.log.json');

const artifactsTrace = require('../results/artifacts/defaultPass.trace.json');
const artifactsDevtoolsLog = require('../results/artifacts/defaultPass.devtoolslog.json');

/* eslint-env jest */

describe('Performance: metrics', () => {
Expand All @@ -32,6 +35,21 @@ describe('Performance: metrics', () => {
expect(result.details.items[0]).toMatchSnapshot();
});

it('evaluates valid input correctly (throttlingMethod=provided)', async () => {
const artifacts = {
traces: {
[MetricsAudit.DEFAULT_PASS]: pwaTrace,
},
devtoolsLogs: {
[MetricsAudit.DEFAULT_PASS]: pwaDevtoolsLog,
},
};

const context = {settings: {throttlingMethod: 'provided'}, computedCache: new Map()};
const result = await MetricsAudit.audit(artifacts, context);
expect(result.details.items[0]).toMatchSnapshot();
});

it('evaluates valid input (with lcp) correctly', async () => {
const artifacts = {
traces: {
Expand All @@ -47,6 +65,22 @@ describe('Performance: metrics', () => {
expect(result.details.items[0]).toMatchSnapshot();
});

it('evaluates valid input (with CLS) correctly', async () => {
const artifacts = {
traces: {
[MetricsAudit.DEFAULT_PASS]: artifactsTrace,
},
devtoolsLogs: {
[MetricsAudit.DEFAULT_PASS]: artifactsDevtoolsLog,
},
};

const context = {settings: {throttlingMethod: 'simulate'}, computedCache: new Map()};
const {details} = await MetricsAudit.audit(artifacts, context);
expect(details.items[0].cumulativeLayoutShift).toMatchInlineSnapshot(`0.42`);
expect(details.items[0].observedCumulativeLayoutShift).toMatchInlineSnapshot(`0.42`);
});

it('does not fail the entire audit when TTI errors', async () => {
const artifacts = {
traces: {
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/test/computed/metrics/timing-summary-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ describe('Timing summary', () => {
"largestContentfulPaint": undefined,
"largestContentfulPaintTs": undefined,
"maxPotentialFID": 396.0000000000001,
"observedCumulativeLayoutShift": undefined,
"observedCumulativeLayoutShiftTs": undefined,
"observedCumulativeLayoutShift": 0,
"observedDomContentLoaded": 560.294,
"observedDomContentLoadedTs": 225414732309,
"observedFirstContentfulPaint": 498.87,
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,7 @@
"estimatedInputLatency": 16,
"totalBlockingTime": 117,
"maxPotentialFID": 123,
"cumulativeLayoutShift": 0,
"cumulativeLayoutShift": 0.42,
"observedNavigationStart": 0,
"observedNavigationStartTs": 185603319912,
"observedFirstPaint": 3969,
Expand All @@ -1397,6 +1397,7 @@
"observedLoadTs": 185608244374,
"observedDomContentLoaded": 4901,
"observedDomContentLoadedTs": 185608220734,
"observedCumulativeLayoutShift": 0.42,
"observedFirstVisualChange": 3969,
"observedFirstVisualChangeTs": 185607288912,
"observedLastVisualChange": 4791,
Expand Down
2 changes: 0 additions & 2 deletions types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,6 @@ declare global {
traceEnd: number;
load?: number;
domContentLoaded?: number;
cumulativeLayoutShift?: number;
}

export interface TraceOfTab {
Expand Down Expand Up @@ -637,7 +636,6 @@ declare global {
observedNavigationStart: number;
observedNavigationStartTs: number;
observedCumulativeLayoutShift: number | undefined;
observedCumulativeLayoutShiftTs: number | undefined;
observedFirstPaint: number | undefined;
observedFirstPaintTs: number | undefined;
observedFirstContentfulPaint: number;
Expand Down

0 comments on commit 7aeb4d2

Please sign in to comment.