Repository: GoogleChrome/lighthouse
Pull request #12758 (2021-07-07 20:11:33)

core(fr): split timespan support for server-response-time
`server-response-time` already supported timespan mode, this just makes the audit N/A if the main resource is unavailable.

The additional puppeteer revealed many erroneous audits when the timespan is started after page load. Most failures come from a missing main resource, but others may require a follow-up investigation.


Patch:
diff --git a/lighthouse-core/audits/server-response-time.js b/lighthouse-core/audits/server-response-time.js
index f2203e025..6f77f3e78 100644
--- a/lighthouse-core/audits/server-response-time.js
+++ b/lighthouse-core/audits/server-response-time.js
@@ -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. */
@@ -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'],
     };
   }
 
@@ -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;


Test patch:
diff --git a/lighthouse-core/test/audits/server-response-time-test.js b/lighthouse-core/test/audits/server-response-time-test.js
index d9a39f3cc..90ba34799 100644
--- a/lighthouse-core/test/audits/server-response-time-test.js
+++ b/lighthouse-core/test/audits/server-response-time-test.js
@@ -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 */
@@ -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()});
@@ -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',
@@ -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/);
+  });
 });
diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
index 13f775f98..b8a648603 100644
--- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js
+++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
@@ -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'
   );
@@ -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', () => {
@@ -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.length).toMatchInlineSnapshot(`8`);
+      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');
 
@@ -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.length).toMatchInlineSnapshot(`5`);
+      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.length).toMatchInlineSnapshot(`22`);
+    });
   });
 
   describe('navigation', () => {