cli: correctly parse screenEmulation boolean flags (#12250)

GoogleChrome · Mar 17, 2021 · 051d34d · 051d34d
1 parent aae0d0e
commit 051d34d
Show file tree

Hide file tree

Showing 2 changed files with 133 additions and 7 deletions.
diff --git a/lighthouse-cli/cli-flags.js b/lighthouse-cli/cli-flags.js
@@ -13,13 +13,14 @@ const {isObjectOfUnknownValues} = require('../lighthouse-core/lib/type-verifiers
 
 /**
  * @param {string=} manualArgv
+ * @param {{noExitOnFailure?: boolean}=} options
  * @return {LH.CliFlags}
  */
-function getFlags(manualArgv) {
+function getFlags(manualArgv, options = {}) {
   // @ts-expect-error - undocumented, but yargs() supports parsing a single `string`.
   const y = manualArgv ? yargs(manualArgv) : yargs;
 
-  const argv = y.help('help')
+  let parser = y.help('help')
       .showHelpOnFail(false, 'Specify --help for available options')
 
       .usage('lighthouse <url> <options>')
@@ -308,11 +309,20 @@ function getFlags(manualArgv) {
         throw new Error('Please provide a url');
       })
       .epilogue('For more information on Lighthouse, see https://developers.google.com/web/tools/lighthouse/.')
-      .wrap(yargs.terminalWidth())
-      .argv;
+      .wrap(yargs.terminalWidth());
+
+  if (options.noExitOnFailure) {
+    // Silence console.error() logging and don't process.exit().
+    // `parser.fail(false)` can be used in yargs once v17 is released.
+    parser = parser.fail((msg, err) => {
+      if (err) throw err;
+      else if (msg) throw new Error(msg);
+    });
+  }
 
   // Augmenting yargs type with auto-camelCasing breaks in tsc@4.1.2 and @types/yargs@15.0.11,
   // so for now cast to add yarg's camelCase properties to type.
+  const argv = parser.argv;
   const cliFlags = /** @type {typeof argv & CamelCasify<typeof argv>} */ (argv);
 
   return cliFlags;
@@ -456,10 +466,17 @@ function coerceScreenEmulation(value) {
         break;
       case 'mobile':
       case 'disabled':
-        if (possibleSetting !== undefined && typeof possibleSetting !== 'boolean') {
+        // Manually coerce 'true'/'false' strings to booleans since nested property types aren't set.
+        if (possibleSetting === 'true') {
+          screenEmulationSettings[key] = true;
+        } else if (possibleSetting === 'false') {
+          screenEmulationSettings[key] = false;
+        } else if (possibleSetting === undefined || typeof possibleSetting === 'boolean') {
+          screenEmulationSettings[key] = possibleSetting;
+        } else {
           throw new Error(`Invalid value: 'screenEmulation.${key}' must be a boolean`);
         }
-        screenEmulationSettings[key] = possibleSetting;
+
         break;
       default:
         throw new Error(`Unrecognized screenEmulation option: ${key}`);

diff --git a/lighthouse-cli/test/cli/cli-flags-test.js b/lighthouse-cli/test/cli/cli-flags-test.js
@@ -9,7 +9,7 @@
 const assert = require('assert').strict;
 const getFlags = require('../../cli-flags.js').getFlags;
 
-describe('CLI bin', function() {
+describe('CLI flags', function() {
   it('all options should have descriptions', () => {
     getFlags('chrome://version');
     const yargs = require('yargs');
@@ -94,4 +94,113 @@ describe('CLI bin', function() {
       expect(flags).toHaveProperty('extraHeaders', require(headersFile));
     });
   });
+
+  describe('screenEmulation', () => {
+    const url = 'http://www.example.com';
+
+    describe('width', () => {
+      it('parses a number value', () => {
+        const flags = getFlags(`${url} --screenEmulation.width=500`, {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({width: 500});
+      });
+
+      it('throws on a non-number', () => {
+        expect(() => getFlags(`${url} --screenEmulation.width=yah`, {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.width' must be a number`);
+      });
+
+      it('throws with no flag value', () => {
+        expect(() => getFlags(`${url} --screenEmulation.width`, {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.width' must be a number`);
+      });
+    });
+
+    describe('height', () => {
+      it('parses a number value', () => {
+        const flags = getFlags(`${url} --screenEmulation.height=123`, {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({height: 123});
+      });
+
+      it('throws on a non-number', () => {
+        expect(() => getFlags(`${url} --screenEmulation.height=false`, {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.height' must be a number`);
+      });
+
+      it('throws with no flag value', () => {
+        expect(() => getFlags(`${url} --screenEmulation.height`, {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.height' must be a number`);
+      });
+    });
+
+    describe('deviceScaleFactor', () => {
+      it('parses a non-integer numeric value', () => {
+        const flags = getFlags(`${url} --screenEmulation.deviceScaleFactor=1.325`,
+            {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({deviceScaleFactor: 1.325});
+      });
+
+      it('throws on a non-number', () => {
+        expect(() => getFlags(`${url} --screenEmulation.deviceScaleFactor=12px`,
+            {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.deviceScaleFactor' must be a number`);
+      });
+
+      it('throws with no flag value', () => {
+        expect(() => getFlags(`${url} --screenEmulation.deviceScaleFactor`,
+            {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.deviceScaleFactor' must be a number`);
+      });
+    });
+
+    describe('mobile', () => {
+      it('parses the flag with no value as true', () => {
+        const flags = getFlags(`${url} --screenEmulation.mobile`, {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({mobile: true});
+      });
+
+      it('parses the --no-mobile flag as false', () => {
+        const flags = getFlags(`${url} --no-screenEmulation.mobile`, {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({mobile: false});
+      });
+
+      it('parses the flag with a boolean value', () => {
+        const flagsTrue = getFlags(`${url} --screenEmulation.mobile=true`, {noExitOnFailure: true});
+        expect(flagsTrue.screenEmulation).toEqual({mobile: true});
+        const flagsFalse = getFlags(`${url} --screenEmulation.mobile=false`,
+            {noExitOnFailure: true});
+        expect(flagsFalse.screenEmulation).toEqual({mobile: false});
+      });
+
+      it('throws on a non-boolean value', () => {
+        expect(() => getFlags(`${url} --screenEmulation.mobile=2`, {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.mobile' must be a boolean`);
+      });
+    });
+
+    describe('disabled', () => {
+      it('parses the flag with no value as true', () => {
+        const flags = getFlags(`${url} --screenEmulation.disabled`, {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({disabled: true});
+      });
+
+      it('parses the --no-disabled flag as false', () => {
+        const flags = getFlags(`${url} --no-screenEmulation.disabled`, {noExitOnFailure: true});
+        expect(flags.screenEmulation).toEqual({disabled: false});
+      });
+
+      it('parses the flag with a boolean value', () => {
+        const flagsTrue = getFlags(`${url} --screenEmulation.disabled=true`,
+            {noExitOnFailure: true});
+        expect(flagsTrue.screenEmulation).toEqual({disabled: true});
+        const flagsFalse = getFlags(`${url} --screenEmulation.disabled=false`,
+            {noExitOnFailure: true});
+        expect(flagsFalse.screenEmulation).toEqual({disabled: false});
+      });
+
+      it('throws on a non-boolean value', () => {
+        expect(() => getFlags(`${url} --screenEmulation.disabled=str`, {noExitOnFailure: true}))
+          .toThrow(`Invalid value: 'screenEmulation.disabled' must be a boolean`);
+      });
+    });
+  });
 });