Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Fail on launcher-, reporter-, plugin-, or preprocessor-load err… #1948

Closed
wants to merge 1 commit into from
Closed

feat: Fail on launcher-, reporter-, plugin-, or preprocessor-load err… #1948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions lib/launcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var processDecorator = require('./launchers/process').decoratorFactory

// TODO(vojta): remove once nobody uses it
var baseBrowserDecoratorFactory = function (baseLauncherDecorator, captureTimeoutLauncherDecorator,
retryLauncherDecorator, processLauncherDecorator) {
retryLauncherDecorator, processLauncherDecorator) {
return function (launcher) {
baseLauncherDecorator(launcher)
captureTimeoutLauncherDecorator(launcher)
Expand All @@ -20,7 +20,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 +61,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 +195,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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Open in desktop
Original file line number Diff line number Diff line change
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
Loading