Passing in `--settings` as a string to CLI

[niieani]

Describe the bug Hi! 👋

Passing in --settings to CLI is broken. The string is never parsed, so the code ends up trying to create properties on the string, which obviously fails.

To Reproduce

$ lhci collect --url https://my-url.com --numberOfRuns 1 --settings '{"extraHeaders": {"Cookie": "some-cookie-string"}}'

Running Lighthouse 1 time(s) on https://my-url.com
Run #1...failed!
TypeError: Cannot create property 'chromeFlags' on string '{"extraHeaders": {"Cookie": "some-cookie-string"}}'
    at Function.computeArgumentsAndCleanup (/@lhci/cli/src/collect/node-runner.js:49:51)
    at LighthouseRunner.run (/@lhci/cli/src/collect/node-runner.js:97:48)
    at LighthouseRunner.runUntilSuccess (/@lhci/cli/src/collect/node-runner.js:140:27)
    at runOnUrl (/@lhci/cli/src/collect/collect.js:120:32)
    at Object.runCommand (/@lhci/cli/src/collect/collect.js:241:13)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

error Command failed with exit code 1.

Expected behavior

--settings flag is supposed to work as designed :)

Environment (please complete the following information):

Node 14

Additional context

Here is the diff that solved my problem:

diff --git a/node_modules/@lhci/cli/src/collect/collect.js b/node_modules/@lhci/cli/src/collect/collect.js
index 2ef6ec7..ed2008c 100644
--- a/node_modules/@lhci/cli/src/collect/collect.js
+++ b/node_modules/@lhci/cli/src/collect/collect.js
@@ -109,7 +109,7 @@ async function runOnUrl(url, options, context) {
   const runner = getRunner(options);
   process.stdout.write(`Running Lighthouse ${options.numberOfRuns} time(s) on ${url}\n`);
 
-  const baseSettings = options.settings || {};
+  const baseSettings = options.settings ? JSON.parse(options.settings) : {};
   const settings = context.puppeteer.isActive()
     ? {...baseSettings, port: await context.puppeteer.getBrowserPort()}
     : baseSettings;

This issue body was partially generated by patch-package.

[patrickhulce]

Thanks for filing @niieani!

Passing in --settings to CLI is broken.

--settings flag is supposed to work as designed :)

This is not how it was designed :) Are there any examples we forgot to update when LHCI came out of alpha that gave you this misleading impression?

Your example is expected to be used in a config file, but if you must from the CLI...

lhci collect --url=https://example.com --numberOfRuns=1 --settings.extraHeaders.Cookie=some-cookie-string

We can let this issue track feature support for string parsing of settings from the CLI flag, but this is definitely not the recommended route and we wouldn't want to promote using it.

[niieani]

oooh! I couldn't find any docs on how to use --settings (searched the entire repo, including tests), so I just assumed it takes a string with JSON. I would have never thought it takes multiple --settings with dots in between 😄

Thanks for explaining @patrickhulce. I think this is mostly a documentation bug then.

Ideally I'd want to be able to pass in a path to the settings JSON. Maybe --settingsFile=./mySettings.json.