Skip to content

Commit

Permalink
feat: Fail on launcher-, reporter-, plugin-, or preprocessor-load err…
Browse files Browse the repository at this point in the history 
…ors.

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
  • Loading branch information
@srawlins @budde377
srawlins authored and budde377 committed Mar 1, 2016
1 parent a751293 commit fca930e
Show file tree
Hide file tree
Showing 8 changed files with 203 additions and 53 deletions.
18 changes: 12 additions & 6 deletions lib/launcher.js
View file
Expand Up @@ -10,8 +10,12 @@ var retryDecorator = require('./launchers/retry').decoratorFactory
var processDecorator = require('./launchers/process').decoratorFactory

// TODO(vojta): remove once nobody uses it
var baseBrowserDecoratorFactory = function (baseLauncherDecorator, captureTimeoutLauncherDecorator,
retryLauncherDecorator, processLauncherDecorator) {
var baseBrowserDecoratorFactory = function (
baseLauncherDecorator,
captureTimeoutLauncherDecorator,
retryLauncherDecorator,
processLauncherDecorator
) {
return function (launcher) {
baseLauncherDecorator(launcher)
captureTimeoutLauncherDecorator(launcher)
Expand All @@ -20,7 +24,7 @@ var baseBrowserDecoratorFactory = function (baseLauncherDecorator, captureTimeou
}
}

var Launcher = function (emitter, injector) {
var Launcher = function (server, emitter, injector) {
var browsers = []
var lastStartTime

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

emitter.emit('load_error', 'launcher', name)
return
}

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

Launcher.$inject = ['emitter', 'injector']
Launcher.$inject = ['server', 'emitter', 'injector']

Launcher.generateId = function () {
return '' + Math.floor(Math.random() * 100000000)
Expand Down
38 changes: 21 additions & 17 deletions lib/plugin.js
View file
Expand Up @@ -6,7 +6,7 @@ var log = require('./logger').create('plugin')

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

exports.resolve = function (plugins) {
exports.resolve = function (plugins, emitter) {
var modules = []

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

plugins.forEach(function (plugin) {
if (helper.isString(plugin)) {
if (plugin.indexOf('*') !== -1) {
var pluginDirectory = path.normalize(path.join(__dirname, '/../..'))
var regexp = new RegExp('^' + plugin.replace('*', '.*'))

log.debug('Loading %s from %s', plugin, pluginDirectory)
fs.readdirSync(pluginDirectory).filter(function (pluginName) {
return IGNORED_PACKAGES.indexOf(pluginName) === -1 && regexp.test(pluginName)
}).forEach(function (pluginName) {
requirePlugin(pluginDirectory + '/' + pluginName)
})
} else {
if (plugin.indexOf('*') === -1) {
requirePlugin(plugin)
return
}
} else if (helper.isObject(plugin)) {
var pluginDirectory = path.normalize(path.join(__dirname, '/../..'))
var regexp = new RegExp('^' + plugin.replace('*', '.*'))

log.debug('Loading %s from %s', plugin, pluginDirectory)
fs.readdirSync(pluginDirectory).filter(function (pluginName) {
return IGNORED_PACKAGES.indexOf(pluginName) === -1 && regexp.test(pluginName)
}).forEach(function (pluginName) {
requirePlugin(pluginDirectory + '/' + pluginName)
})
return
}
if (helper.isObject(plugin)) {
log.debug('Loading inlined plugin (defining %s).', Object.keys(plugin).join(', '))
modules.push(plugin)
} else {
log.warn('Invalid plugin %s', plugin)
return
}
log.error('Invalid plugin %s', plugin)
emitter.emit('load_error', 'plug_in', plugin)
})

return modules
Expand Down
23 changes: 13 additions & 10 deletions lib/preprocessor.js
View file
Expand Up @@ -38,12 +38,14 @@ var createNextProcessor = function (preprocessors, file, done) {
}

var createPreprocessor = function (config, basePath, injector) {
var alreadyDisplayedWarnings = {}
var alreadyDisplayedErrors = {}
var instances = {}
var patterns = Object.keys(config)

var emitter = injector.get('emitter')

var instantiatePreprocessor = function (name) {
if (alreadyDisplayedWarnings[name]) {
if (alreadyDisplayedErrors[name]) {
return
}

Expand All @@ -53,13 +55,13 @@ var createPreprocessor = function (config, basePath, injector) {
p = injector.get('preprocessor:' + name)
} catch (e) {
if (e.message.indexOf('No provider for "preprocessor:' + name + '"') !== -1) {
log.warn('Can not load "%s", it is not registered!\n ' +
'Perhaps you are missing some plugin?', name)
log.error('Can not load "%s", it is not registered!\n ' +
'Perhaps you are missing some plugin?', name)
} else {
log.warn('Can not load "%s"!\n ' + e.stack, name)
log.error('Can not load "%s"!\n ' + e.stack, name)
}

alreadyDisplayedWarnings[name] = true
alreadyDisplayedErrors[name] = true
emitter.emit('load_error', 'preprocessor', name)
}

return p
Expand Down Expand Up @@ -105,9 +107,10 @@ var createPreprocessor = function (config, basePath, injector) {
}

if (p == null) {
if (!alreadyDisplayedWarnings[name]) {
alreadyDisplayedWarnings[name] = true
log.warn('Failed to instantiate preprocessor %s', name)
if (!alreadyDisplayedErrors[name]) {
alreadyDisplayedErrors[name] = true
log.error('Failed to instantiate preprocessor %s', name)
emitter.emit('load_error', 'preprocessor', name)
}
return
}
Expand Down
5 changes: 3 additions & 2 deletions lib/reporter.js
View file
Expand Up @@ -107,11 +107,12 @@ var createReporters = function (names, config, emitter, injector) {
reporters.push(injector.createChild([locals], ['reporter:' + name]).get('reporter:' + name))
} catch (e) {
if (e.message.indexOf('No provider for "reporter:' + name + '"') !== -1) {
log.warn('Can not load "%s", it is not registered!\n ' +
log.error('Can not load reporter "%s", it is not registered!\n ' +
'Perhaps you are missing some plugin?', name)
} else {
log.warn('Can not load "%s"!\n ' + e.stack, name)
log.error('Can not load "%s"!\n ' + e.stack, name)
}
emitter.emit('load_error', 'reporter', name)
return
}
var color_name = name + '_color'
Expand Down
20 changes: 18 additions & 2 deletions lib/server.js
View file
Expand Up @@ -51,13 +51,16 @@ var Server = function (cliOptions, done) {

this.log = logger.create()

this.loadErrors = []

var config = cfg.parseConfig(cliOptions.configFile, cliOptions)

var modules = [{
helper: ['value', helper],
logger: ['value', logger],
done: ['value', done || process.exit],
emitter: ['value', this],
server: ['value', this],
launcher: ['type', Launcher],
config: ['value', config],
preprocess: ['factory', preprocessor.createPreprocessor],
Expand All @@ -82,8 +85,9 @@ var Server = function (cliOptions, done) {
}]
}]

this._setUpLoadErrorListener()
// Load the plugins
modules = modules.concat(plugin.resolve(config.plugins))
modules = modules.concat(plugin.resolve(config.plugins, this))

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

self.emit('listening', config.port)

if (config.browsers && config.browsers.length) {
self._injector.invoke(launcher.launch, launcher).forEach(function (browserLauncher) {
singleRunDoneBrowsers[browserLauncher.id] = false
})
}
var noLoadErrors = self.loadErrors.length
if (noLoadErrors > 0) {
self.log.error('Found %d load error%s', noLoadErrors, noLoadErrors === 1 ? '' : 's')
process.kill(process.pid, 'SIGINT')
}
})
}

Expand Down Expand Up @@ -363,6 +371,14 @@ Server.prototype._start = function (config, launcher, preprocess, fileList, webS
})
}

Server.prototype._setUpLoadErrorListener = function () {
var self = this
self.on('load_error', function (type, name) {
self.log.debug('Registered a load error of type %s with name %s', type, name)
self.loadErrors.push([type, name])
})
}

Server.prototype._detach = function (config, done) {
var log = this.log
var tmpFile = tmp.fileSync({keep: true})
Expand Down
106 changes: 106 additions & 0 deletions test/e2e/load.feature
View file
@@ -0,0 +1,106 @@
Feature: Basic Testrunner
In order to use Karma
As a person who wants to write great tests
I want Karma to terminate upon misconfiguration

Scenario: Execute with missing browser
Given a configuration with:
"""
files = ['basic/plus.js', 'basic/test.js'];
browsers = ['NonExistingBrowser', 'PhantomJS'];
plugins = [
'karma-jasmine',
'karma-phantomjs-launcher'
];
singleRun = false
"""
When I start Karma
Then it fails with like:
"""
Cannot load browser "NonExistingBrowser": it is not registered! Perhaps you are missing some plugin\?
"""
And it fails with like:
"""
Found 1 load error
"""

Scenario: Execute with missing plugin
Given a configuration with:
"""
files = ['basic/plus.js', 'basic/test.js'];
browsers = ['PhantomJS'];
plugins = [
'karma-totally-non-existing-plugin',
'karma-jasmine',
'karma-phantomjs-launcher'
];
singleRun = false
"""
When I start Karma
Then it fails with like:
"""
Cannot find plugin "karma-totally-non-existing-plugin".
[\s]+Did you forget to install it\?
[\s]+npm install karma-totally-non-existing-plugin --save-dev
"""
And it fails with like:
"""
Found 1 load error
"""

Scenario: Execute with missing reporter
Given a configuration with:
"""
files = ['basic/plus.js', 'basic/test.js'];
browsers = ['PhantomJS'];
reporters = ['unreal-reporter']
plugins = [
'karma-jasmine',
'karma-phantomjs-launcher'
];
singleRun = false
"""
When I start Karma
Then it fails with like:
"""
Can not load reporter "unreal-reporter", it is not registered!
[\s]+Perhaps you are missing some plugin\?
"""
And it fails with like:
"""
Found 1 load error
"""

Scenario: Execute with missing reporter, plugin and browser
Given a configuration with:
"""
files = ['basic/plus.js', 'basic/test.js'];
browsers = ['NonExistingBrowser', 'PhantomJS'];
reporters = ['unreal-reporter']
plugins = [
'karma-totally-non-existing-plugin',
'karma-jasmine',
'karma-phantomjs-launcher'
];
singleRun = false
"""
When I start Karma
Then it fails with like:
"""
Can not load reporter "unreal-reporter", it is not registered!
[\s]+Perhaps you are missing some plugin\?
"""
And it fails with like:
"""
Cannot find plugin "karma-totally-non-existing-plugin".
[\s]+Did you forget to install it\?
[\s]+npm install karma-totally-non-existing-plugin --save-dev
"""
And it fails with like:
"""
Cannot load browser "NonExistingBrowser": it is not registered! Perhaps you are missing some plugin\?
"""
And it fails with like:
"""
Found 3 load errors
"""
17 changes: 15 additions & 2 deletions test/unit/launcher.spec.js
View file
Expand Up @@ -69,18 +69,22 @@ describe('launcher', () => {

describe('Launcher', () => {
var emitter
var l = emitter = null
var server
var l = emitter = server = null

beforeEach(() => {
emitter = new events.EventEmitter()
server = {'loadErrors': []}

var injector = new di.Injector([{
'launcher:Fake': ['type', FakeBrowser],
'launcher:Script': ['type', ScriptBrowser],
'server': ['value', server],
'emitter': ['value', emitter],
'config': ['value', {captureTimeout: 0}],
'timer': ['factory', createMockTimer]
}])
l = new launcher.Launcher(emitter, injector)
l = new launcher.Launcher(server, emitter, injector)
})

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

it('should not start when server has load errors', () => {
server.loadErrors = ['error']
l.launch(['Fake'], 'http:', 'localhost', 1234, '/root/', 1)
var browser = FakeBrowser._instances.pop()
expect(browser.start).to.not.have.been.called
expect(browser.id).to.equal(lastGeneratedId)
expect(browser.name).to.equal('Fake')
})

it('should allow launching a script', () => {
l.launch(['/usr/local/bin/special-browser'], 'http:', 'localhost', 1234, '/', 1)

Expand Down

0 comments on commit fca930e

Please sign in to comment.