Skip to content

Commit

Permalink
Fix: Basic chrome-flags parsing for embedded quotes (#2754)
Browse files Browse the repository at this point in the history
* Basic chrome-flags parsing for embedded quotes

* Fix formatting in run.ts

* Use yargs instead of hand-rolled arg parsing

* Fix up formatting

* Shorten up parseChromeFlags thanks to @patrickhulce

* Add unit tests for Chrome flag parsing

* check undefined.

* update comment
  • Loading branch information
jeremywiebe authored and patrickhulce committed Aug 3, 2017
1 parent db3f324 commit 78a8bd7
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 2 deletions.
33 changes: 31 additions & 2 deletions lighthouse-cli/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import * as Printer from './printer';
import {Results} from './types/types';
import {Flags} from './cli-flags';
import {launch, LaunchedChrome} from '../chrome-launcher/chrome-launcher';

const yargs = require('yargs');
const lighthouse = require('../lighthouse-core');
const log = require('lighthouse-logger');
const getFilenamePrefix = require('../lighthouse-core/lib/file-namer.js').getFilenamePrefix;
Expand All @@ -27,13 +29,40 @@ interface LighthouseError extends Error {
code?: string
}

// Boolean values that are true are rendered without a value (--debug vs. --debug=true)
function formatArg(arg: string, value: any): string {
if (value === true) {
return `--${arg}`
}

// Only quote the value if it contains spaces
if (value.indexOf(' ') !== -1) {
value = `"${value}"`
}

return `--${arg}=${value}`
}

// exported for testing
export function parseChromeFlags(flags: string) {
let args = yargs(flags).argv;
const allKeys = Object.keys(args);

// Remove unneeded arguments (yargs includes a _ arg, $-prefixed args, and camelCase dupes)
return allKeys.filter(k => k !== '_' && !k.startsWith('$') && !/[A-Z]/.test(k))
.map(k => formatArg(k, args[k]));
}

/**
* Attempts to connect to an instance of Chrome with an open remote-debugging
* port. If none is found, launches a debuggable instance.
*/
async function getDebuggableChrome(flags: Flags) {
return await launch(
{port: flags.port, chromeFlags: flags.chromeFlags.split(' '), logLevel: flags.logLevel});
return await launch({
port: flags.port,
chromeFlags: parseChromeFlags(flags.chromeFlags),
logLevel: flags.logLevel
});
}

function showConnectionError() {
Expand Down
29 changes: 29 additions & 0 deletions lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const path = require('path');
const fs = require('fs');

const run = require('../../run');
const parseChromeFlags = require('../../run').parseChromeFlags;
const fastConfig = {
'extends': 'lighthouse:default',
'settings': {
Expand Down Expand Up @@ -43,3 +44,31 @@ describe('CLI run', function() {
});
}).timeout(60000);
});

describe('Parsing --chrome-flags', () => {
it('returns boolean flags that are true as a bare flag', () => {
assert.deepStrictEqual(['--debug'], parseChromeFlags('--debug'));
});

it('returns boolean flags that are false with value', () => {
assert.deepStrictEqual(['--debug=false'], parseChromeFlags('--debug=false'));
});

it('returns boolean flags that empty when passed undefined', () => {
assert.deepStrictEqual([], parseChromeFlags());
});

it('returns flags correctly for issue 2817', () => {
assert.deepStrictEqual(
['--host-resolver-rules="MAP www.example.org:443 127.0.0.1:8443"'],
parseChromeFlags('--host-resolver-rules="MAP www.example.org:443 127.0.0.1:8443"')
);
});

it('returns all flags as provided', () => {
assert.deepStrictEqual(
['--spaces="1 2 3 4"', '--debug=false', '--verbose', '--more-spaces="9 9 9"'],
parseChromeFlags('--spaces="1 2 3 4" --debug=false --verbose --more-spaces="9 9 9"')
);
});
});

0 comments on commit 78a8bd7

Please sign in to comment.