Repository: GoogleChrome/lighthouse
Pull request #12905 (2021-08-11 22:50:24)

misc(build): prefer paths when using gh-pages-app


Patch:
diff --git a/build/build-bundle.js b/build/build-bundle.js
index c29375081..5156133c8 100644
--- a/build/build-bundle.js
+++ b/build/build-bundle.js
@@ -13,6 +13,7 @@
 const fs = require('fs');
 const path = require('path');
 const assert = require('assert').strict;
+const stream = require('stream');
 const mkdir = fs.promises.mkdir;
 const LighthouseRunner = require('../lighthouse-core/runner.js');
 const exorcist = require('exorcist');
@@ -66,7 +67,23 @@ async function browserifyFile(entryPath, distPath) {
     })
     // Transform the fs.readFile etc into inline strings.
     .transform('@wardpeet/brfs', {
-      readFileSyncTransform: minifyFileTransform,
+      /** @param {string} file */
+      readFileTransform: (file) => {
+        // Don't include locales in DevTools.
+        if (isDevtools(entryPath) && locales.includes(file)) {
+          return new stream.Transform({
+            transform(chunk, enc, next) {
+              next();
+            },
+            final(next) {
+              this.push('{}');
+              next();
+            },
+          });
+        }
+
+        return minifyFileTransform(file);
+      },
       global: true,
       parserOpts: {ecmaVersion: 12},
     })
@@ -90,12 +107,6 @@ async function browserifyFile(entryPath, distPath) {
     bundle.ignore(require.resolve('../report/report-assets.js'));
   }
 
-  // Don't include locales in DevTools.
-  if (isDevtools(entryPath)) {
-    // @ts-expect-error bundle.ignore does accept an array of strings.
-    bundle.ignore(locales);
-  }
-
   // Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded.
   // Exposed path must be a relative path from lighthouse-core/config/config-helpers.js (where loading occurs).
   const corePath = './lighthouse-core/';
