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(fr): split timespan support for server-response-time #12758

Merged
merged 3 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
22 changes: 20 additions & 2 deletions lighthouse-core/audits/server-response-time.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
const Audit = require('./audit.js');
const i18n = require('../lib/i18n/i18n.js');
const MainResource = require('../computed/main-resource.js');
const NetworkRecords = require('../computed/network-records.js');
const NetworkAnalyzer = require('../lib/dependency-graph/simulator/network-analyzer.js');

const UIStrings = {
/** Title of a diagnostic audit that provides detail on how long it took from starting a request to when the server started responding. This descriptive title is shown to users when the amount is acceptable and no user action is required. */
Expand Down Expand Up @@ -37,7 +39,8 @@ class ServerResponseTime extends Audit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs', 'URL'],
supportedModes: ['timespan', 'navigation'],
requiredArtifacts: ['devtoolsLogs', 'URL', 'GatherContext'],
};
}

Expand All @@ -56,7 +59,22 @@ class ServerResponseTime extends Audit {
*/
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);

/** @type {LH.Artifacts.NetworkRequest} */
let mainResource;
if (artifacts.GatherContext.gatherMode === 'timespan') {
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const optionalMainResource = NetworkAnalyzer.findOptionalMainDocument(
networkRecords,
artifacts.URL.finalUrl
);
if (!optionalMainResource) {
return {score: null, notApplicable: true};
}
mainResource = optionalMainResource;
} else {
mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);
}

const responseTime = ServerResponseTime.calculateResponseTime(mainResource);
const passed = responseTime < TOO_SLOW_THRESHOLD_MS;
Expand Down
62 changes: 57 additions & 5 deletions lighthouse-core/test/audits/server-response-time-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
'use strict';

const ServerResponseTime = require('../../audits/server-response-time.js');
const assert = require('assert').strict;
const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js');

/* eslint-env jest */
Expand All @@ -22,6 +21,7 @@ describe('Performance: server-response-time audit', () => {
const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'navigation'},
};

const result = await ServerResponseTime.audit(artifacts, {computedCache: new Map()});
Expand All @@ -35,7 +35,7 @@ describe('Performance: server-response-time audit', () => {
});
});

it('succeeds when response time of root document is lower than 600ms', () => {
it('succeeds when response time of root document is lower than 600ms', async () => {
const mainResource = {
url: 'https://example.com/',
requestId: '0',
Expand All @@ -46,11 +46,63 @@ describe('Performance: server-response-time audit', () => {
const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'navigation'},
};

return ServerResponseTime.audit(artifacts, {computedCache: new Map()}).then(result => {
assert.strictEqual(result.numericValue, 200);
assert.strictEqual(result.score, 1);
const result = await ServerResponseTime.audit(artifacts, {computedCache: new Map()});
expect(result).toMatchObject({
numericValue: 200,
score: 1,
});
});

it('identifies main resource in timespan mode', async () => {
const mainResource = {
url: 'https://example.com/',
requestId: '0',
timing: {receiveHeadersEnd: 400, sendEnd: 200},
};
const devtoolsLog = networkRecordsToDevtoolsLog([mainResource]);

const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'timespan'},
};

const result = await ServerResponseTime.audit(artifacts, {computedCache: new Map()});
expect(result).toMatchObject({
numericValue: 200,
score: 1,
});
});

it('result is n/a if no main resource in timespan', async () => {
const devtoolsLog = networkRecordsToDevtoolsLog([]);

const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'timespan'},
};

const result = await ServerResponseTime.audit(artifacts, {computedCache: new Map()});
expect(result).toEqual({
score: null,
notApplicable: true,
});
});

it('throws error if no main resource in navigation', async () => {
const devtoolsLog = networkRecordsToDevtoolsLog([]);

const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'navigation'},
};

const resultPromise = ServerResponseTime.audit(artifacts, {computedCache: new Map()});
await expect(resultPromise).rejects.toThrow(/Unable to identify the main resource/);
});
});
39 changes: 37 additions & 2 deletions lighthouse-core/test/fraggle-rock/api-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ function getAuditsBreakdown(lhr) {
audit => !irrelevantDisplayModes.has(audit.scoreDisplayMode)
);

const notApplicableAudits = auditResults.filter(
audit => audit.scoreDisplayMode === 'notApplicable'
);

const informativeAudits = applicableAudits.filter(
audit => audit.scoreDisplayMode === 'informative'
);
Expand All @@ -34,7 +38,7 @@ function getAuditsBreakdown(lhr) {

const failedAudits = applicableAudits.filter(audit => audit.score !== null && audit.score < 1);

return {auditResults, erroredAudits, failedAudits, informativeAudits};
return {auditResults, erroredAudits, failedAudits, informativeAudits, notApplicableAudits};
}

describe('Fraggle Rock API', () => {
Expand Down Expand Up @@ -119,10 +123,18 @@ describe('Fraggle Rock API', () => {
const bestPractices = lhr.categories['best-practices'];
expect(bestPractices.score).toBeLessThan(1);

const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
const {
auditResults,
erroredAudits,
failedAudits,
notApplicableAudits,
} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`63`);

expect(notApplicableAudits).toHaveLength(8);
adamraine marked this conversation as resolved.
Show resolved Hide resolved
expect(notApplicableAudits.map(audit => audit.id)).not.toContain('server-response-time');

expect(erroredAudits).toHaveLength(0);
expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');

Expand All @@ -145,6 +157,29 @@ describe('Fraggle Rock API', () => {
if (!details || details.type !== 'table') throw new Error('Unexpected byte weight details');
expect(details.items).toMatchObject([{url: `${serverBaseUrl}/onclick.html`}]);
});

it('should compute results from timespan after page load', async () => {
await page.goto(`${serverBaseUrl}/onclick.html`);
await page.waitForSelector('button');

const run = await lighthouse.startTimespan({page});

await page.click('button');
await page.waitForSelector('input');

const result = await run.endTimespan();

if (!result) throw new Error('Lighthouse failed to produce a result');

const {auditResults, erroredAudits, notApplicableAudits} = getAuditsBreakdown(result.lhr);
expect(auditResults.length).toMatchInlineSnapshot(`63`);

expect(notApplicableAudits).toHaveLength(5);
adamraine marked this conversation as resolved.
Show resolved Hide resolved
expect(notApplicableAudits.map(audit => audit.id)).toContain('server-response-time');

// TODO(FR-COMPAT): Reduce this number by handling the error, making N/A, or removing timespan support.
expect(erroredAudits).toHaveLength(22);
adamraine marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('navigation', () => {
Expand Down