feat: Fail on launcher-, reporter-, plugin-, or preprocessor-load errors.

srawlins · budde377 · commit fca930e3a5a4 · 2016-03-01T16:31:23.000+01:00
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
diff --git a/lib/launcher.js b/lib/launcher.js
@@ -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)
@@ -20,7 +24,7 @@ var baseBrowserDecoratorFactory = function (baseLauncherDecorator, captureTimeou
   }
 }
 
-var Launcher = function (emitter, injector) {
+var Launcher = function (server, emitter, injector) {
   var browsers = []
   var lastStartTime
 
@@ -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 () {
@@ -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)
diff --git a/lib/plugin.js b/lib/plugin.js
@@ -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) {
@@ -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
diff --git a/lib/preprocessor.js b/lib/preprocessor.js
@@ -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
     }
 
@@ -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
@@ -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
           }
diff --git a/lib/reporter.js b/lib/reporter.js
@@ -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'
diff --git a/lib/server.js b/lib/server.js
@@ -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],
@@ -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)
 }
@@ -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')
+      }
     })
   }
 
@@ -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})
diff --git a/test/e2e/load.feature b/test/e2e/load.feature
@@ -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
+      """
diff --git a/test/unit/launcher.spec.js b/test/unit/launcher.spec.js
@@ -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', () => {
@@ -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)
 
diff --git a/test/unit/preprocessor.spec.js b/test/unit/preprocessor.spec.js
