Fixes from feedback

src/config.js

@@ -16,7 +16,7 @@ type ApplyConfigToArgvParams = {|
   // This is the argv object which will get updated by each
   // config applied.
   argv: Object,
-  // This is the argv that only has CLI values applies to it.
+  // This is the argv that only has CLI values applied to it.
   argvFromCLI: Object,
   configObject: Object,
   options: Object,
@@ -157,14 +157,14 @@ export async function discoverConfigFiles(
         log.debug(
           `Discovered config "${resolvedFileName}" does not ` +
           'exist or is not readable');
-        return false;
+        return undefined;
       }
     }
   ));
 
   const existingConfigs = [];
   configs.forEach((f) => {
-    if (f) {
+    if (typeof f === 'string') {
       existingConfigs.push(f);
     }
   });

src/program.js

@@ -36,10 +36,10 @@ type ExecuteOptions = {
   logStream?: typeof defaultLogStream,
   getVersion?: VersionGetterFn,
   applyConfigToArgv?: typeof defaultApplyConfigToArgv,
+  discoverConfigFiles?: typeof defaultConfigDiscovery,
   loadJSConfigFile?: typeof defaultLoadJSConfigFile,
   shouldExitProgram?: boolean,
   globalEnv?: string,
-  discoverConfigFiles?: typeof defaultConfigDiscovery,
 }
 
 
@@ -129,10 +129,10 @@ export class Program {
       logStream = defaultLogStream,
       getVersion = defaultVersionGetter,
       applyConfigToArgv = defaultApplyConfigToArgv,
+      discoverConfigFiles = defaultConfigDiscovery,
       loadJSConfigFile = defaultLoadJSConfigFile,
       shouldExitProgram = true,
       globalEnv = WEBEXT_BUILD_ENV,
-      discoverConfigFiles = defaultConfigDiscovery,
     }: ExecuteOptions = {}
   ): Promise<void> {
 

tests/unit/helpers.js

@@ -110,9 +110,8 @@ export function fixturePath(...pathParts: Array<string>): string {
  *
  * Usage:
  *
- *  Promise.resolve()
- *    .then(makeSureItFails())
- *    .catch((error) => {
+ *  Promise.reject(new Error('some error'))
+ *    .then(makeSureItFails(), (error) => {
  *      // Safely make assertions about the error...
  *    });
  */

tests/unit/test-util/test.file-exists.js

@@ -7,7 +7,7 @@ import {fs} from 'mz';
 
 import fileExists from '../../../src/util/file-exists';
 import {withTempDir} from '../../../src/util/temp-dir';
-import {ErrorWithCode} from '../helpers';
+import {makeSureItFails, ErrorWithCode} from '../helpers';
 
 
 describe('util/file-exists', () => {
@@ -46,4 +46,15 @@ describe('util/file-exists', () => {
     });
     assert.equal(exists, false);
   });
+
+  it('throws unexpected errors', async () => {
+    const exists = fileExists('pretend/file', {
+      fileIsReadable: async () => {
+        throw new ErrorWithCode('EBUSY', 'device is busy');
+      },
+    });
+    await exists.then(makeSureItFails(), (error) => {
+      assert.equal(error.message, 'EBUSY: device is busy');
+    });
+  });
 });

tests/unit/test.config.js

@@ -249,7 +249,7 @@ describe('config', () => {
       assert.strictEqual(newArgv.overwriteFiles, true);
     });
 
-    it('can load multiple configs', () => {
+    it('can load multiple configs for global options', () => {
       const params = makeArgv({
         userCmd: ['fakecommand'],
         globalOpt: {
@@ -260,6 +260,7 @@ describe('config', () => {
         },
       });
 
+      // Make sure the second global option overrides the first.
       const firstConfigObject = {
         filePath: 'first/path',
       };
@@ -505,7 +506,7 @@ describe('config', () => {
       assert.strictEqual(newArgv.apiKey, cmdApiKey);
     });
 
-    it('can load multiple configs', () => {
+    it('can load multiple configs for sub-command options', () => {
       const params = makeArgv({
         userCmd: ['sign'],
         command: 'sign',
@@ -517,6 +518,7 @@ describe('config', () => {
         },
       });
 
+      // Make sure the second sub-command option overrides the first.
       const firstConfigObject = {
         sign: {
           filePath: 'first/path',

tests/unit/test.program.js

@@ -7,6 +7,7 @@ import {fs} from 'mz';
 import sinon, {spy} from 'sinon';
 import {assert} from 'chai';
 
+import {applyConfigToArgv} from '../../src/config';
 import {defaultVersionGetter, main, Program} from '../../src/program';
 import commands from '../../src/cmd';
 import {
@@ -538,12 +539,13 @@ describe('program.main', () => {
       return configObject;
     });
 
+    const discoveredFile = 'fake/config.js';
     await execProgram(
       ['lint'],
       {
         commands: fakeCommands,
         runOptions: {
-          discoverConfigFiles: async () => ['fake/config.js'],
+          discoverConfigFiles: async () => [discoveredFile],
           loadJSConfigFile: fakeLoadJSConfigFile,
         },
       }
@@ -554,6 +556,8 @@ describe('program.main', () => {
     // to the lint command options.
     assert.equal(
       options.selfHosted, configObject.lint.selfHosted);
+
+    sinon.assert.calledWith(fakeLoadJSConfigFile, discoveredFile);
   });
 
   it('lets you disable config discovery', async () => {
@@ -599,12 +603,14 @@ describe('program.main', () => {
         },
       },
     });
+    const fakeApplyConfigToArgv = sinon.spy(applyConfigToArgv);
 
     await execProgram(
       ['lint', '--config', customConfig],
       {
         commands: fakeCommands,
         runOptions: {
+          applyConfigToArgv: fakeApplyConfigToArgv,
           discoverConfigFiles: async () => [
             globalConfig, projectConfig,
           ],
@@ -613,10 +619,22 @@ describe('program.main', () => {
       }
     );
 
+    // Check that the config files were all applied to argv.
     const options = fakeCommands.lint.firstCall.args[0];
     assert.equal(options.noInput, true);
     assert.equal(options.verbose, true);
     assert.equal(options.selfHosted, true);
+
+    // Make sure the config files were loaded in the right order.
+    assert.include(fakeApplyConfigToArgv.firstCall.args[0], {
+      configFileName: globalConfig,
+    });
+    assert.include(fakeApplyConfigToArgv.secondCall.args[0], {
+      configFileName: projectConfig,
+    });
+    assert.include(fakeApplyConfigToArgv.thirdCall.args[0], {
+      configFileName: customConfig,
+    });
   });
 
   it('overwrites old config values', async () => {