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.

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

feat: add waitForInspectableTarget option (fix #145) #191

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ npm install chrome-launcher
// Default: 50
maxConnectionRetries: number;

// (optional) Interval in ms, which defines whether and how long an inspectable target should be awaited.
// `0` means that list of inspectable targets will not be requested and awaited.
// Default: 0
waitForInspectableTarget: number;
Copy link
Collaborator

@connorjclark connorjclark Mar 14, 2022

Choose a reason for hiding this comment

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

Wondering if this ought to be the default behavior. about to do a major version anyhow, so we can do a breaking change. Will play with this more later.

also, having a timeout be configurable doesn't make much sense to a user (what is a "correct" value?), should probably just internally wait for some reasonable upper limit (10s?) then fail.


// (optional) A dict of environmental key value pairs to pass to the spawned chrome process.
envVars: {[key: string]: string};
};
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"is-wsl": "^2.1.0",
"lighthouse-logger": "^1.0.0",
"mkdirp": "0.5.1",
"phin": "^3.4.1",
"povtor": "^1.1.0",
"rimraf": "^2.6.1"
},
"version": "0.13.0",
Expand Down
42 changes: 40 additions & 2 deletions src/chrome-launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import * as childProcess from 'child_process';
import * as fs from 'fs';
import * as net from 'net';
import * as phin from 'phin';
import {retry} from 'povtor';
import * as rimraf from 'rimraf';
import * as chromeFinder from './chrome-finder';
import {getRandomPort} from './random-port';
Expand Down Expand Up @@ -40,6 +42,7 @@ export interface Options {
ignoreDefaultFlags?: boolean;
connectionPollInterval?: number;
maxConnectionRetries?: number;
waitForInspectableTarget?: number;
envVars?: {[key: string]: string|undefined};
}

