Feedback

GoogleChrome · Oct 14, 2018 · c3e15ca · c3e15ca
1 parent 6f501bc
commit c3e15ca
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 17 deletions.
diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
@@ -106,7 +106,7 @@ class Driver {
    */
   async getBrowserVersion() {
     const version = await this.sendCommand('Browser.getVersion');
-    const match = version.product.match(/\/(\d+)/);
+    const match = version.product.match(/\/(\d+)/); // eg 'Chrome/71.0.3577.0'
     const milestone = match ? parseInt(match[1]) : 0;
     return Object.assign(version, {milestone});
   }
@@ -928,7 +928,7 @@ class Driver {
 
     // In Chrome <71, gotta use the chatty 'toplevel' cat instead of our own.
     // TODO(COMPAT): Once m71 ships to stable, drop this section
-    const milestone = (await this.getBrowserVersion()).milestone; // eg 'Chrome/71.0.3577.0'
+    const milestone = (await this.getBrowserVersion()).milestone;
     if (milestone < 71) {
       const toplevelIndex = traceCategories.indexOf('disabled-by-default-lighthouse');
       traceCategories.splice(toplevelIndex, 1, 'toplevel');

diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
@@ -13,7 +13,7 @@ const Connection = require('../../gather/connections/connection.js');
 const Element = require('../../lib/element.js');
 const assert = require('assert');
 const EventEmitter = require('events').EventEmitter;
-const {browserVersion} = require('./fake-driver');
+const {protocolGetVersionResponse} = require('./fake-driver');
 
 const connection = new Connection();
 const driverStub = new Driver(connection);
@@ -64,7 +64,7 @@ connection.sendCommand = function(command, params) {
 
   switch (command) {
     case 'Browser.getVersion':
-      return Promise.resolve(browserVersion);
+      return Promise.resolve(protocolGetVersionResponse);
     case 'DOM.getDocument':
       return Promise.resolve({root: {nodeId: 249}});
     case 'DOM.querySelector':
@@ -259,22 +259,21 @@ describe('Browser Driver', () => {
     });
   });
 
-  it('will adjust traceCategories based on chrome version', () => {
+  it('will adjust traceCategories based on chrome version', async () => {
     // m70 doesn't have disabled-by-default-lighthouse, so 'toplevel' is used instead.
-    const m70BrowserVersion = Object.assign({}, browserVersion, {
+    const m70ProtocolGetVersionResponse = Object.assign({}, protocolGetVersionResponse, {
       product: 'Chrome/70.0.3577.0',
       // eslint-disable-next-line max-len
       userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3577.0 Safari/537.36',
     });
-    createOnceMethodResponse('Browser.getVersion', m70BrowserVersion);
+    createOnceMethodResponse('Browser.getVersion', m70ProtocolGetVersionResponse);
 
     // eslint-disable-next-line max-len
-    return driverStub.beginTrace().then(() => {
-      const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
-      const categories = traceCmd.params.categories;
-      assert.ok(categories.includes('toplevel'), 'contains old toplevel category');
-      assert.equal(categories.indexOf('disabled-by-default-lighthouse'), -1, 'excludes new cat');
-    });
+    await driverStub.beginTrace();
+    const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
+    const categories = traceCmd.params.categories;
+    assert.ok(categories.includes('toplevel'), 'contains old toplevel category');
+    assert.equal(categories.indexOf('disabled-by-default-lighthouse'), -1, 'excludes new cat');
   });
 
   it('should send the Network.setExtraHTTPHeaders command when there are extra-headers', () => {

diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js
@@ -5,7 +5,8 @@
  */
 'use strict';
 
-const browserVersion = {
+// https://chromedevtools.github.io/devtools-protocol/tot/Browser#method-getVersion
+const protocolGetVersionResponse = {
   protocolVersion: '1.3',
   product: 'Chrome/71.0.3577.0',
   revision: '@fc334a55a70eec12fc77853c53979f81e8496c21',
@@ -16,7 +17,7 @@ const browserVersion = {
 
 const fakeDriver = {
   getBrowserVersion() {
-    return Promise.resolve(browserVersion);
+    return Promise.resolve(Object.assign({}, protocolGetVersionResponse, {milestone: 71}));
   },
   getBenchmarkIndex() {
     return Promise.resolve(125.2);
@@ -83,4 +84,4 @@ const fakeDriver = {
 };
 
 module.exports = fakeDriver;
-module.exports.browserVersion = browserVersion;
+module.exports.protocolGetVersionResponse = protocolGetVersionResponse;
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
@@ -123,7 +123,7 @@ describe('GatherRunner', function() {
     const options = {url, driver, config, settings};
 
     const results = await GatherRunner.run([], options);
-    expect(results.HostUserAgent).toEqual(fakeDriver.browserVersion.userAgent);
+    expect(results.HostUserAgent).toEqual(fakeDriver.protocolGetVersionResponse.userAgent);
     expect(results.HostUserAgent).toMatch(/Chrome\/\d+/);
   });