diff --git a/build/build-lightrider-bundles.js b/build/build-lightrider-bundles.js
index 27970be71..6a570d397 100644
--- a/build/build-lightrider-bundles.js
+++ b/build/build-lightrider-bundles.js
@@ -40,7 +40,7 @@ function buildReportGenerator() {
   browserify(generatorFilename, {standalone: 'ReportGenerator'})
     // Transform the fs.readFile etc into inline strings.
     .transform('@wardpeet/brfs', {
-      readFileSyncTransform: minifyFileTransform,
+      readFileTransform: minifyFileTransform,
       global: true,
       parserOpts: {ecmaVersion: 12},
     })
diff --git a/build/build-treemap.js b/build/build-treemap.js
index 63f4b724e..255575ee5 100644
--- a/build/build-treemap.js
+++ b/build/build-treemap.js
@@ -49,21 +49,19 @@ async function run() {
     appDir: `${LH_ROOT}/lighthouse-treemap/app`,
     html: {path: 'index.html'},
     stylesheets: [
-      fs.readFileSync(require.resolve('tabulator-tables/dist/css/tabulator.min.css'), 'utf8'),
+      {path: require.resolve('tabulator-tables/dist/css/tabulator.min.css')},
       {path: 'styles/*'},
     ],
     javascripts: [
-      /* eslint-disable max-len */
-      fs.readFileSync(require.resolve('idb-keyval/dist/idb-keyval-min.js'), 'utf8'),
-      fs.readFileSync(require.resolve('event-target-shim/umd'), 'utf8'),
-      fs.readFileSync(require.resolve('webtreemap-cdt'), 'utf8'),
-      fs.readFileSync(require.resolve('tabulator-tables/dist/js/tabulator_core.js'), 'utf8'),
-      fs.readFileSync(require.resolve('tabulator-tables/dist/js/modules/sort.js'), 'utf8'),
-      fs.readFileSync(require.resolve('tabulator-tables/dist/js/modules/format.js'), 'utf8'),
-      fs.readFileSync(require.resolve('tabulator-tables/dist/js/modules/resize_columns.js'), 'utf8'),
-      fs.readFileSync(require.resolve('pako/dist/pako_inflate.js'), 'utf-8'),
-      /* eslint-enable max-len */
       buildStrings(),
+      {path: require.resolve('idb-keyval/dist/idb-keyval-min.js')},
+      {path: require.resolve('event-target-shim/umd')},
+      {path: require.resolve('webtreemap-cdt')},
+      {path: require.resolve('tabulator-tables/dist/js/tabulator_core.js')},
+      {path: require.resolve('tabulator-tables/dist/js/modules/sort.js')},
+      {path: require.resolve('tabulator-tables/dist/js/modules/format.js')},
+      {path: require.resolve('tabulator-tables/dist/js/modules/resize_columns.js')},
+      {path: require.resolve('pako/dist/pako_inflate.js')},
       {path: 'src/main.js', rollup: true},
     ],
     assets: [
diff --git a/build/build-viewer.js b/build/build-viewer.js
index 32afb9f24..cce422999 100644
--- a/build/build-viewer.js
+++ b/build/build-viewer.js
@@ -5,7 +5,6 @@
  */
 'use strict';
 
-const fs = require('fs');
 const browserify = require('browserify');
 const GhPagesApp = require('./gh-pages-app.js');
 const {minifyFileTransform} = require('./build-utils.js');
@@ -20,7 +19,7 @@ async function run() {
   const generatorFilename = `${LH_ROOT}/report/report-generator.js`;
   const generatorBrowserify = browserify(generatorFilename, {standalone: 'ReportGenerator'})
     .transform('@wardpeet/brfs', {
-      readFileSyncTransform: minifyFileTransform,
+      readFileTransform: minifyFileTransform,
     });
 
   /** @type {Promise<string>} */
@@ -41,7 +40,7 @@ async function run() {
     ],
     javascripts: [
       await generatorJsPromise,
-      fs.readFileSync(require.resolve('pako/dist/pako_inflate.js'), 'utf-8'),
+      {path: require.resolve('pako/dist/pako_inflate.js')},
       {path: 'src/main.js', rollup: true},
     ],
     assets: [
diff --git a/build/gh-pages-app.js b/build/gh-pages-app.js
index 9456151ca..0b21cbcc7 100644
--- a/build/gh-pages-app.js
+++ b/build/gh-pages-app.js
@@ -131,9 +131,9 @@ class GhPagesApp {
       if (typeof source === 'string') {
         result.push(source);
       } else if (source.rollup) {
-        result.push(await this._rollupSource(`${this.opts.appDir}/${source.path}`));
+        result.push(await this._rollupSource(path.resolve(this.opts.appDir, source.path)));
       } else {
-        result.push(...loadFiles(`${this.opts.appDir}/${source.path}`));
+        result.push(...loadFiles(path.resolve(this.opts.appDir, source.path)));
       }
     }
 
diff --git a/docs/hacking-tips.md b/docs/hacking-tips.md
index 25140c015..74735a069 100644
--- a/docs/hacking-tips.md
+++ b/docs/hacking-tips.md
@@ -1,5 +1,10 @@
 A few assorted scripts and tips to make hacking on Lighthouse a bit easier
 
+# Eng team resources
+
+* [LH Build Tracker](https://lh-build-tracker.herokuapp.com/builds/limit/100) - plotted results of [build-tracker](../build-tracker.config.js) [data](../.github/workflows/ci.yml#:~:text=buildtracker)
+* [LH PR Tracking](https://paulirish.github.io/lh-pr-tracking/) - stats about open PRs, collected [daily](https://github.com/paulirish/lh-pr-tracking/blob/master/.github/workflows/update-stats.yml).
+
 ## Evaluate Lighthouse's runtime performance
 
 Lighthouse has instrumentation to collect timing data for its operations. The data is exposed at `LHR.timing.entries`.  You can generate a trace from this data for closer analysis.
diff --git a/docs/recipes/auth/README.md b/docs/recipes/auth/README.md
index b2aa3db76..0183b7582 100644
--- a/docs/recipes/auth/README.md
+++ b/docs/recipes/auth/README.md
@@ -21,6 +21,7 @@ The server responds with different HTML for each of these pages and session stat
 ```sh
 # be in root lighthouse directory
 yarn # install global project deps
+yarn build-report
 cd docs/recipes/auth
 yarn # install deps related to just this recipe
 yarn start # start the server on http://localhost:10632
diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js
index 43b608ac8..59d2014a9 100644
--- a/lighthouse-core/config/config.js
+++ b/lighthouse-core/config/config.js
@@ -508,12 +508,10 @@ class Config {
 
     // The `full-page-screenshot` audit belongs to no category, but we still want to include
     // it (unless explictly excluded) because there are audits in every category that can use it.
-    if (settings.onlyCategories) {
-      const explicitlyExcludesFullPageScreenshot =
-        settings.skipAudits && settings.skipAudits.includes('full-page-screenshot');
-      if (!explicitlyExcludesFullPageScreenshot) {
-        includedAudits.add('full-page-screenshot');
-      }
+    const explicitlyExcludesFullPageScreenshot =
+      settings.skipAudits && settings.skipAudits.includes('full-page-screenshot');
+    if (!explicitlyExcludesFullPageScreenshot && (settings.onlyCategories || settings.skipAudits)) {
+      includedAudits.add('full-page-screenshot');
     }
 
     return {categories, requestedAuditNames: includedAudits};
diff --git a/lighthouse-core/fraggle-rock/gather/session.js b/lighthouse-core/fraggle-rock/gather/session.js
index b5d34b044..4168ccd96 100644
--- a/lighthouse-core/fraggle-rock/gather/session.js
+++ b/lighthouse-core/fraggle-rock/gather/session.js
@@ -5,8 +5,12 @@
  */
 'use strict';
 
+const LHError = require('../../lib/lh-error.js');
 const SessionEmitMonkeypatch = Symbol('monkeypatch');
 
+// Controls how long to wait for a response after sending a DevTools protocol command.
+const DEFAULT_PROTOCOL_TIMEOUT = 30000;
+
 /** @implements {LH.Gatherer.FRProtocolSession} */
 class ProtocolSession {
   /**
@@ -14,6 +18,8 @@ class ProtocolSession {
    */
   constructor(session) {
     this._session = session;
+    /** @type {number|undefined} */
+    this._nextProtocolTimeout = undefined;
 
     // FIXME: Monkeypatch puppeteer to be able to listen to *all* protocol events.
     // This patched method will now emit a copy of every event on `*`.
@@ -32,21 +38,21 @@ class ProtocolSession {
    * @return {boolean}
    */
   hasNextProtocolTimeout() {
-    return false;
+    return this._nextProtocolTimeout !== undefined;
   }
 
   /**
    * @return {number}
    */
   getNextProtocolTimeout() {
-    return Number.MAX_SAFE_INTEGER;
+    return this._nextProtocolTimeout || DEFAULT_PROTOCOL_TIMEOUT;
   }
 
   /**
    * @param {number} ms
    */
-  setNextProtocolTimeout(ms) { // eslint-disable-line no-unused-vars
-    // TODO(FR-COMPAT): support protocol timeout
+  setNextProtocolTimeout(ms) {
+    this._nextProtocolTimeout = ms;
   }
 
   /**
@@ -102,7 +108,26 @@ class ProtocolSession {
    * @return {Promise<LH.CrdpCommands[C]['returnType']>}
    */
   sendCommand(method, ...params) {
-    return this._session.send(method, ...params);
+    const timeoutMs = this.getNextProtocolTimeout();
+    this._nextProtocolTimeout = undefined;
+
+    /** @type {NodeJS.Timer|undefined} */
+    let timeout;
+    const timeoutPromise = new Promise((resolve, reject) => {
+      if (timeoutMs === Infinity) return;
+
+      timeout = setTimeout((() => {
+        const err = new LHError(LHError.errors.PROTOCOL_TIMEOUT, {protocolMethod: method});
+        reject(err);
+      }), timeoutMs);
+    });
+
+    const resultPromise = this._session.send(method, ...params);
+    const resultWithTimeoutPromise = Promise.race([resultPromise, timeoutPromise]);
+
+    return resultWithTimeoutPromise.finally(() => {
+      if (timeout) clearTimeout(timeout);
+    });
   }
 }
 


Test patch:
diff --git a/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js b/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
index f33eed2f8..c2d7da9db 100644
--- a/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
+++ b/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
@@ -181,7 +181,7 @@ async function begin() {
   const invertMatch = argv.invertMatch;
   const testDefns = getDefinitionsToRun(allTestDefns, requestedTestIds, {invertMatch});
 
-  let isPassing;
+  let smokehouseResult;
   let server;
   let serverForOffline;
   let takeNetworkRequestUrls = undefined;
@@ -205,13 +205,19 @@ async function begin() {
       takeNetworkRequestUrls,
     };
 
-    isPassing = (await runSmokehouse(prunedTestDefns, options)).success;
+    smokehouseResult = (await runSmokehouse(prunedTestDefns, options));
   } finally {
     if (server) await server.close();
     if (serverForOffline) await serverForOffline.close();
   }
 
-  const exitCode = isPassing ? 0 : 1;
+  if (!smokehouseResult.success) {
+    const failedTestResults = smokehouseResult.testResults.filter(r => r.failed);
+    const cmd = `yarn smoke ${failedTestResults.map(r => r.id).join(' ')}`;
+    console.log(`rerun failures: ${cmd}`);
+  }
+
+  const exitCode = smokehouseResult.success ? 0 : 1;
   process.exit(exitCode);
 }
 
diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js
index 0a05be625..b32a64cde 100644
--- a/lighthouse-core/test/config/config-test.js
+++ b/lighthouse-core/test/config/config-test.js
@@ -1242,7 +1242,7 @@ describe('Config', () => {
       assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
     });
 
-    it('should keep uncategorized audits even if onlyCategories is set', () => {
+    it('should keep full-page-screenshot even if onlyCategories is set', () => {
       assert.ok(origConfig.audits.includes('full-page-screenshot'));
       // full-page-screenshot does not belong to a category.
       const matchCategories = Object.values(origConfig.categories).filter(cat =>
@@ -1259,6 +1259,17 @@ describe('Config', () => {
 
       assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
     });
+
+    it('should keep full-page-screenshot even if skipAudits is set', () => {
+      const extended = {
+        extends: 'lighthouse:default',
+        settings: {
+          skipAudits: ['font-size'],
+        },
+      };
+      const config = new Config(extended);
+      assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
+    });
   });
 
   describe('#requireAudits', () => {
diff --git a/lighthouse-core/test/fraggle-rock/gather/session-test.js b/lighthouse-core/test/fraggle-rock/gather/session-test.js
index f04d4ec62..ed3e198dd 100644
--- a/lighthouse-core/test/fraggle-rock/gather/session-test.js
+++ b/lighthouse-core/test/fraggle-rock/gather/session-test.js
@@ -7,10 +7,19 @@
 
 const {EventEmitter} = require('events');
 const ProtocolSession = require('../../../fraggle-rock/gather/session.js');
+const {
+  flushAllTimersAndMicrotasks,
+  makePromiseInspectable,
+  createDecomposedPromise,
+} = require('../../test-utils.js');
 
 /* eslint-env jest */
 
+jest.useFakeTimers();
+
 describe('ProtocolSession', () => {
+  const DEFAULT_TIMEOUT = 30_000;
+
   /** @type {import('puppeteer').CDPSession} */
   let puppeteerSession;
   /** @type {ProtocolSession} */
@@ -18,7 +27,7 @@ describe('ProtocolSession', () => {
 
   beforeEach(() => {
     // @ts-expect-error - Individual mock functions are applied as necessary.
-    puppeteerSession = {emit: jest.fn()};
+    puppeteerSession = {emit: jest.fn(), send: jest.fn().mockResolvedValue()};
     session = new ProtocolSession(puppeteerSession);
   });
 
@@ -126,5 +135,92 @@ describe('ProtocolSession', () => {
       expect(result).toEqual(123);
       expect(send).toHaveBeenCalledWith('Page.navigate', {url: 'foo'});
     });
+
+    it('times out a request by default', async () => {
+      const sendPromise = createDecomposedPromise();
+      puppeteerSession.send = jest.fn().mockReturnValue(sendPromise.promise);
+
+      const resultPromise = makePromiseInspectable(session.sendCommand('Page.navigate', {url: ''}));
+
+      await jest.advanceTimersByTime(DEFAULT_TIMEOUT + 1);
+      await flushAllTimersAndMicrotasks();
+
+      expect(resultPromise).toBeDone();
+      await expect(resultPromise).rejects.toMatchObject({
+        code: 'PROTOCOL_TIMEOUT',
+        protocolMethod: 'Page.navigate',
+      });
+    });
+
+    it('times out a request with explicit timeout', async () => {
+      const sendPromise = createDecomposedPromise();
+      puppeteerSession.send = jest.fn().mockReturnValue(sendPromise.promise);
+
+      session.setNextProtocolTimeout(60_000);
+      const resultPromise = makePromiseInspectable(session.sendCommand('Page.navigate', {url: ''}));
+
+      await jest.advanceTimersByTime(DEFAULT_TIMEOUT + 1);
+      await flushAllTimersAndMicrotasks();
+
+      expect(resultPromise).not.toBeDone();
+
+      await jest.advanceTimersByTime(DEFAULT_TIMEOUT + 1);
+      await flushAllTimersAndMicrotasks();
+
+      expect(resultPromise).toBeDone();
+      await expect(resultPromise).rejects.toMatchObject({
+        code: 'PROTOCOL_TIMEOUT',
+        protocolMethod: 'Page.navigate',
+      });
+    });
+
+    it('respects a timeout of infinity', async () => {
+      const sendPromise = createDecomposedPromise();
+      puppeteerSession.send = jest.fn().mockReturnValue(sendPromise.promise);
+
+      session.setNextProtocolTimeout(Infinity);
+      const resultPromise = makePromiseInspectable(session.sendCommand('Page.navigate', {url: ''}));
+
+      await jest.advanceTimersByTime(100_000);
+      await flushAllTimersAndMicrotasks();
+
+      expect(resultPromise).not.toBeDone();
+
+      sendPromise.resolve('result');
+      await flushAllTimersAndMicrotasks();
+
+      expect(resultPromise).toBeDone();
+      expect(await resultPromise).toBe('result');
+    });
+  });
+
+  describe('.has/get/setNextProtocolTimeout', () => {
+    it('should handle when none has been set', () => {
+      expect(session.hasNextProtocolTimeout()).toBe(false);
+      expect(session.getNextProtocolTimeout()).toBe(DEFAULT_TIMEOUT);
+    });
+
+    it('should handle when one has been set', () => {
+      session.setNextProtocolTimeout(5_000);
+      expect(session.hasNextProtocolTimeout()).toBe(true);
+      expect(session.getNextProtocolTimeout()).toBe(5_000);
+    });
+
+    it('should handle when default has been explicitly set', () => {
+      session.setNextProtocolTimeout(DEFAULT_TIMEOUT);
+      expect(session.hasNextProtocolTimeout()).toBe(true);
+      expect(session.getNextProtocolTimeout()).toBe(DEFAULT_TIMEOUT);
+    });
+
+    it('should handle result after a command', () => {
+      session.setNextProtocolTimeout(10_000);
+      expect(session.hasNextProtocolTimeout()).toBe(true);
+      expect(session.getNextProtocolTimeout()).toBe(10_000);
+
+      session.sendCommand('Page.navigate', {url: ''});
+
+      expect(session.hasNextProtocolTimeout()).toBe(false);
+      expect(session.getNextProtocolTimeout()).toBe(DEFAULT_TIMEOUT);
+    });
   });
 });
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 4605e3587..4e475b14d 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -1044,13 +1044,26 @@ describe('GatherRunner', function() {
         'warning0',
         'warning1',
         'warning2',
+        {
+          i18nId: 'LH.IcuMessage0',
+          formattedDefault: 'warning3',
+        },
+        {
+          i18nId: 'LH.IcuMessage1',
+          formattedDefault: 'warning4',
+        },
       ];
 
       class WarningGatherer extends Gatherer {
         /** @param {LH.Gatherer.PassContext} passContext */
         afterPass(passContext) {
-          passContext.LighthouseRunWarnings.push(...runWarnings, ...runWarnings);
-          assert.strictEqual(passContext.LighthouseRunWarnings.length, runWarnings.length * 2);
+          passContext.LighthouseRunWarnings.push(
+            ...runWarnings,
+            ...runWarnings,
+            // adding one of the warnings with keys in different order
+            {formattedDefault: 'warning3', i18nId: 'LH.IcuMessage0'}
+          );
+          assert.strictEqual(passContext.LighthouseRunWarnings.length, runWarnings.length * 2 + 1);
 
           return '';
         }