Fix: Basic chrome-flags parsing for embedded quotes (#2754)

* 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

Author: jeremywiebe

lighthouse-cli/run.ts

@@ -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;
@@ -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() {

lighthouse-cli/test/cli/run-test.js

@@ -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': {
@@ -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"')
+    );
+  });
+});