Skip to content

Commit 65b688a

Browse files
committed
fix: Merge config child nodes on config.set()
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.
1 parent c67a7d3 commit 65b688a

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed

lib/config.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ var log = logger.create('config')
55
var helper = require('./helper')
66
var constant = require('./constants')
77

8+
var _ = require('lodash')
9+
810
var COFFEE_SCRIPT_AVAILABLE = false
911
var LIVE_SCRIPT_AVAILABLE = false
1012

@@ -226,8 +228,11 @@ var Config = function () {
226228
this.LOG_DEBUG = constant.LOG_DEBUG
227229

228230
this.set = function (newConfig) {
229-
Object.keys(newConfig).forEach(function (key) {
230-
config[key] = newConfig[key]
231+
_.merge(config, newConfig, function (obj, src) {
232+
// Overwrite arrays to keep consistent with #283
233+
if (_.isArray(src)) {
234+
return src
235+
}
231236
})
232237
}
233238

test/unit/config.spec.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ describe('config', () => {
3939
'/home/config6.js': wrapCfg({reporters: 'junit'}),
4040
'/home/config7.js': wrapCfg({browsers: ['Chrome', 'Firefox']}),
4141
'/home/config8.js': (config) => config.set({ files: config.suite === 'e2e' ? ['tests/e2e.spec.js'] : ['tests/unit.spec.js'] }),
42+
'/home/config9.js': wrapCfg({client: {useIframe: false}}),
4243
'/conf/invalid.js': () => { throw new SyntaxError('Unexpected token =') },
4344
'/conf/exclude.js': wrapCfg({exclude: ['one.js', 'sub/two.js']}),
4445
'/conf/absolute.js': wrapCfg({files: ['http://some.com', 'https://more.org/file.js']}),
@@ -138,13 +139,21 @@ describe('config', () => {
138139
expect(config.basePath).to.equal(resolveWinPath('/abs/base'))
139140
})
140141

141-
it('should override config with cli options, but not deep merge', () => {
142+
it('should override config with cli options if the config property is an array', () => {
142143
// regression https://github.com/karma-runner/karma/issues/283
143144
var config = e.parseConfig('/home/config7.js', {browsers: ['Safari']})
144145

145146
expect(config.browsers).to.deep.equal(['Safari'])
146147
})
147148

149+
it('should merge config with cli options if the config property is an object', () => {
150+
// regression https://github.com/karma-runner/grunt-karma/issues/165
151+
var config = e.parseConfig('/home/config9.js', {client: {captureConsole: false}})
152+
153+
expect(config.client.useIframe).to.equal(false)
154+
expect(config.client.captureConsole).to.equal(false)
155+
})
156+
148157
it('should have access to cli options in the config file', () => {
149158
var config = e.parseConfig('/home/config8.js', {suite: 'e2e'})
150159
expect(patternsFrom(config.files)).to.deep.equal([resolveWinPath('/home/tests/e2e.spec.js')])

0 commit comments

Comments
 (0)