fix: Merge config child nodes on config.set()

gnail · gnail · commit 65b688a9f50c · 2016-07-10T16:42:21.000+12:00
The old behaviour will override child option nodes like options.client, causing properties to be lost if the new option node is only partially specified. This partially causes karma-runner/grunt-karma#165 and karma-runner/grunt-karma#166.
diff --git a/lib/config.js b/lib/config.js
@@ -5,6 +5,8 @@ var log = logger.create('config')
 var helper = require('./helper')
 var constant = require('./constants')
 
+var _ = require('lodash')
+
 var COFFEE_SCRIPT_AVAILABLE = false
 var LIVE_SCRIPT_AVAILABLE = false
 
@@ -226,8 +228,11 @@ var Config = function () {
   this.LOG_DEBUG = constant.LOG_DEBUG
 
   this.set = function (newConfig) {
-    Object.keys(newConfig).forEach(function (key) {
-      config[key] = newConfig[key]
+    _.merge(config, newConfig, function (obj, src) {
+      // Overwrite arrays to keep consistent with #283
+      if (_.isArray(src)) {
+        return src
+      }
     })
   }
 
diff --git a/test/unit/config.spec.js b/test/unit/config.spec.js
@@ -39,6 +39,7 @@ describe('config', () => {
       '/home/config6.js': wrapCfg({reporters: 'junit'}),
       '/home/config7.js': wrapCfg({browsers: ['Chrome', 'Firefox']}),
       '/home/config8.js': (config) => config.set({ files: config.suite === 'e2e' ? ['tests/e2e.spec.js'] : ['tests/unit.spec.js'] }),
+      '/home/config9.js': wrapCfg({client: {useIframe: false}}),
       '/conf/invalid.js': () => { throw new SyntaxError('Unexpected token =') },
       '/conf/exclude.js': wrapCfg({exclude: ['one.js', 'sub/two.js']}),
       '/conf/absolute.js': wrapCfg({files: ['http://some.com', 'https://more.org/file.js']}),
@@ -138,13 +139,21 @@ describe('config', () => {
       expect(config.basePath).to.equal(resolveWinPath('/abs/base'))
     })
 
-    it('should override config with cli options, but not deep merge', () => {
+    it('should override config with cli options if the config property is an array', () => {
       // regression https://github.com/karma-runner/karma/issues/283
       var config = e.parseConfig('/home/config7.js', {browsers: ['Safari']})
 
       expect(config.browsers).to.deep.equal(['Safari'])
     })
 
+    it('should merge config with cli options if the config property is an object', () => {
+      // regression https://github.com/karma-runner/grunt-karma/issues/165
+      var config = e.parseConfig('/home/config9.js', {client: {captureConsole: false}})
+
+      expect(config.client.useIframe).to.equal(false)
+      expect(config.client.captureConsole).to.equal(false)
+    })
+
     it('should have access to cli options in the config file', () => {
       var config = e.parseConfig('/home/config8.js', {suite: 'e2e'})
       expect(patternsFrom(config.files)).to.deep.equal([resolveWinPath('/home/tests/e2e.spec.js')])
