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.

Sign up for GitHub

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

Jump to bottom

feat: configurable page timeout #1672

Merged
merged 6 commits into from
Feb 16, 2017
Merged

feat: configurable page timeout #1672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9da9013
feat: configurable page timeout
patrickhulce Feb 9, 2017
e47b102
feedback
patrickhulce Feb 9, 2017
b87ef4b
more feedback
patrickhulce Feb 13, 2017
4012ca7
update max-wait-for-load description
patrickhulce Feb 14, 2017
a5b1cf2
update readme text
patrickhulce Feb 15, 2017
67a85dc
bump yargs
patrickhulce Feb 16, 2017
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
15 changes: 7 additions & 8 deletions lighthouse-cli/bin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {ChromeLauncher} from './chrome-launcher';
import * as Commands from './commands/commands';
const lighthouse = require('../lighthouse-core');
const log = require('../lighthouse-core/lib/log');
const Driver = require('../lighthouse-core/gather/driver.js');
import * as path from 'path';
const perfOnlyConfig = require('../lighthouse-core/config/perf.json');
const performanceXServer = require('./performance-experiment/server');
Expand Down Expand Up @@ -62,7 +63,8 @@ const cliFlags = yargs
'list-trace-categories',
'config-path',
'perf',
'port'
'port',
'max-wait-for-load'
], 'Configuration:')
.describe({
'disable-device-emulation': 'Disable Nexus 5X emulation',
Expand All @@ -75,6 +77,7 @@ const cliFlags = yargs
'config-path': 'The path to the config JSON.',
'perf': 'Use a performance-test-only configuration',
'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',
'skip-autolaunch': 'Skip autolaunch of Chrome when already running instance is not found',
'select-chrome': 'Interactively choose version of Chrome to use when multiple installations are found',
'interactive': 'Open Lighthouse in interactive mode'
Expand Down Expand Up @@ -114,6 +117,7 @@ Example: --output-path=./lighthouse-results.html`
.default('output', Printer.GetValidOutputOptions()[Printer.OutputMode.pretty])
.default('output-path', 'stdout')
.default('port', 9222)
.default('max-wait-for-load', Driver.MAX_WAIT_FOR_FULLY_LOADED)
.check((argv: {listAllAudits?: boolean, listTraceCategories?: boolean, _: Array<any>}) => {
// Make sure lighthouse has been passed a url, or at least one of --list-all-audits
// or --list-trace-categories. If not, stop the program and ask for a url
Expand All @@ -137,12 +141,6 @@ if (cliFlags.listTraceCategories) {

const url = cliFlags._[0];

// Work around camelCase bug for default value in yargs 3.30.
// see: https://github.com/yargs/yargs/issues/341
if (!cliFlags.outputPath && cliFlags['output-path']) {
cliFlags.outputPath = cliFlags['output-path'];
}

let config: Object | null = null;
if (cliFlags.configPath) {
// Resolve the config file path relative to where cli was called.
Expand Down Expand Up @@ -278,7 +276,8 @@ function saveResults(results: Results,

function runLighthouse(url: string,
flags: {port: number, skipAutolaunch: boolean, selectChrome: boolean, output: any,
outputPath: string, interactive: boolean, saveArtifacts: boolean, saveAssets: boolean},
outputPath: string, interactive: boolean, saveArtifacts: boolean, saveAssets: boolean
maxWaitForLoad: number},
config: Object): Promise<undefined> {

let chromeLauncher: ChromeLauncher;
Expand Down
26 changes: 15 additions & 11 deletions lighthouse-core/gather/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ const URL = require('../lib/url-shim');
const log = require('../lib/log.js');
const DevtoolsLog = require('./devtools-log');

const MAX_WAIT_FOR_FULLY_LOADED = 25 * 1000;
const PAUSE_AFTER_LOAD = 500;

class Driver {
static get MAX_WAIT_FOR_FULLY_LOADED() {
return 25 * 1000;
}

/**
* @param {!Connection} connection
Expand Down Expand Up @@ -417,13 +419,14 @@ class Driver {
* Returns a promise that resolves when:
* - it's been pauseAfterLoadMs milliseconds after both onload and the network
* has gone idle, or
* - MAX_WAIT_FOR_FULLY_LOADED milliseconds have passed.
* - maxWaitForLoadedMs milliseconds have passed.
* See https://github.com/GoogleChrome/lighthouse/issues/627 for more.
* @param {number} pauseAfterLoadMs
* @param {number} maxWaitForLoadedMs
* @return {!Promise}
* @private
*/
_waitForFullyLoaded(pauseAfterLoadMs) {
_waitForFullyLoaded(pauseAfterLoadMs, maxWaitForLoadedMs) {
let maxTimeoutHandle;

// Listener for onload. Resolves pauseAfterLoadMs ms after load.
Expand All @@ -443,10 +446,10 @@ class Driver {
};
});

// Last resort timeout. Resolves MAX_WAIT_FOR_FULLY_LOADED ms from now on
// Last resort timeout. Resolves maxWaitForLoadedMs ms from now on
// cleanup function that removes loadEvent and network idle listeners.
const maxTimeoutPromise = new Promise((resolve, reject) => {
maxTimeoutHandle = setTimeout(resolve, MAX_WAIT_FOR_FULLY_LOADED);
maxTimeoutHandle = setTimeout(resolve, maxWaitForLoadedMs);
}).then(_ => {
return function() {
log.warn('Driver', 'Timed out waiting for page load. Moving on...');
Expand All @@ -472,16 +475,17 @@ class Driver {
* @param {!Object} options
* @return {!Promise}
*/
gotoURL(url, options) {
const _options = options || {};
const waitForLoad = _options.waitForLoad || false;
const disableJS = _options.disableJavaScript || false;
const pauseAfterLoadMs = (_options.flags && _options.flags.pauseAfterLoad) || PAUSE_AFTER_LOAD;
gotoURL(url, options = {}) {
const waitForLoad = options.waitForLoad || false;
const disableJS = options.disableJavaScript || false;
const pauseAfterLoadMs = (options.flags && options.flags.pauseAfterLoad) || PAUSE_AFTER_LOAD;
const maxWaitMs = (options.flags && options.flags.maxWaitForLoad) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we support --max-wait-for-load = 0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that would lead to a useless lighthouse run that wouldn't return any results so filling in default is fine with me :)

Driver.MAX_WAIT_FOR_FULLY_LOADED;

return this.sendCommand('Page.enable')
.then(_ => this.sendCommand('Emulation.setScriptExecutionDisabled', {value: disableJS}))
.then(_ => this.sendCommand('Page.navigate', {url}))
.then(_ => waitForLoad && this._waitForFullyLoaded(pauseAfterLoadMs));
.then(_ => waitForLoad && this._waitForFullyLoaded(pauseAfterLoadMs, maxWaitMs));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"speedline": "1.0.3",
"whatwg-url": "4.0.0",
"ws": "1.1.1",
"yargs": "3.30.0"
"yargs": "3.32.0"
},
"repository": "googlechrome/lighthouse",
"keywords": [
Expand Down
8 changes: 7 additions & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ Configuration:
--list-trace-categories Prints a list of all required trace categories and exits [boolean]
--config-path The path to the config JSON.
--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]
--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]

Output:
--output Reporter for the results
Expand Down Expand Up @@ -176,7 +182,7 @@ Options:
* `./report.json`
* `./report.artifacts.log`

`--save-artifacts` prints a pretty report to `stdout` **and** generates
`--save-artifacts` prints a pretty report to `stdout` **and** generates
* `./<HOST>_<DATE>.report.html`
* `./<HOST>_<DATE>.artifacts.log`

Expand Down
43 changes: 17 additions & 26 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,11 +361,11 @@ camelcase-keys@^2.0.0:
camelcase "^2.0.0"
map-obj "^1.0.0"

camelcase@^1.0.2, camelcase@^1.2.1:
camelcase@^1.0.2:
version "1.2.1"
resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-1.2.1.tgz#9bb5304d2e0b56698b2c758b08a3eaa9daa58a39"

camelcase@^2.0.0:
camelcase@^2.0.0, camelcase@^2.0.1:
version "2.1.1"
resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-2.1.1.tgz#7c1d16d679a1bbe59ca02cacecfb011e201f5a1f"

Expand Down Expand Up @@ -689,14 +689,10 @@ es6-weak-map@^2.0.1:
es6-iterator "2"
es6-symbol "3"

escape-string-regexp@1.0.5, escape-string-regexp@^1.0.3, escape-string-regexp@^1.0.5:
escape-string-regexp@1.0.5, escape-string-regexp@^1.0.2, escape-string-regexp@^1.0.3, escape-string-regexp@^1.0.5:
version "1.0.5"
resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4"

escape-string-regexp@^1.0.2:
version "1.0.2"
resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.2.tgz#4dbc2fe674e71949caf3fb2695ce7f2dc1d9a8d1"

escodegen@1.8.x, escodegen@^1.6.1:
version "1.8.1"
resolved "https://registry.yarnpkg.com/escodegen/-/escodegen-1.8.1.tgz#5a5b53af4693110bebb0867aa3430dd3b70a1018"
Expand Down Expand Up @@ -1055,7 +1051,7 @@ glob@^5.0.15:
once "^1.3.0"
path-is-absolute "^1.0.0"

glob@^7.0.0, glob@^7.0.3, glob@^7.0.5:
glob@^7.0.0, glob@^7.0.3:
version "7.1.1"
resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.1.tgz#805211df04faaf1c63a3600306cdf5ade50b2ec8"
dependencies:
Expand Down Expand Up @@ -2388,16 +2384,10 @@ right-align@^0.1.1:
dependencies:
align-text "^0.1.1"

rimraf@2.2.8:
rimraf@2.2.8, rimraf@^2.2.8:
version "2.2.8"
resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.2.8.tgz#e439be2aaee327321952730f99a8929e4fc50582"

rimraf@^2.2.8:
version "2.5.4"
resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.5.4.tgz#96800093cbf1a0c86bd95b4625467535c29dfa04"
dependencies:
glob "^7.0.5"

run-async@^0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/run-async/-/run-async-0.1.0.tgz#c8ad4a5e110661e402a7d21b530e009f25f8e389"
Expand All @@ -2412,14 +2402,14 @@ sax@^1.1.4:
version "1.2.1"
resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.1.tgz#7b8e656190b228e81a66aea748480d828cd2d37a"

"semver@2 || 3 || 4 || 5", semver@^4.1.0:
version "4.3.6"
resolved "https://registry.yarnpkg.com/semver/-/semver-4.3.6.tgz#300bc6e0e86374f7ba61068b5b1ecd57fc6532da"

semver@5.3.0:
"semver@2 || 3 || 4 || 5", semver@5.3.0:
version "5.3.0"
resolved "https://registry.yarnpkg.com/semver/-/semver-5.3.0.tgz#9b2ce5d3de02d17c6012ad326aa6b4d0cf54f94f"

semver@^4.1.0:
version "4.3.6"
resolved "https://registry.yarnpkg.com/semver/-/semver-4.3.6.tgz#300bc6e0e86374f7ba61068b5b1ecd57fc6532da"

sequencify@~0.0.7:
version "0.0.7"
resolved "https://registry.yarnpkg.com/sequencify/-/sequencify-0.0.7.tgz#90cff19d02e07027fd767f5ead3e7b95d1e7380c"
Expand Down Expand Up @@ -2813,7 +2803,7 @@ window-size@0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/window-size/-/window-size-0.1.0.tgz#5438cd2ea93b202efa3a19fe8887aee7c94f9c9d"

window-size@^0.1.2:
window-size@^0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/window-size/-/window-size-0.1.4.tgz#f8e1aa1ee5a53ec5bf151ffa09742a6ad7697876"

Expand Down Expand Up @@ -2864,15 +2854,16 @@ y18n@^3.2.0:
version "3.2.1"
resolved "https://registry.yarnpkg.com/y18n/-/y18n-3.2.1.tgz#6d15fba884c08679c0d77e88e7759e811e07fa41"

yargs@3.30.0:
version "3.30.0"
resolved "https://registry.yarnpkg.com/yargs/-/yargs-3.30.0.tgz#5ec82ef8c7296fe02fa0ee37863f6d9d9e24acef"
yargs@3.32.0:
version "3.32.0"
resolved "https://registry.yarnpkg.com/yargs/-/yargs-3.32.0.tgz#03088e9ebf9e756b69751611d2a5ef591482c995"
dependencies:
camelcase "^1.2.1"
camelcase "^2.0.1"
cliui "^3.0.3"
decamelize "^1.1.1"
os-locale "^1.4.0"
window-size "^0.1.2"
string-width "^1.0.1"
window-size "^0.1.4"
y18n "^3.2.0"

yargs@~3.10.0:
Expand Down