feat(preprocessor): allow preprocessor to cancel test run

Now a preprocessor can return an error, which
  - stops preprocessing (does not trigger other preprocessors),
  - cancels the test run.

This changes the preprocessor API - preprocessor's `done` function accepts two arguments:
  - an error (`null` if no error),
  - content.

The change is done in a backwards compatible way - if a preprocessor calls `done` with a single
string argument, it is treated as no error.

Closes #550

lib/file_list.js

@@ -68,6 +68,22 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
   var pendingDeferred;
   var pendingTimeout;
 
+  var errors = [];
+
+  var addError = function(path) {
+    if (errors.indexOf(path) === -1) {
+      errors.push(path);
+    }
+  };
+
+  var removeError = function(path) {
+    var idx = errors.indexOf(path);
+
+    if (idx !== -1) {
+      errors.splice(idx, 1);
+    }
+  };
+
   var resolveFiles = function(buckets) {
     var uniqueMap = {};
     var files = {
@@ -99,7 +115,12 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
       clearTimeout(pendingTimeout);
     }
 
-    pendingDeferred.resolve(files || resolveFiles(self.buckets));
+    if (!errors.length) {
+      pendingDeferred.resolve(files || resolveFiles(self.buckets));
+    } else {
+      pendingDeferred.reject(errors.slice());
+    }
+
     pendingDeferred = pendingTimeout = null;
   };
 
@@ -143,6 +164,8 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
       }
     };
 
