fix(runner): Wait for file list refresh to finish before running

The runner middleware was starting a file list refresh before executing a test run, but was not waiting for it to complete. This meant that the context.html generation would often not see the new file list, and so stale test file sha hashes would be included in context.html
karma-runner · Jul 24, 2015 · 94cddc0 · 94cddc0
1 parent cae9019
commit 94cddc0
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 9 deletions.
diff --git a/lib/middleware/runner.js b/lib/middleware/runner.js
@@ -73,11 +73,13 @@ var createRunnerMiddleware = function (emitter, fileList, capturedBrowsers, repo
 
       if (fullRefresh && data.refresh !== false) {
         log.debug('Refreshing all the files / patterns')
-        fileList.refresh()
-
-        if (!config.autoWatch) {
-          executor.schedule()
-        }
+        fileList.refresh().then(function () {
+          // Wait for the file list refresh to complete before starting test run,
+          // otherwise the context.html generation might not see new/updated files.
+          if (!config.autoWatch) {
+            executor.schedule()
+          }
+        })
       } else {
         executor.schedule()
       }

diff --git a/test/unit/middleware/runner.spec.coffee b/test/unit/middleware/runner.spec.coffee
@@ -10,6 +10,7 @@ describe 'middleware.runner', ->
   BrowserCollection = require '../../../lib/browser_collection'
   MultReporter = require('../../../lib/reporters/multi')
   createRunnerMiddleware = require('../../../lib/middleware/runner').create
+  Promise = require('bluebird')
 
   handler = nextSpy = response = mockReporter = capturedBrowsers = emitter = config = null
   fileListMock = executor = null
@@ -25,7 +26,7 @@ describe 'middleware.runner', ->
     emitter = new EventEmitter
     capturedBrowsers = new BrowserCollection emitter
     fileListMock =
-      refresh: -> null
+      refresh: -> Promise.resolve(null)
       addFile: -> null
       removeFile: -> null
       changeFile: -> null
@@ -49,8 +50,11 @@ describe 'middleware.runner', ->
 
     handler new HttpRequestMock('/__run__'), response, nextSpy
 
-    mockReporter.write 'result'
-    emitter.emit 'run_complete', capturedBrowsers, {exitCode: 0}
+    # Wrap this in a setTimeout so the fileListPromise has time to resolve.
+    setTimeout( ->
+      mockReporter.write 'result'
+      emitter.emit 'run_complete', capturedBrowsers, {exitCode: 0}
+    , 2)
 
 
   it 'should not run if there is no browser captured', (done) ->
@@ -141,14 +145,37 @@ describe 'middleware.runner', ->
       expect(executor.schedule).to.have.been.called
       done()
 
+  it 'should wait for refresh to finish if applicable before scheduling execution', (done) ->
+    capturedBrowsers.add new Browser
+    sinon.stub capturedBrowsers, 'areAllReady', -> true
+
+    resolve = null
+    fileListPromise = new Promise (_resolve, _reject) ->
+      resolve = _resolve
+    sinon.stub(fileListMock, 'refresh').returns fileListPromise
+    sinon.stub executor, 'schedule'
+
+    request = new HttpRequestMock '/__run__'
+    handler request, response, nextSpy
+
+    process.nextTick ->
+      expect(fileListMock.refresh).to.have.been.called
+      expect(executor.schedule).to.not.have.been.called
+
+      # Now try resolving the promise
+      resolve()
+      setTimeout(->
+        expect(executor.schedule).to.have.been.called
+        done()
+      , 2)
 
   it 'should not schedule execution if refreshing and autoWatch', (done) ->
     config.autoWatch = true
 
     capturedBrowsers.add new Browser
     sinon.stub capturedBrowsers, 'areAllReady', -> true
 
-    sinon.stub fileListMock, 'refresh'
+    sinon.stub(fileListMock, 'refresh').returns Promise.resolve(null)
     sinon.stub executor, 'schedule'
 
     handler new HttpRequestMock('/__run__'), response, nextSpy