Merge pull request #1515 from KJTsanaktsidis/fix_runner_race

fix(runner): Wait for file list refresh to finish before running
karma-runner · Jul 24, 2015 · f60e194 · f60e194
2 parents cae9019 + 94cddc0
commit f60e194
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