+    errors = [];
+
     if (!pendingDeferred) {
       pendingDeferred = q.defer();
       emitter.emit('file_list_modified', pendingDeferred.promise);
@@ -193,8 +216,13 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
                 // TODO(vojta): reuse file objects
                 var file = new File(path, stat.mtime);
 
-                preprocess(file, function() {
+                preprocess(file, function(err) {
                   buckets[i].push(file);
+
+                  if (err) {
+                    addError(path);
+                  }
+
                   finish();
                 });
               } else {
@@ -279,9 +307,14 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
       if (self.buckets === buckets) {
         addedFile.mtime = stat.mtime;
 
-        return preprocess(addedFile, function() {
+        return preprocess(addedFile, function(err) {
           // TODO(vojta): ignore if refresh/reload happens
           log.info('Added file "%s".', path);
+
+          if (err) {
+            addError(path);
+          }
+
           fireEventAndDefer();
           done();
         });
@@ -336,7 +369,13 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
         // 1/ the preprocessor should not change the object in place, but create a copy that would
         // be eventually merged into the original file, here in the callback, synchronously.
         // 2/ delay the promise resolution - wait for any changeFile operations to finish
-        return preprocess(changedFile, function() {
+        return preprocess(changedFile, function(err) {
+          if (err) {
+            addError(path);
+          } else {
+            removeError(path);
+          }
+
           // TODO(vojta): ignore if refresh/reload happens
           fireEventAndDefer();
           done();
@@ -364,6 +403,7 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) {
         if (buckets[i][j].originalPath === path) {
           buckets[i].splice(j, 1);
           log.info('Removed file "%s".', path);
+          removeError(path);
           fireEventAndDefer();
           return done();
         }

lib/middleware/karma.js

@@ -110,6 +110,14 @@ var createKarmaMiddleware = function(filesPromise, serveStaticFile,
 
           return data.replace('%SCRIPTS%', scriptTags.join('\n')).replace('%MAPPINGS%', mappings);
         });
+      }, function(errorFiles) {
+        serveStaticFile(requestUrl, response, function(data) {
+          common.setNoCacheHeaders(response);
+          return data.replace('%SCRIPTS%', '').replace('%MAPPINGS%',
+              'window.__karma__.error("TEST RUN WAS CANCELLED because ' +
+              (errorFiles.length > 1 ? 'these files contain' : 'this file contains') +
+              ' some errors:\\n  ' + errorFiles.join('\\n  ') + '");');
+        });
       });
     }
 

lib/preprocessor.js

@@ -17,7 +17,19 @@ var createPreprocessor = function(config, basePath, injector) {
 
   return function(file, done) {
     var preprocessors = [];
-    var nextPreprocessor = function(content) {
+    var nextPreprocessor = function(error, content) {
+      // normalize B-C
+      if (arguments.length === 1 && typeof error === 'string') {
+        content = error;
+        error = null;
+      }
+
+      if (error) {
+        file.content = null;
+        file.contentPath = null;
+        return done(error);
+      }
+
       if (!preprocessors.length) {
         file.contentPath = null;
         file.content = content;

lib/server.js

@@ -34,6 +34,8 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
   if (config.autoWatch) {
     filesPromise.then(function() {
       injector.invoke(watcher.watch);
+    }, function() {
+      injector.invoke(watcher.watch);
     });
   }
 

test/unit/file_list.spec.coffee

@@ -19,6 +19,7 @@ describe 'file_list', ->
     '**':         ['/a.txt', '/b.txt', '/c.txt', '/a.txt', '/c.txt', '/b.txt', '/a.txt', '/c.txt',
                    '/a.txt', '/a.txt', '/c.txt']
 
+  # TODO(vojta): create new fs, as we mutate the file stats now
   mockFs = mocks.fs.create
     some:
       '0.js': mocks.fs.file '2012-04-04'
@@ -153,6 +154,20 @@ describe 'file_list', ->
         done()
 
 
+    it 'should reject the promise if a preprocessor fails', (done) ->
+      preprocessMock = (file, next) ->
+        next new Error('Prerocess failure.'), null
+
+      # MATCH /some/a.js, /some/b.js
+      list = new m.List patterns('/some/*.js'), [], emitter, preprocessMock
+      spyResolve = sinon.spy()
+      spyReject = sinon.spy()
+
+      list.refresh().then(spyResolve, spyReject).done ->
+        expect(spyResolve).not.to.have.been.called
+        expect(spyReject).to.have.been.called
+        done()
+
 
   #============================================================================
   # List.reload()
@@ -436,6 +451,140 @@ describe 'file_list', ->
           expect(onFileListModifiedSpy).not.to.have.been.called
 
 
+  describe 'preprocess failure', ->
+    spyResolve = spyReject = null
+
+    preprocessMock2 = (file, next) ->
+      matchFile = (pattern) ->
+        pattern.test and pattern.test(file.path) or pattern is file.path
+
+      if preprocessMock2._fail.some matchFile
+        next new Error('Failed to preprocess.'), null
+      else
+        next()
+
+    preprocessMock2.fail = (pattern) -> preprocessMock2._fail.push(pattern)
+    preprocessMock2.fix = (pattern) ->
+      preprocessMock2._fail = preprocessMock2._fail.filter((p) -> pattern is not p)
+
+
+    beforeEach ->
+      spyResolve = sinon.spy()
+      spyReject = sinon.spy()
+      preprocessMock2._fail = []
+
+      mockFs._touchFile '/some/a.js', '2012-04-04'
+      mockFs._touchFile '/some/b.js', '2012-05-05'
+
+
+    it 'should reject when an incorrect file added', (done) ->
+      # MATCH: /some/a.js, /some/b.js, /a.txt
+      list = new m.List patterns('/some/*.js', '/a.*'), [], emitter, preprocessMock2
+
+      # once files are resolved, execute next item in the queue
+      emitter.on 'file_list_modified', (files) -> files.then(spyResolve, spyReject).done next
+
+      # refresh the list and kick off the queue
+      list.refresh()
+
+      scheduleNext ->
+        spyResolve.reset()
+        spyReject.reset()
+        preprocessMock2.fail '/some/0.js'
+        list.addFile '/some/0.js'
+
+      scheduleNext ->
+        expect(spyResolve).not.to.have.been.called
+        expect(spyReject).to.have.been.called
+        done()
+
+
+    it 'should resolve once all the files are fixed', (done) ->
+      # MATCH: /some/a.js, /some/b.js, /a.txt
+      list = new m.List patterns('/some/*.js', '/a.*'), [], emitter, preprocessMock2
+
+      # once files are resolved, execute next item in the queue
+      emitter.on 'file_list_modified', (files) -> files.then(spyResolve, spyReject).done next
+
+      # refresh the list and kick off the queue
+      list.refresh()
+
+      scheduleNext ->
+        preprocessMock2.fail '/some/a.js'
+        mockFs._touchFile '/some/a.js', '2020-01-01'
+        list.changeFile '/some/a.js'
+
+      scheduleNext ->
+        spyResolve.reset()
+        spyReject.reset()
+        preprocessMock2.fix '/some/a.js'
+        mockFs._touchFile '/some/a.js', '2020-01-02'
+        list.changeFile '/some/a.js'
+
+      scheduleNext ->
+        expect(spyResolve).to.have.been.called
+        expect(spyReject).not.to.have.been.called
+        done()
+
+
+    it 'should reject if only some files are fixed', (done) ->
+      # MATCH: /some/a.js, /some/b.js, /a.txt
+      list = new m.List patterns('/some/*.js', '/a.*'), [], emitter, preprocessMock2
+
+      # once files are resolved, execute next item in the queue
+      emitter.on 'file_list_modified', (files) -> files.then(spyResolve, spyReject).done next
+
+      # refresh the list and kick off the queue
+      list.refresh()
+
+      scheduleNext ->
+        preprocessMock2.fail '/some/a.js'
+        preprocessMock2.fail '/some/b.js'
+        mockFs._touchFile '/some/a.js', '2020-01-01'
+        list.changeFile '/some/a.js'
+        mockFs._touchFile '/some/b.js', '2020-01-01'
+        list.changeFile '/some/b.js'
+
+      scheduleNext ->
+        spyResolve.reset()
+        spyReject.reset()
+        preprocessMock2.fix '/some/a.js'
+        mockFs._touchFile '/some/a.js', '2020-01-02'
+        list.changeFile '/some/a.js'
+
+      scheduleNext ->
+        # /some/b.js still contains error
+        expect(spyResolve).not.to.have.been.called
+        expect(spyReject).to.have.been.called
+        done()
+
+
+    it 'should resolve if incorrect file is removed', (done) ->
+      # MATCH: /some/a.js, /some/b.js, /a.txt
+      list = new m.List patterns('/some/*.js', '/a.*'), [], emitter, preprocessMock2
+
+      # once files are resolved, execute next item in the queue
+      emitter.on 'file_list_modified', (files) -> files.then(spyResolve, spyReject).done next
+
+      # refresh the list and kick off the queue
+      list.refresh()
+
+      scheduleNext ->
+        preprocessMock2.fail '/some/a.js'
+        mockFs._touchFile '/some/a.js', '2020-01-01'
+        list.changeFile '/some/a.js'
+
+      scheduleNext ->
+        spyResolve.reset()
+        spyReject.reset()
+        list.removeFile '/some/a.js'
+
+      scheduleNext ->
+        expect(spyResolve).to.have.been.called
+        expect(spyReject).not.to.have.been.called
+        done()
+
+
   #============================================================================
   # Batch Interval processing
   #============================================================================

test/unit/mocha-globals.coffee

@@ -47,13 +47,28 @@ chai.use (chai, utils) ->
 # TODO(vojta): move it somewhere ;-)
 nextTickQueue = []
 nextTickCallback = ->
+  if not nextTickQueue.length then throw new Error 'Nothing scheduled!'
   nextTickQueue.shift()()
-
   if nextTickQueue.length then process.nextTick nextTickCallback
 
 global.scheduleNextTick = (action) ->
   nextTickQueue.push action
   if nextTickQueue.length is 1 then process.nextTick nextTickCallback
 
+nextQueue = []
+nextCallback = ->
+  if not nextQueue.length then throw new Error 'Nothing scheduled!'
+  nextQueue.shift()()
+
+global.scheduleNextTick = (action) ->
+  nextTickQueue.push action
+  if nextTickQueue.length is 1 then process.nextTick nextTickCallback
+
+global.scheduleNext = (action) ->
+  nextQueue.push action
+
+global.next = nextCallback
+
 beforeEach ->
   nextTickQueue.length = 0
+  nextQueue.length = 0