From cfd8e3f6a6fcf8b021d3d2c18b173e7bb0add16c Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Sun, 23 Jul 2017 20:09:00 -0400 Subject: [PATCH 1/9] Fulfills GoogleChrome/lighthouse#2727 Decided to make hostname second parameter to avoid breaking existing implementations --- lighthouse-core/gather/connections/cri.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/connections/cri.js b/lighthouse-core/gather/connections/cri.js index b5e674ef22ff..b6e6f9ece9cc 100644 --- a/lighthouse-core/gather/connections/cri.js +++ b/lighthouse-core/gather/connections/cri.js @@ -10,18 +10,20 @@ const WebSocket = require('ws'); const http = require('http'); const log = require('lighthouse-logger'); -const hostname = 'localhost'; +const DEFAULT_HOSTNAME = 'localhost'; const CONNECT_TIMEOUT = 10000; const DEFAULT_PORT = 9222; class CriConnection extends Connection { /** * @param {number=} port Optional port number. Defaults to 9222; + * @param {string} hostname Optional hostname. Defaults to localhost. * @constructor */ - constructor(port) { + constructor(port, hostname) { super(); + this.hostname = hostname || DEFAULT_HOSTNAME; this.port = port || DEFAULT_PORT; } @@ -77,7 +79,7 @@ class CriConnection extends Connection { _runJsonCommand(command) { return new Promise((resolve, reject) => { const request = http.get({ - hostname: hostname, + hostname: this.hostname, port: this.port, path: '/json/' + command }, response => { From eace435359ad79d73fbe661d4377f98a28eb845b Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Sun, 23 Jul 2017 20:11:38 -0400 Subject: [PATCH 2/9] Add fix to pass tests --- lighthouse-viewer/app/src/github-api.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lighthouse-viewer/app/src/github-api.js b/lighthouse-viewer/app/src/github-api.js index c2f366a946bc..a9901073cda1 100644 --- a/lighthouse-viewer/app/src/github-api.js +++ b/lighthouse-viewer/app/src/github-api.js @@ -116,7 +116,8 @@ class GithubApi { const filename = Object.keys(json.files) .find(filename => filename.endsWith(GithubApi.LH_JSON_EXT)); if (!filename) { - throw new Error(`Failed to find a Lighthouse report (*${GithubApi.LH_JSON_EXT}) in gist ${id}`); + throw new Error(`Failed to find a Lighthouse report + (*${GithubApi.LH_JSON_EXT}) in gist ${id}`); } const f = json.files[filename]; if (f.truncated) { From 3db077107ab2c061deb457084d2d6c43473ddd66 Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Mon, 24 Jul 2017 11:27:07 -0400 Subject: [PATCH 3/9] Fix annotation --- lighthouse-core/gather/connections/cri.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/gather/connections/cri.js b/lighthouse-core/gather/connections/cri.js index b6e6f9ece9cc..919f3d346e29 100644 --- a/lighthouse-core/gather/connections/cri.js +++ b/lighthouse-core/gather/connections/cri.js @@ -17,7 +17,7 @@ const DEFAULT_PORT = 9222; class CriConnection extends Connection { /** * @param {number=} port Optional port number. Defaults to 9222; - * @param {string} hostname Optional hostname. Defaults to localhost. + * @param {string=} hostname Optional hostname. Defaults to localhost. * @constructor */ constructor(port, hostname) { From f224ab3be475b4bc7b590c25cae4cfc312393316 Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Mon, 24 Jul 2017 16:53:18 -0400 Subject: [PATCH 4/9] Add programmatic use case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think… --- lighthouse-core/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index a1f90a66e586..f718669e7296 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -35,7 +35,7 @@ module.exports = function(url, flags = {}, configJSON) { // Use ConfigParser to generate a valid config file const config = new Config(configJSON, flags.configPath); - const connection = new ChromeProtocol(flags.port); + const connection = new ChromeProtocol(flags.port, flags.host); // kick off a lighthouse run return Runner.run(connection, {url, flags, config}) From d056543289cca988bcaa7334feba547909e88091 Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Mon, 24 Jul 2017 17:52:57 -0400 Subject: [PATCH 5/9] Add documentation --- docs/readme.md | 1 + lighthouse-cli/cli-flags.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/readme.md b/docs/readme.md index 26449483dcb3..d5eaccce492e 100644 --- a/docs/readme.md +++ b/docs/readme.md @@ -13,6 +13,7 @@ const chromeLauncher = require('lighthouse/chrome-launcher'); function launchChromeAndRunLighthouse(url, flags = {}, config = null) { return chromeLauncher.launch().then(chrome => { + flags.host = 'localhost'; flags.port = chrome.port; return lighthouse(url, flags, config).then(results => chrome.kill().then(() => results)); diff --git a/lighthouse-cli/cli-flags.ts b/lighthouse-cli/cli-flags.ts index 3650a91bbf50..79bd974f201b 100644 --- a/lighthouse-cli/cli-flags.ts +++ b/lighthouse-cli/cli-flags.ts @@ -12,9 +12,9 @@ const Driver = require('../lighthouse-core/gather/driver.js'); import {GetValidOutputOptions, OutputMode} from './printer'; export interface Flags { - port: number, chromeFlags: string, output: any, outputPath: string, interactive: boolean, - saveArtifacts: boolean, saveAssets: boolean, view: boolean, maxWaitForLoad: number, - logLevel: string + port: number, host: string, chromeFlags: string, output: any, outputPath: string, + interactive: boolean, saveArtifacts: boolean, saveAssets: boolean, view: boolean, + maxWaitForLoad: number, logLevel: string } export function getFlags(manualArgv?: string) { @@ -53,7 +53,7 @@ export function getFlags(manualArgv?: string) { .group( [ 'save-assets', 'save-artifacts', 'list-all-audits', 'list-trace-categories', - 'additional-trace-categories', 'config-path', 'chrome-flags', 'perf', 'port', + 'additional-trace-categories', 'config-path', 'chrome-flags', 'perf', 'port', 'host', 'max-wait-for-load' ], 'Configuration:') @@ -77,6 +77,7 @@ export function getFlags(manualArgv?: string) { CHROME_PATH: Explicit path of intended Chrome binary. If set must point to an executable of a build of Chromium version 54.0 or later. By default, any detected Chrome Canary or Chrome (stable) will be launched. `, 'perf': 'Use a performance-test-only configuration', + 'host': 'The host to use for the debugging protocol.', 'port': 'The port to use for the debugging protocol. Use 0 for a random port', 'max-wait-for-load': 'The timeout (in milliseconds) to wait before the page is considered done loading and the run should continue. WARNING: Very high values can lead to large traces and instability', @@ -107,6 +108,7 @@ Example: --output-path=./lighthouse-results.html`, .default('disable-cpu-throttling', false) .default('output', GetValidOutputOptions()[OutputMode.domhtml]) .default('port', 0) + .default('host', 'localhost') .default('max-wait-for-load', Driver.MAX_WAIT_FOR_FULLY_LOADED) .check((argv: {listAllAudits?: boolean, listTraceCategories?: boolean, _: Array}) => { // Make sure lighthouse has been passed a url, or at least one of --list-all-audits From 508b6715f931d939d16ac0e659f1e3a54bcaefb8 Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Mon, 24 Jul 2017 17:55:15 -0400 Subject: [PATCH 6/9] Fix PR conflicts --- lighthouse-viewer/app/src/github-api.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lighthouse-viewer/app/src/github-api.js b/lighthouse-viewer/app/src/github-api.js index a9901073cda1..d8d9b5c05b6d 100644 --- a/lighthouse-viewer/app/src/github-api.js +++ b/lighthouse-viewer/app/src/github-api.js @@ -116,8 +116,9 @@ class GithubApi { const filename = Object.keys(json.files) .find(filename => filename.endsWith(GithubApi.LH_JSON_EXT)); if (!filename) { - throw new Error(`Failed to find a Lighthouse report - (*${GithubApi.LH_JSON_EXT}) in gist ${id}`); + throw new Error( + `Failed to find a Lighthouse report (*${GithubApi.LH_JSON_EXT}) in gist ${id}` + ); } const f = json.files[filename]; if (f.truncated) { From e87d50f407fbe0a4ef4bec14bcbd9097f3993447 Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Wed, 2 Aug 2017 00:46:49 -0400 Subject: [PATCH 7/9] Add ES6 default parameters Also remove comments from docs/readme.md, add comment to readme.md, and make cli-flags.ts change more digestible. --- docs/readme.md | 1 - lighthouse-cli/cli-flags.ts | 4 ++-- lighthouse-core/gather/connections/cri.js | 5 +---- readme.md | 2 ++ 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/readme.md b/docs/readme.md index d5eaccce492e..26449483dcb3 100644 --- a/docs/readme.md +++ b/docs/readme.md @@ -13,7 +13,6 @@ const chromeLauncher = require('lighthouse/chrome-launcher'); function launchChromeAndRunLighthouse(url, flags = {}, config = null) { return chromeLauncher.launch().then(chrome => { - flags.host = 'localhost'; flags.port = chrome.port; return lighthouse(url, flags, config).then(results => chrome.kill().then(() => results)); diff --git a/lighthouse-cli/cli-flags.ts b/lighthouse-cli/cli-flags.ts index 79bd974f201b..2903351aa304 100644 --- a/lighthouse-cli/cli-flags.ts +++ b/lighthouse-cli/cli-flags.ts @@ -12,9 +12,9 @@ const Driver = require('../lighthouse-core/gather/driver.js'); import {GetValidOutputOptions, OutputMode} from './printer'; export interface Flags { - port: number, host: string, chromeFlags: string, output: any, outputPath: string, + port: number, chromeFlags: string, output: any, outputPath: string, interactive: boolean, saveArtifacts: boolean, saveAssets: boolean, view: boolean, - maxWaitForLoad: number, logLevel: string + maxWaitForLoad: number, logLevel: string, host: string } export function getFlags(manualArgv?: string) { diff --git a/lighthouse-core/gather/connections/cri.js b/lighthouse-core/gather/connections/cri.js index 919f3d346e29..950470cb5acf 100644 --- a/lighthouse-core/gather/connections/cri.js +++ b/lighthouse-core/gather/connections/cri.js @@ -20,11 +20,8 @@ class CriConnection extends Connection { * @param {string=} hostname Optional hostname. Defaults to localhost. * @constructor */ - constructor(port, hostname) { + constructor(port = DEFAULT_PORT, hostname = DEFAULT_HOSTNAME) { super(); - - this.hostname = hostname || DEFAULT_HOSTNAME; - this.port = port || DEFAULT_PORT; } /** diff --git a/readme.md b/readme.md index bb7bc533a16f..ac7c1ea4e690 100644 --- a/readme.md +++ b/readme.md @@ -58,6 +58,8 @@ Configuration: http://peter.sh/experiments/chromium-command-line-switches/. [default: ""] --perf Use a performance-test-only configuration [boolean] --port The port to use for the debugging protocol. Use 0 for a random port [default: 9222] + --host The host to use for the debugging protocol. + host [default: localhost] --max-wait-for-load The timeout (in milliseconds) to wait before the page is considered done loading and the run should continue. WARNING: Very high values can lead to large traces and instability [default: 25000] From 284d5a701ff04fa8d4ec92ee1d9cb47c9cbf6946 Mon Sep 17 00:00:00 2001 From: Frederick Morlock Date: Thu, 3 Aug 2017 19:20:17 -0400 Subject: [PATCH 8/9] Fix tests --- lighthouse-cli/cli-flags.ts | 6 +++--- lighthouse-core/gather/connections/cri.js | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lighthouse-cli/cli-flags.ts b/lighthouse-cli/cli-flags.ts index 2903351aa304..40173aa855f3 100644 --- a/lighthouse-cli/cli-flags.ts +++ b/lighthouse-cli/cli-flags.ts @@ -12,9 +12,9 @@ const Driver = require('../lighthouse-core/gather/driver.js'); import {GetValidOutputOptions, OutputMode} from './printer'; export interface Flags { - port: number, chromeFlags: string, output: any, outputPath: string, - interactive: boolean, saveArtifacts: boolean, saveAssets: boolean, view: boolean, - maxWaitForLoad: number, logLevel: string, host: string + port: number, chromeFlags: string, output: any, outputPath: string, interactive: boolean, + saveArtifacts: boolean, saveAssets: boolean, view: boolean, maxWaitForLoad: number, + logLevel: string, host: string } export function getFlags(manualArgv?: string) { diff --git a/lighthouse-core/gather/connections/cri.js b/lighthouse-core/gather/connections/cri.js index 950470cb5acf..6d7403db19b1 100644 --- a/lighthouse-core/gather/connections/cri.js +++ b/lighthouse-core/gather/connections/cri.js @@ -22,6 +22,8 @@ class CriConnection extends Connection { */ constructor(port = DEFAULT_PORT, hostname = DEFAULT_HOSTNAME) { super(); + this.port = port; + this.hostname = hostname; } /** From df95aaa6f9340ddb7e97ecb3d73d2d59b375f2fe Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Mon, 14 Aug 2017 14:16:01 -0700 Subject: [PATCH 9/9] rename host to hostname for spec correctness --- lighthouse-cli/cli-flags.ts | 10 +++++----- lighthouse-core/index.js | 2 +- readme.md | 3 +-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lighthouse-cli/cli-flags.ts b/lighthouse-cli/cli-flags.ts index 40173aa855f3..ed013cb50db0 100644 --- a/lighthouse-cli/cli-flags.ts +++ b/lighthouse-cli/cli-flags.ts @@ -14,7 +14,7 @@ import {GetValidOutputOptions, OutputMode} from './printer'; export interface Flags { port: number, chromeFlags: string, output: any, outputPath: string, interactive: boolean, saveArtifacts: boolean, saveAssets: boolean, view: boolean, maxWaitForLoad: number, - logLevel: string, host: string + logLevel: string, hostname: string } export function getFlags(manualArgv?: string) { @@ -53,8 +53,8 @@ export function getFlags(manualArgv?: string) { .group( [ 'save-assets', 'save-artifacts', 'list-all-audits', 'list-trace-categories', - 'additional-trace-categories', 'config-path', 'chrome-flags', 'perf', 'port', 'host', - 'max-wait-for-load' + 'additional-trace-categories', 'config-path', 'chrome-flags', 'perf', 'port', + 'hostname', 'max-wait-for-load' ], 'Configuration:') .describe({ @@ -77,7 +77,7 @@ export function getFlags(manualArgv?: string) { CHROME_PATH: Explicit path of intended Chrome binary. If set must point to an executable of a build of Chromium version 54.0 or later. By default, any detected Chrome Canary or Chrome (stable) will be launched. `, 'perf': 'Use a performance-test-only configuration', - 'host': 'The host to use for the debugging protocol.', + 'hostname': 'The hostname to use for the debugging protocol.', 'port': 'The port to use for the debugging protocol. Use 0 for a random port', 'max-wait-for-load': 'The timeout (in milliseconds) to wait before the page is considered done loading and the run should continue. WARNING: Very high values can lead to large traces and instability', @@ -108,7 +108,7 @@ Example: --output-path=./lighthouse-results.html`, .default('disable-cpu-throttling', false) .default('output', GetValidOutputOptions()[OutputMode.domhtml]) .default('port', 0) - .default('host', 'localhost') + .default('hostname', 'localhost') .default('max-wait-for-load', Driver.MAX_WAIT_FOR_FULLY_LOADED) .check((argv: {listAllAudits?: boolean, listTraceCategories?: boolean, _: Array}) => { // Make sure lighthouse has been passed a url, or at least one of --list-all-audits diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index f718669e7296..0f2b6607fabb 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -35,7 +35,7 @@ module.exports = function(url, flags = {}, configJSON) { // Use ConfigParser to generate a valid config file const config = new Config(configJSON, flags.configPath); - const connection = new ChromeProtocol(flags.port, flags.host); + const connection = new ChromeProtocol(flags.port, flags.hostname); // kick off a lighthouse run return Runner.run(connection, {url, flags, config}) diff --git a/readme.md b/readme.md index ac7c1ea4e690..3674db50b874 100644 --- a/readme.md +++ b/readme.md @@ -58,8 +58,7 @@ Configuration: http://peter.sh/experiments/chromium-command-line-switches/. [default: ""] --perf Use a performance-test-only configuration [boolean] --port The port to use for the debugging protocol. Use 0 for a random port [default: 9222] - --host The host to use for the debugging protocol. - host [default: localhost] + --hostname The hostname to use for the debugging protocol. [default: localhost] --max-wait-for-load The timeout (in milliseconds) to wait before the page is considered done loading and the run should continue. WARNING: Very high values can lead to large traces and instability [default: 25000]