Expand Down Expand Up @@ -112,6 +115,7 @@ class Launcher {
private requestedPort?: number;
private connectionPollInterval: number;
private maxConnectionRetries: number;
private waitForInspectableTarget: number;
private fs: typeof fs;
private rimraf: RimrafModule;
private spawn: typeof childProcess.spawn;
Expand All @@ -122,6 +126,7 @@ class Launcher {
userDataDir?: string;
port?: number;
pid?: number;
getTargetRetryTimeout: number = 500;

constructor(private opts: Options = {}, moduleOverrides: ModuleOverrides = {}) {
this.fs = moduleOverrides.fs || fs;
Expand All @@ -138,6 +143,7 @@ class Launcher {
this.ignoreDefaultFlags = defaults(this.opts.ignoreDefaultFlags, false);
this.connectionPollInterval = defaults(this.opts.connectionPollInterval, 500);
this.maxConnectionRetries = defaults(this.opts.maxConnectionRetries, 50);
this.waitForInspectableTarget = defaults(this.opts.waitForInspectableTarget, 0);
this.envVars = defaults(opts.envVars, Object.assign({}, process.env));

if (typeof this.opts.userDataDir === 'boolean') {
Expand Down Expand Up @@ -278,8 +284,26 @@ class Launcher {
}
}

getTargetList() {
return phin({
url: `http://127.0.0.1:${this.port}/json/list`,
parse: 'json'
});
}

waitForTarget() {
return retry({
action: this.getTargetList,
actionContext: this,
retryOnError: true,
retryTest: (response: phin.IResponse) => !response || !Array.isArray(response.body) || !response.body.length,
retryTimeout: this.getTargetRetryTimeout,
timeLimit: this.waitForInspectableTarget
}).promise;
}

// resolves if ready, rejects otherwise
private isDebuggerReady(): Promise<{}> {
isDebuggerReady(): Promise<{}> {
return new Promise((resolve, reject) => {
const client = net.createConnection(this.port!);
client.once('error', err => {
Expand Down Expand Up @@ -312,7 +336,21 @@ class Launcher {
launcher.isDebuggerReady()
.then(() => {
log.log('ChromeLauncher', waitStatus + `${log.greenify(log.tick)}`);
resolve();
if (launcher.waitForInspectableTarget > 0) {
log.log('ChromeLauncher', 'Waiting for an inspectable target...');
launcher.waitForTarget()
.then((response: phin.IResponse) => {
log.log('ChromeLauncher', 'Received target list: %O', response.body);
resolve(response.body);
})
.catch((reason: unknown) => {
log.error('ChromeLauncher', `Cannot get target list. Reason: ${reason}`);
reject(reason);
});
}
else {
resolve();
}
})
.catch(err => {
if (retries > launcher.maxConnectionRetries) {
Expand Down
122 changes: 122 additions & 0 deletions test/chrome-launcher-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,126 @@ describe('Launcher', () => {
const chromeInstance = new Launcher({chromePath: ''});
chromeInstance.launch().catch(() => done());
});

describe('waitForTarget method', () => {
function getChromeInstance(targetList: unknown, waitForInspectableTarget?: number) {
const chromeInstance = new Launcher({waitForInspectableTarget: typeof waitForInspectableTarget === 'number' ? waitForInspectableTarget : 100});
const getTargetListStub = stub(chromeInstance, 'getTargetList').returns(Promise.resolve(targetList));

return {
chromeInstance,
getTargetListStub
};
}

it('returns promise that is resolved with the same value as promise returned by getTargetList method', () => {
const response = {
body: ['test', 'list']
};
const {chromeInstance, getTargetListStub} = getChromeInstance(response);

return chromeInstance.waitForTarget().then((result) => {
assert.ok(getTargetListStub.calledOnce);
assert.strictEqual(result, response);
});
});

it('returns promise that is resolved with the same value as promise returned by getTargetList method after interval specified in waitForInspectableTarget option', () => {
const response = {
body: []
};
const waitTime = 90;
const {chromeInstance, getTargetListStub} = getChromeInstance(response, waitTime);
chromeInstance.getTargetRetryTimeout = 50;
const startTime = new Date().getTime();

return chromeInstance.waitForTarget().then((result) => {
assert.ok(getTargetListStub.callCount === 3);
assert.ok(new Date().getTime() - startTime > waitTime);
assert.strictEqual(result, response);
});
});

it('returns promise that is rejected with the same value as promise returned by getTargetList method after interval specified in waitForInspectableTarget option', () => {
const reason = 'No target';
const waitTime = 100;
const {chromeInstance, getTargetListStub} = getChromeInstance(Promise.reject(reason), waitTime);
chromeInstance.getTargetRetryTimeout = 40;
const startTime = new Date().getTime();

return chromeInstance.waitForTarget().catch((result) => {
assert.ok(getTargetListStub.callCount === 4);
assert.ok(new Date().getTime() - startTime > waitTime);
assert.strictEqual(result, reason);
});
});
});

describe('waitForInspectableTarget option', () => {
function getChromeInstance(options?: Options) {
const chromeInstance = new Launcher(options);
stub(chromeInstance, 'isDebuggerReady').returns(Promise.resolve());

return chromeInstance;
}

it('waitUntilReady does not call waitForTarget method when the option is not set', () => {
const chromeInstance = getChromeInstance();
const waitForTargetSpy = spy(chromeInstance, 'waitForTarget');

return chromeInstance.waitUntilReady().then(() => {
assert.ok(waitForTargetSpy.notCalled);
});
});

it('waitUntilReady does not call waitForTarget method when 0 is set for the option', () => {
const chromeInstance = getChromeInstance({waitForInspectableTarget: 0});
const waitForTargetSpy = spy(chromeInstance, 'waitForTarget');

return chromeInstance.waitUntilReady().then(() => {
assert.ok(waitForTargetSpy.notCalled);
});
});

it('waitUntilReady does not call waitForTarget method when negative value is set for the option', () => {
const chromeInstance = getChromeInstance({waitForInspectableTarget: -1});
const waitForTargetSpy = spy(chromeInstance, 'waitForTarget');

return chromeInstance.waitUntilReady().then(() => {
assert.ok(waitForTargetSpy.notCalled);
});
});

it('waitUntilReady calls waitForTarget method when the option is set', () => {
const chromeInstance = getChromeInstance({waitForInspectableTarget: 1000});
const response = {
body: [{
description: '',
devtoolsFrontendUrl: '/devtools/inspector.html?ws=127.0.0.1:54321/devtools/page/1C2C62A45591F2DECB9CC50E7C3B1FA5',
id: '1C2C62A45591F2DECB9CC50E7C3B1FA5',
title: '',
type: 'page',
url: 'about:blank',
webSocketDebuggerUrl: 'ws://127.0.0.1:54321/devtools/page/1C2C62A45591F2DECB9CC50E7C3B1FA5'
}]
};
const waitForTargetStub = stub(chromeInstance, 'waitForTarget').returns(Promise.resolve(response));

return chromeInstance.waitUntilReady().then((result) => {
assert.ok(waitForTargetStub.calledOnce);
assert.deepEqual(result, response.body);
});
});

it('waitUntilReady rejects when waitForTarget method returns rejected promise', () => {
const chromeInstance = getChromeInstance({waitForInspectableTarget: 1});
const reason = 'No targets';
const waitForTargetStub = stub(chromeInstance, 'waitForTarget').returns(Promise.reject(reason));

return chromeInstance.waitUntilReady().catch((result) => {
assert.ok(waitForTargetStub.calledOnce);
assert.strictEqual(result, reason);
});
});
});
});
17 changes: 17 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ camelcase@^5.0.0:
resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-5.3.1.tgz#e3c9b31569e106811df242f715725a1f4c494320"
integrity sha512-L28STB170nwWS63UjtlEOE3dldQApaJXZkOI1uMFfzf3rRuPegHaHesyee+YxQ+W6SvRDQV6UrdOdRiR153wJg==

centra@^2.2.1:
version "2.4.0"
resolved "https://registry.yarnpkg.com/centra/-/centra-2.4.0.tgz#53846f97db27705e9f90c46e0f824f6eb697e2d1"
integrity sha512-AWmF3EHNe/noJHviynZOrdnUuQzT5AMgl9nJPXGvnzGXrI2ZvNDrEcdqskc4EtQwt2Q1IggXb0OXy7zZ1Xvvew==

chalk@^2.0.1:
version "2.4.2"
resolved "https://registry.yarnpkg.com/chalk/-/chalk-2.4.2.tgz#cd42541677a54333cf541a49108c1432b44c9424"
Expand Down Expand Up @@ -518,6 +523,18 @@ path-to-regexp@^1.7.0:
dependencies:
isarray "0.0.1"

phin@^3.4.1:
version "3.4.1"
resolved "https://registry.yarnpkg.com/phin/-/phin-3.4.1.tgz#b023d14fa86fc6e4b40b6d0dfd5fe478c9c1bbb8"
integrity sha512-NkBCNRPxeyrgaPlWx4DHTAdca3s2LkvIBiiG6RoSbykcOtW/pA/7rUP/67FPIinvbo7h+HENST/vJ17LdRNUdw==
dependencies:
centra "^2.2.1"

povtor@^1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/povtor/-/povtor-1.1.0.tgz#bebe6618c0bcd0df55bd9f6dd2bebebb4d15c5a5"
integrity sha512-gUhd8L9iC4rSipLzx3mCInjusheig56wDrQLiwi5DH5FuumXJE0fEtvZNuheDqjXgMxARLoCz2erqOaa6Trgiw==

require-directory@^2.1.1:
version "2.1.1"
resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42"
Expand Down