Skip to content

Commit fca930e

Browse files
srawlinsbudde377
srawlins
authored and
budde377
committed
feat: Fail on launcher-, reporter-, plugin-, or preprocessor-load errors.
Stop karma if a launcher, reporter, plugin, or preprocessor fails to load, after attempting to load each of them, and reporting errors for each load error. Closes #855
1 parent a751293 commit fca930e

8 files changed

Lines changed: 203 additions & 53 deletions

File tree

‎lib/launcher.js‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@ var retryDecorator = require('./launchers/retry').decoratorFactory
1010
var processDecorator = require('./launchers/process').decoratorFactory
1111

1212
// TODO(vojta): remove once nobody uses it
13-
var baseBrowserDecoratorFactory = function (baseLauncherDecorator, captureTimeoutLauncherDecorator,
14-
retryLauncherDecorator, processLauncherDecorator) {
13+
var baseBrowserDecoratorFactory = function (
14+
baseLauncherDecorator,
15+
captureTimeoutLauncherDecorator,
16+
retryLauncherDecorator,
17+
processLauncherDecorator
18+
) {
1519
return function (launcher) {
1620
baseLauncherDecorator(launcher)
1721
captureTimeoutLauncherDecorator(launcher)
@@ -20,7 +24,7 @@ var baseBrowserDecoratorFactory = function (baseLauncherDecorator, captureTimeou
2024
}
2125
}
2226

23-
var Launcher = function (emitter, injector) {
27+
var Launcher = function (server, emitter, injector) {
2428
var browsers = []
2529
var lastStartTime
2630

@@ -61,15 +65,17 @@ var Launcher = function (emitter, injector) {
6165
var browser = injector.createChild([locals], ['launcher:' + name]).get('launcher:' + name)
6266
} catch (e) {
6367
if (e.message.indexOf('No provider for "launcher:' + name + '"') !== -1) {
64-
log.warn('Can not load "%s", it is not registered!\n ' +
68+
log.error('Cannot load browser "%s": it is not registered! ' +
6569
'Perhaps you are missing some plugin?', name)
6670
} else {
67-
log.warn('Can not load "%s"!\n ' + e.stack, name)
71+
log.error('Cannot load browser "%s"!\n ' + e.stack, name)
6872
}
6973

74+
emitter.emit('load_error', 'launcher', name)
7075
return
7176
}
7277

78+
if (server.loadErrors.length > 0) return []
7379
// TODO(vojta): remove in v1.0 (BC for old launchers)
7480
if (!browser.forceKill) {
7581
browser.forceKill = function () {
@@ -193,7 +199,7 @@ var Launcher = function (emitter, injector) {
193199
emitter.on('exit', this.killAll)
194200
}
195201

196-
Launcher.$inject = ['emitter', 'injector']
202+
Launcher.$inject = ['server', 'emitter', 'injector']
197203

198204
Launcher.generateId = function () {
199205
return '' + Math.floor(Math.random() * 100000000)

‎lib/plugin.js‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ var log = require('./logger').create('plugin')
66

77
var IGNORED_PACKAGES = ['karma-cli', 'karma-runner.github.com']
88

9-
exports.resolve = function (plugins) {
9+
exports.resolve = function (plugins, emitter) {
1010
var modules = []
1111

1212
var requirePlugin = function (name) {
@@ -15,35 +15,39 @@ exports.resolve = function (plugins) {
1515
modules.push(require(name))
1616
} catch (e) {
1717
if (e.code === 'MODULE_NOT_FOUND' && e.message.indexOf(name) !== -1) {
18-
log.warn('Cannot find plugin "%s".\n Did you forget to install it ?\n' +
18+
log.error('Cannot find plugin "%s".\n Did you forget to install it?\n' +
1919
' npm install %s --save-dev', name, name)
2020
} else {
21-
log.warn('Error during loading "%s" plugin:\n %s', name, e.message)
21+
log.error('Error during loading "%s" plugin:\n %s', name, e.message)
2222
}
23+
emitter.emit('load_error', 'plug_in', name)
2324
}
2425
}
2526

2627
plugins.forEach(function (plugin) {
2728
if (helper.isString(plugin)) {
28-
if (plugin.indexOf('*') !== -1) {
29-
var pluginDirectory = path.normalize(path.join(__dirname, '/../..'))
30-
var regexp = new RegExp('^' + plugin.replace('*', '.*'))
31-
32-
log.debug('Loading %s from %s', plugin, pluginDirectory)
33-
fs.readdirSync(pluginDirectory).filter(function (pluginName) {
34-
return IGNORED_PACKAGES.indexOf(pluginName) === -1 && regexp.test(pluginName)
35-
}).forEach(function (pluginName) {
36-
requirePlugin(pluginDirectory + '/' + pluginName)
37-
})
38-
} else {
29+
if (plugin.indexOf('*') === -1) {
3930
requirePlugin(plugin)
31+
return
4032
}
41-
} else if (helper.isObject(plugin)) {
33+
var pluginDirectory = path.normalize(path.join(__dirname, '/../..'))
34+
var regexp = new RegExp('^' + plugin.replace('*', '.*'))
35+
36+
log.debug('Loading %s from %s', plugin, pluginDirectory)
37+
fs.readdirSync(pluginDirectory).filter(function (pluginName) {
38+
return IGNORED_PACKAGES.indexOf(pluginName) === -1 && regexp.test(pluginName)
39+
}).forEach(function (pluginName) {
40+
requirePlugin(pluginDirectory + '/' + pluginName)
41+
})
42+
return
43+
}
44+
if (helper.isObject(plugin)) {
4245
log.debug('Loading inlined plugin (defining %s).', Object.keys(plugin).join(', '))
4346
modules.push(plugin)
44-
} else {
45-
log.warn('Invalid plugin %s', plugin)
47+
return
4648
}
49+
log.error('Invalid plugin %s', plugin)
50+
emitter.emit('load_error', 'plug_in', plugin)
4751
})
4852

4953
return modules

‎lib/preprocessor.js‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,14 @@ var createNextProcessor = function (preprocessors, file, done) {
3838
}
3939

4040
var createPreprocessor = function (config, basePath, injector) {
41-
var alreadyDisplayedWarnings = {}
41+
var alreadyDisplayedErrors = {}
4242
var instances = {}
4343
var patterns = Object.keys(config)
4444

45+
var emitter = injector.get('emitter')
46+
4547
var instantiatePreprocessor = function (name) {
46-
if (alreadyDisplayedWarnings[name]) {
48+
if (alreadyDisplayedErrors[name]) {
4749
return
4850
}
4951

@@ -53,13 +55,13 @@ var createPreprocessor = function (config, basePath, injector) {
5355
p = injector.get('preprocessor:' + name)
5456
} catch (e) {
5557
if (e.message.indexOf('No provider for "preprocessor:' + name + '"') !== -1) {
56-
log.warn('Can not load "%s", it is not registered!\n ' +
57-
'Perhaps you are missing some plugin?', name)
58+
log.error('Can not load "%s", it is not registered!\n ' +
59+
'Perhaps you are missing some plugin?', name)
5860
} else {
59-
log.warn('Can not load "%s"!\n ' + e.stack, name)
61+
log.error('Can not load "%s"!\n ' + e.stack, name)
6062
}
61-
62-
alreadyDisplayedWarnings[name] = true
63+
alreadyDisplayedErrors[name] = true
64+
emitter.emit('load_error', 'preprocessor', name)
6365
}
6466

6567
return p
@@ -105,9 +107,10 @@ var createPreprocessor = function (config, basePath, injector) {
105107
}
106108

107109
if (p == null) {
108-
if (!alreadyDisplayedWarnings[name]) {
109-
alreadyDisplayedWarnings[name] = true
110-
log.warn('Failed to instantiate preprocessor %s', name)
110+
if (!alreadyDisplayedErrors[name]) {
111+
alreadyDisplayedErrors[name] = true
112+
log.error('Failed to instantiate preprocessor %s', name)
113+
emitter.emit('load_error', 'preprocessor', name)
111114
}
112115
return
113116
}

‎lib/reporter.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,12 @@ var createReporters = function (names, config, emitter, injector) {
107107
reporters.push(injector.createChild([locals], ['reporter:' + name]).get('reporter:' + name))
108108
} catch (e) {
109109
if (e.message.indexOf('No provider for "reporter:' + name + '"') !== -1) {
110-
log.warn('Can not load "%s", it is not registered!\n ' +
110+
log.error('Can not load reporter "%s", it is not registered!\n ' +
111111
'Perhaps you are missing some plugin?', name)
112112
} else {
113-
log.warn('Can not load "%s"!\n ' + e.stack, name)
113+
log.error('Can not load "%s"!\n ' + e.stack, name)
114114
}
115+
emitter.emit('load_error', 'reporter', name)
115116
return
116117
}
117118
var color_name = name + '_color'

‎lib/server.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,16 @@ var Server = function (cliOptions, done) {
5151

5252
this.log = logger.create()
5353

54+
this.loadErrors = []
55+
5456
var config = cfg.parseConfig(cliOptions.configFile, cliOptions)
5557

5658
var modules = [{
5759
helper: ['value', helper],
5860
logger: ['value', logger],
5961
done: ['value', done || process.exit],
6062
emitter: ['value', this],
63+
server: ['value', this],
6164
launcher: ['type', Launcher],
6265
config: ['value', config],
6366
preprocess: ['factory', preprocessor.createPreprocessor],
@@ -82,8 +85,9 @@ var Server = function (cliOptions, done) {
8285
}]
8386
}]
8487

88+
this._setUpLoadErrorListener()
8589
// Load the plugins
86-
modules = modules.concat(plugin.resolve(config.plugins))
90+
modules = modules.concat(plugin.resolve(config.plugins, this))
8791

8892
this._injector = new di.Injector(modules)
8993
}
@@ -169,12 +173,16 @@ Server.prototype._start = function (config, launcher, preprocess, fileList, webS
169173
config.protocol, config.hostname, config.port, config.urlRoot)
170174

171175
self.emit('listening', config.port)
172-
173176
if (config.browsers && config.browsers.length) {
174177
self._injector.invoke(launcher.launch, launcher).forEach(function (browserLauncher) {
175178
singleRunDoneBrowsers[browserLauncher.id] = false
176179
})
177180
}
181+
var noLoadErrors = self.loadErrors.length
182+
if (noLoadErrors > 0) {
183+
self.log.error('Found %d load error%s', noLoadErrors, noLoadErrors === 1 ? '' : 's')
184+
process.kill(process.pid, 'SIGINT')
185+
}
178186
})
179187
}
180188

@@ -363,6 +371,14 @@ Server.prototype._start = function (config, launcher, preprocess, fileList, webS
363371
})
364372
}
365373

374+
Server.prototype._setUpLoadErrorListener = function () {
375+
var self = this
376+
self.on('load_error', function (type, name) {
377+
self.log.debug('Registered a load error of type %s with name %s', type, name)
378+
self.loadErrors.push([type, name])
379+
})
380+
}
381+
366382
Server.prototype._detach = function (config, done) {
367383
var log = this.log
368384
var tmpFile = tmp.fileSync({keep: true})

‎test/e2e/load.feature‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
Feature: Basic Testrunner
2+
In order to use Karma
3+
As a person who wants to write great tests
4+
I want Karma to terminate upon misconfiguration
5+
6+
Scenario: Execute with missing browser
7+
Given a configuration with:
8+
"""
9+
files = ['basic/plus.js', 'basic/test.js'];
10+
browsers = ['NonExistingBrowser', 'PhantomJS'];
11+
plugins = [
12+
'karma-jasmine',
13+
'karma-phantomjs-launcher'
14+
];
15+
singleRun = false
16+
"""
17+
When I start Karma
18+
Then it fails with like:
19+
"""
20+
Cannot load browser "NonExistingBrowser": it is not registered! Perhaps you are missing some plugin\?
21+
"""
22+
And it fails with like:
23+
"""
24+
Found 1 load error
25+
"""
26+
27+
Scenario: Execute with missing plugin
28+
Given a configuration with:
29+
"""
30+
files = ['basic/plus.js', 'basic/test.js'];
31+
browsers = ['PhantomJS'];
32+
plugins = [
33+
'karma-totally-non-existing-plugin',
34+
'karma-jasmine',
35+
'karma-phantomjs-launcher'
36+
];
37+
singleRun = false
38+
"""
39+
When I start Karma
40+
Then it fails with like:
41+
"""
42+
Cannot find plugin "karma-totally-non-existing-plugin".
43+
[\s]+Did you forget to install it\?
44+
[\s]+npm install karma-totally-non-existing-plugin --save-dev
45+
"""
46+
And it fails with like:
47+
"""
48+
Found 1 load error
49+
"""
50+
51+
Scenario: Execute with missing reporter
52+
Given a configuration with:
53+
"""
54+
files = ['basic/plus.js', 'basic/test.js'];
55+
browsers = ['PhantomJS'];
56+
reporters = ['unreal-reporter']
57+
plugins = [
58+
'karma-jasmine',
59+
'karma-phantomjs-launcher'
60+
];
61+
singleRun = false
62+
"""
63+
When I start Karma
64+
Then it fails with like:
65+
"""
66+
Can not load reporter "unreal-reporter", it is not registered!
67+
[\s]+Perhaps you are missing some plugin\?
68+
"""
69+
And it fails with like:
70+
"""
71+
Found 1 load error
72+
"""
73+
74+
Scenario: Execute with missing reporter, plugin and browser
75+
Given a configuration with:
76+
"""
77+
files = ['basic/plus.js', 'basic/test.js'];
78+
browsers = ['NonExistingBrowser', 'PhantomJS'];
79+
reporters = ['unreal-reporter']
80+
plugins = [
81+
'karma-totally-non-existing-plugin',
82+
'karma-jasmine',
83+
'karma-phantomjs-launcher'
84+
];
85+
singleRun = false
86+
"""
87+
When I start Karma
88+
Then it fails with like:
89+
"""
90+
Can not load reporter "unreal-reporter", it is not registered!
91+
[\s]+Perhaps you are missing some plugin\?
92+
"""
93+
And it fails with like:
94+
"""
95+
Cannot find plugin "karma-totally-non-existing-plugin".
96+
[\s]+Did you forget to install it\?
97+
[\s]+npm install karma-totally-non-existing-plugin --save-dev
98+
"""
99+
And it fails with like:
100+
"""
101+
Cannot load browser "NonExistingBrowser": it is not registered! Perhaps you are missing some plugin\?
102+
"""
103+
And it fails with like:
104+
"""
105+
Found 3 load errors
106+
"""

‎test/unit/launcher.spec.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,18 +69,22 @@ describe('launcher', () => {
6969

7070
describe('Launcher', () => {
7171
var emitter
72-
var l = emitter = null
72+
var server
73+
var l = emitter = server = null
7374

7475
beforeEach(() => {
7576
emitter = new events.EventEmitter()
77+
server = {'loadErrors': []}
78+
7679
var injector = new di.Injector([{
7780
'launcher:Fake': ['type', FakeBrowser],
7881
'launcher:Script': ['type', ScriptBrowser],
82+
'server': ['value', server],
7983
'emitter': ['value', emitter],
8084
'config': ['value', {captureTimeout: 0}],
8185
'timer': ['factory', createMockTimer]
8286
}])
83-
l = new launcher.Launcher(emitter, injector)
87+
l = new launcher.Launcher(server, emitter, injector)
8488
})
8589

8690
describe('launch', () => {
@@ -93,6 +97,15 @@ describe('launcher', () => {
9397
expect(browser.name).to.equal('Fake')
9498
})
9599

100+
it('should not start when server has load errors', () => {
101+
server.loadErrors = ['error']
102+
l.launch(['Fake'], 'http:', 'localhost', 1234, '/root/', 1)
103+
var browser = FakeBrowser._instances.pop()
104+
expect(browser.start).to.not.have.been.called
105+
expect(browser.id).to.equal(lastGeneratedId)
106+
expect(browser.name).to.equal('Fake')
107+
})
108+
96109
it('should allow launching a script', () => {
97110
l.launch(['/usr/local/bin/special-browser'], 'http:', 'localhost', 1234, '/', 1)
98111

0 commit comments

Comments
 (0)