test: more specific testing

mozilla · Nov 20, 2017 · 67d04a8 · 67d04a8
1 parent 737dd74
commit 67d04a8
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 208 deletions.
diff --git a/src/config.js b/src/config.js
@@ -6,7 +6,7 @@ import camelCase from 'camelcase';
 import decamelize from 'decamelize';
 
 import {createLogger} from './util/logger';
-import {UsageError} from './errors';
+import {UsageError, WebExtError} from './errors';
 
 const log = createLogger(__filename);
 
@@ -50,7 +50,8 @@ export function applyConfigToArgv({
         `an unknown option: "${option}"`);
     }
     if (options[decamelizedOptName].type === undefined) {
-      throw new UsageError(
+      // This means yargs option type wasn't not defined correctly
+      throw new WebExtError(
         `Option: ${option} was defined without a type.`);
     }
 

diff --git a/src/program.js b/src/program.js
@@ -288,7 +288,7 @@ Example: $0 --help run.
     },
     'config': {
       alias: 'c',
-      describe: 'Location of the config file',
+      describe: 'Path to the config file',
       default: undefined,
       requiresArg: true,
       type: 'string',

diff --git a/src/util/logger.js b/src/util/logger.js
@@ -110,7 +110,7 @@ export type CreateBunyanLogParams = {|
 export type CreateBunyanLogFn = (params: CreateBunyanLogParams) => Logger;
 
 export type CreateLoggerOptions = {|
-  createBunyanLog?: CreateBunyanLogFn,
+  createBunyanLog: CreateBunyanLogFn,
 |};
 
 export function createLogger(

diff --git a/tests/unit/test.config.js b/tests/unit/test.config.js
@@ -12,7 +12,7 @@ import {
   loadJSConfigFile,
 } from '../../src/config';
 import {withTempDir} from '../../src/util/temp-dir';
-import {UsageError} from '../../src/errors';
+import {UsageError, WebExtError} from '../../src/errors';
 
 type MakeArgvParams = {|
   userCmd?: Array<string>,
@@ -657,39 +657,8 @@ describe('config', () => {
       };
       assert.throws(() => {
         applyConf({...params, configObject});
-      }, UsageError,
-        'UsageError: Option: apiUrl was defined without a type.');
-    });
-
-    it('throws an error when the type of one of them is in config' +
-      ' missing', () => {
-      const params = makeArgv({
-        userCmd: ['sign'],
-        command: 'sign',
-        commandOpt: {
-          'api-url': {
-            requiresArg: true,
-            demand: false,
-            default: 'pretend-default-value-of-apiKey',
-          },
-          'api-key': {
-            requiresArg: true,
-            demand: false,
-            type: 'string',
-            default: 'pretend-default-value-of-apiKey',
-          },
-        },
-      });
-      const configObject = {
-        sign: {
-          apiUrl: 2,
-          apiKey: 'fake-api-key',
-        },
-      };
-      assert.throws(() => {
-        applyConf({...params, configObject});
-      }, UsageError,
-        'UsageError: Option: apiUrl was defined without a type.');
+      }, WebExtError,
+        'WebExtError: Option: apiUrl was defined without a type.');
     });
 
     it('throws an error when type of unrelated sub option is invalid', () => {

diff --git a/tests/unit/test.program.js b/tests/unit/test.program.js
@@ -20,7 +20,7 @@ import {
   ErrorWithCode,
 } from './helpers';
 import {
-  consoleStream,
+  consoleStream, // instance is imported to inspect logged messages
   ConsoleStream,
 } from '../../src/util/logger';
 
@@ -222,6 +222,33 @@ describe('program.Program', () => {
       });
   });
 
+  it('logs UsageErrors into console', () => {
+    // Clear console stream from previous messages and start recording
+    consoleStream.stopCapturing();
+    consoleStream.flushCapturedLogs();
+    consoleStream.startCapturing();
+
+    const program = new Program(['thing']).command('thing', '', () => {
+      throw new UsageError('some error');
+    });
+    program.setGlobalOptions({
+      verbose: {
+        type: 'boolean',
+      },
+    });
+    return execProgram(program)
+      .then(makeSureItFails())
+      .catch(onlyInstancesOf(UsageError, (error) => {
+        const {capturedMessages} = consoleStream;
+        // Stop recording
+        consoleStream.stopCapturing();
+        assert.match(error.message, /some error/);
+        assert.ok(
+          capturedMessages.some((message) => message.match(/some error/)
+        ));
+      }));
+  });
+
   it('throws an error about unknown commands', () => {
     return execProgram(new Program(['nope']))
       .then(makeSureItFails())
@@ -441,8 +468,13 @@ describe('program.main', () => {
       lint: () => Promise.resolve(),
     });
     const fakePath = 'path/to/web-ext-config.js';
+    const configObject = {
+      prop: 'prop',
+    };
     const resolvedFakePath = path.resolve(fakePath);
-    const fakeLoadJSConfigFile = sinon.spy(() => {});
+    const fakeLoadJSConfigFile = sinon.spy(() => {
+      return configObject;
+    });
     const fakeApplyConfigToArgv = sinon.spy(() => {});
 
     return execProgram(
@@ -463,177 +495,14 @@ describe('program.main', () => {
         sinon.assert.calledOnce(fakeApplyConfigToArgv);
         sinon.assert.calledWith(fakeApplyConfigToArgv, sinon.match({
           configFileName: resolvedFakePath,
+          configObject,
+          argv: {
+            _: ['lint'],
+            config: fakePath,
+          },
         }));
       });
   });
-
-  it('throws a UsageError when the config file can\'t be loaded', () => {
-    const fakeProcess = createFakeProcess();
-    const fakeCommands = fake(commands, {
-      lint: () => Promise.resolve(),
-    });
-    const fakePath = 'path/to/web-ext-config.js';
-    const fakeLoadJSConfigFile = sinon.spy(() => {
-      throw new UsageError('bad path');
-    });
-    const fakeApplyConfigToArgv = sinon.spy(() => {});
-
-    consoleStream.stopCapturing();
-    consoleStream.flushCapturedLogs();
-    consoleStream.startCapturing();
-
-    return execProgram(
-      ['lint', '--config', fakePath],
-      {
-        commands: fakeCommands,
-        runOptions: {
-          shouldExitProgram: false,
-          systemProcess: fakeProcess,
-          applyConfigToArgv: fakeApplyConfigToArgv,
-          loadJSConfigFile: fakeLoadJSConfigFile,
-        },
-      })
-      .then(makeSureItFails())
-      .catch(onlyInstancesOf(UsageError, (error) => {
-        const {capturedMessages} = consoleStream;
-        consoleStream.stopCapturing();
-
-        assert.match(error.message, /bad path/);
-        assert.ok(
-          capturedMessages.some(
-            (message) => message.match(/bad path/)
-        ));
-        sinon.assert.notCalled(fakeApplyConfigToArgv);
-        sinon.assert.notCalled(fakeProcess.exit);
-      }));
-  });
-
-  it('throws when config file loading raises an unexpected error', () => {
-    const fakeProcess = createFakeProcess();
-    const fakeCommands = fake(commands, {
-      lint: () => Promise.resolve(),
-    });
-    const fakePath = 'path/to/web-ext-config.js';
-    const fakeLoadJSConfigFile = sinon.spy(() => {
-      throw new Error('some error');
-    });
-    const fakeApplyConfigToArgv = sinon.spy(() => {});
-
-    consoleStream.stopCapturing();
-    consoleStream.flushCapturedLogs();
-    consoleStream.startCapturing();
-
-    return execProgram(
-      ['lint', '--config', fakePath],
-      {
-        commands: fakeCommands,
-        runOptions: {
-          shouldExitProgram: false,
-          systemProcess: fakeProcess,
-          applyConfigToArgv: fakeApplyConfigToArgv,
-          loadJSConfigFile: fakeLoadJSConfigFile,
-        },
-      })
-      .then(makeSureItFails())
-      .catch((error) => {
-        const {capturedMessages} = consoleStream;
-        consoleStream.stopCapturing();
-
-        if (error instanceof UsageError) {
-          throw error;
-        }
-
-        assert.match(error.message, /some error/);
-        assert.ok(capturedMessages.some(
-          (message) => message.match(/some error/)
-        ));
-        sinon.assert.notCalled(fakeApplyConfigToArgv);
-        sinon.assert.notCalled(fakeProcess.exit);
-      });
-  });
-
-  it('throws a UsageError when the loaded config can\'t be applied', () => {
-    const fakeProcess = createFakeProcess();
-    const fakeCommands = fake(commands, {
-      lint: () => Promise.resolve(),
-    });
-    const fakePath = 'path/to/web-ext-config.js';
-    const fakeLoadJSConfigFile = sinon.spy(() => {});
-    const fakeApplyConfigToArgv = sinon.spy(() => {
-      throw new UsageError('bad config option');
-    });
-
-    consoleStream.stopCapturing();
-    consoleStream.flushCapturedLogs();
-    consoleStream.startCapturing();
-
-    return execProgram(
-      ['lint', '--config', fakePath],
-      {
-        commands: fakeCommands,
-        runOptions: {
-          shouldExitProgram: false,
-          systemProcess: fakeProcess,
-          applyConfigToArgv: fakeApplyConfigToArgv,
-          loadJSConfigFile: fakeLoadJSConfigFile,
-        },
-      })
-      .then(makeSureItFails())
-      .catch(onlyInstancesOf(UsageError, (error) => {
-        const {capturedMessages} = consoleStream;
-        consoleStream.stopCapturing();
-
-        assert.match(error.message, /bad config option/);
-        assert.ok(
-          capturedMessages.some(
-            (message) => message.match(/bad config option/)
-        ));
-        sinon.assert.notCalled(fakeProcess.exit);
-      }));
-  });
-
-  it('throws when fakeApplyConfigToArgv throws an unexpected error', () => {
-    const fakeProcess = createFakeProcess();
-    const fakeCommands = fake(commands, {
-      lint: () => Promise.resolve(),
-    });
-    const fakePath = 'path/to/web-ext-config.js';
-    const fakeLoadJSConfigFile = sinon.spy(() => {});
-    const fakeApplyConfigToArgv = sinon.spy(() => {
-      throw new Error('some error');
-    });
-
-    consoleStream.stopCapturing();
-    consoleStream.flushCapturedLogs();
-    consoleStream.startCapturing();
-
-    return execProgram(
-      ['lint', '--config', fakePath],
-      {
-        commands: fakeCommands,
-        runOptions: {
-          shouldExitProgram: false,
-          systemProcess: fakeProcess,
-          applyConfigToArgv: fakeApplyConfigToArgv,
-          loadJSConfigFile: fakeLoadJSConfigFile,
-        },
-      })
-      .then(makeSureItFails())
-      .catch((error) => {
-        const {capturedMessages} = consoleStream;
-        consoleStream.stopCapturing();
-
-        if (error instanceof UsageError) {
-          throw error;
-        }
-
-        assert.match(error.message, /some error/);
-        assert.ok(capturedMessages.some(
-          (message) => message.match(/some error/)
-        ));
-        sinon.assert.notCalled(fakeProcess.exit);
-      });
-  });
 });
 
 describe('program.defaultVersionGetter', () => {