fix: Fix config parsing issues

mozilla · Dec 13, 2017 · 535c026 · 535c026
1 parent 8a8ee94
commit 535c026
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 27 deletions.
diff --git a/src/config.js b/src/config.js
@@ -36,7 +36,7 @@ export function applyConfigToArgv({
       typeof configObject[option] === 'object') {
       // Descend into the nested configuration for a sub-command.
       newArgv = applyConfigToArgv({
-        argv,
+        argv: newArgv,
         configObject: configObject[option],
         options: options[option],
         configFileName});
@@ -87,6 +87,13 @@ export function applyConfigToArgv({
     }
 
     newArgv[option] = configObject[option];
+
+    const coerce = options[decamelizedOptName].coerce;
+    if (coerce) {
+      log.debug(
+        `Calling coerce() on configured value for ${option}`);
+      newArgv[option] = coerce(newArgv[option]);
+    }
   }
   return newArgv;
 }

diff --git a/src/program.js b/src/program.js
@@ -155,7 +155,7 @@ export class Program {
         });
       }
 
-      let argvFromConfig = { ...argv };
+      let argvFromConfig = {...argv};
       if (argv.config) {
         const configFileName = path.resolve(argv.config);
         const configObject = loadJSConfigFile(configFileName);

diff --git a/tests/unit/test.config.js b/tests/unit/test.config.js
@@ -157,6 +157,29 @@ describe('config', () => {
       assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir);
     });
 
+    it('coerces config option values if needed', () => {
+      const coerce = (sourceDir) => `coerced(${sourceDir})`;
+      const params = makeArgv({
+        userCmd: ['fakecommand'],
+        globalOpt: {
+          'source-dir': {
+            requiresArg: true,
+            type: 'string',
+            demand: false,
+            // In the real world this would do something like
+            // (sourceDir) => path.resolve(sourceDir)
+            coerce,
+          },
+        },
+      });
+
+      const sourceDir = '/configured/source/dir';
+      const configObject = {sourceDir};
+
+      const newArgv = applyConf({...params, configObject});
+      assert.strictEqual(newArgv.sourceDir, coerce(sourceDir));
+    });
+
     it('uses a configured boolean value over an implicit default', () => {
       const params = makeArgv({
         globalOpt: {
@@ -508,6 +531,38 @@ describe('config', () => {
       assert.strictEqual(newArgv.apiKey, cmdApiKey);
     });
 
+    it('preserves global option when sub-command options exist', () => {
+      const params = makeArgv({
+        userCmd: ['sign'],
+        command: 'sign',
+        commandOpt: {
+          'api-key': {
+            requiresArg: true,
+            type: 'string',
+            demand: false,
+          },
+        },
+        globalOpt: {
+          'source-dir': {
+            requiresArg: true,
+            type: 'string',
+            demand: false,
+          },
+        },
+      });
+      const sourceDir = 'custom/source/dir';
+      const configObject = {
+        // This global option should not be affected by the
+        // recursion code that processes the sub-command option.
+        sourceDir,
+        sign: {
+          apiKey: 'custom-configured-key',
+        },
+      };
+      const newArgv = applyConf({...params, configObject});
+      assert.strictEqual(newArgv.sourceDir, sourceDir);
+    });
+
     it('handles camel case sub-commands', () => {
       const params = makeArgv({
         userCmd: ['sign-extension'],

diff --git a/tests/unit/test.program.js b/tests/unit/test.program.js
@@ -472,46 +472,30 @@ describe('program.main', () => {
     const fakeCommands = fake(commands, {
       lint: () => Promise.resolve(),
     });
-    const fakePath = 'path/to/web-ext-config.js';
     const configObject = {
-      prop: 'prop',
-    };
-    const resolvedFakePath = path.resolve(fakePath);
-    const expectedArgv = {
-      _: ['lint'],
-      config: fakePath,
+      lint: {
+        selfHosted: true,
+      },
     };
+    // Instead of loading/parsing a real file, just return an object.
     const fakeLoadJSConfigFile = sinon.spy(() => {
       return configObject;
     });
-    const fakeApplyConfigToArgv = sinon.spy(() => {
-      return expectedArgv;
-    });
 
     return execProgram(
-      ['lint', '--config', fakePath],
+      ['lint', '--config', 'path/to/web-ext-config.js'],
       {
         commands: fakeCommands,
         runOptions: {
-          applyConfigToArgv: fakeApplyConfigToArgv,
           loadJSConfigFile: fakeLoadJSConfigFile,
         },
       })
       .then(() => {
         const options = fakeCommands.lint.firstCall.args[0];
-        assert.strictEqual(options.config, fakePath);
-        sinon.assert.calledOnce(fakeLoadJSConfigFile);
-        sinon.assert.calledWith(fakeLoadJSConfigFile,
-                                sinon.match(resolvedFakePath));
-        sinon.assert.calledOnce(fakeApplyConfigToArgv);
-        sinon.assert.calledWith(fakeApplyConfigToArgv, sinon.match({
-          configFileName: resolvedFakePath,
-          configObject,
-          argv: {
-            _: ['lint'],
-            config: fakePath,
-          },
-        }));
+        // This makes sure that the config object was applied
+        // to the lint command options.
+        assert.equal(
+          options.selfHosted, configObject.lint.selfHosted);
       });
   });
 });