Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(audits): clarify diff between load-fast-enough-for-pwa's TTI and perf TTI #6286

Merged
merged 5 commits into from
Oct 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions lighthouse-core/audits/load-fast-enough-for-pwa.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const Audit = require('./audit');
const mobileThrottling = require('../config/constants').throttling.mobileSlow4G;
const Interactive = require('../gather/computed/metrics/interactive.js');

const displayValueText = `Interactive at %d\xa0s`;
const displayValueTextWithOverride = `Interactive on simulated mobile network at %d\xa0s`;

// Maximum TTI to be considered "fast" for PWA baseline checklist
// https://developers.google.com/web/progressive-web-apps/checklist
const MAXIMUM_TTI = 10 * 1000;
Expand Down Expand Up @@ -48,11 +51,16 @@ class LoadFastEnough4Pwa extends Audit {
// If throttling was default devtools or lantern slow 4G throttling, then reuse the given settings
// Otherwise, we'll force the usage of lantern slow 4G.
const settingOverrides = {throttlingMethod: 'simulate', throttling: mobileThrottling};
const settings =
context.settings.throttlingMethod !== 'provided' &&
isDeepEqual(context.settings.throttling, mobileThrottling)
? context.settings
: Object.assign({}, context.settings, settingOverrides);

// Override settings for interactive if provided throttling was used or network
// throttling was not consistent with standard `mobile network throttling`
const override = context.settings.throttlingMethod === 'provided' ||
!isDeepEqual(context.settings.throttling, mobileThrottling);

const displayValueFinal = override ? displayValueTextWithOverride : displayValueText;

const settings = override ? Object.assign({}, context.settings, settingOverrides) :
context.settings;

const metricComputationData = {trace, devtoolsLog, settings};
const tti = await Interactive.request(metricComputationData, context);
Expand All @@ -64,7 +72,7 @@ class LoadFastEnough4Pwa extends Audit {
/** @type {string|undefined} */
let explanation;
if (!score) {
displayValue = [`Interactive at %d\xa0s`, tti.timing / 1000];
displayValue = [displayValueFinal, tti.timing / 1000];
explanation = 'Your page loads too slowly and is not interactive within 10 seconds. ' +
'Look at the opportunities and diagnostics in the "Performance" section to learn how to ' +
'improve.';
Expand Down
35 changes: 34 additions & 1 deletion lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,40 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {

const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
const result = await FastPWAAudit.audit(artifacts, {settings, computedCache: new Map()});
expect(result.rawValue).toBeGreaterThan(2000);
expect(result.rawValue).toBeGreaterThan(2000); // If not overridden this would be 1582
expect(Math.round(result.rawValue)).toMatchSnapshot();
});

it('overrides when throttling is modified but method is not "provided"', async () => {
const artifacts = {
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: devtoolsLog},
};

const settings = {throttlingMethod: 'devtools', throttling: {rttMs: 40, throughput: 100000}};
const result = await FastPWAAudit.audit(artifacts, {settings, computedCache: new Map()});
expect(result.rawValue).toBeGreaterThan(2000); // If not overridden this would be 1582
});

it('overrides when throttling is "provided" and fails the simulated TTI value', async () => {
const topLevelTasks = [
{ts: 1000, duration: 1000},
{ts: 3000, duration: 1000},
{ts: 5000, duration: 1000},
{ts: 9000, duration: 1000},
{ts: 12000, duration: 1000},
{ts: 14900, duration: 1000},
];
const longTrace = createTestTrace({navigationStart: 0, traceEnd: 20000, topLevelTasks});

const artifacts = {
traces: {defaultPass: longTrace},
devtoolsLogs: {defaultPass: devtoolsLog},
};

const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
const result = await FastPWAAudit.audit(artifacts, {settings, computedCache: new Map()});
expect(result.displayValue).toContain('Interactive on simulated mobile network at %d\xa0s');
expect(result.rawValue).toBeGreaterThan(10000);
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
});
});