fix(client): prevent race condition in clear context

client/karma.js

@@ -89,8 +89,8 @@ function Karma (socket, iframe, opener, navigator, location, document) {
           childWindow.close()
         }
         childWindow = opener(url)
-      // run context on parent element (client_with_context)
-      // using window.__karma__.scriptUrls to get the html element strings and load them dynamically
+        // run context on parent element (client_with_context)
+        // using window.__karma__.scriptUrls to get the html element strings and load them dynamically
       } else if (url !== 'about:blank') {
         var loadScript = function (idx) {
           if (idx < window.__karma__.scriptUrls.length) {
@@ -120,14 +120,33 @@ function Karma (socket, iframe, opener, navigator, location, document) {
         }
         loadScript(0)
       }
-    // run in iframe
+      // run in iframe
     } else {
       iframe.src = policy.createURL(url)
     }
   }
 
+  /**
+   * Schedules te next execution after current context is cleared
+   * (or is directly started if context is currently not being cleared)
+   * @param execution {() => void}
+   * @see https://github.com/karma-runner/karma/issues/3424
+   */
+  this.scheduleExecution = function (execution) {
+    if (reloadingContext) {
+      // A context reload is in progress. Wait for it to complete before executing.
+      this.scheduledExecution = execution
+    } else {
+      execution()
+    }
+  }
+
   this.onbeforeunload = function () {
-    if (!reloadingContext) {
+    if (reloadingContext) {
+      reloadingContext = false
+      self.scheduledExecution && self.scheduledExecution()
+      self.scheduledExecution = undefined
+    } else {
       // TODO(vojta): show what test (with explanation about jasmine.UPDATE_INTERVAL)
       self.error('Some of your tests did a full page reload!')
     }
@@ -146,8 +165,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
   this.stringify = stringify
 
   function clearContext () {
-    reloadingContext = true
-
     navigateContextTo('about:blank')
   }
 
@@ -234,6 +251,8 @@ function Karma (socket, iframe, opener, navigator, location, document) {
     }
 
     if (self.config.clearContext) {
+      reloadingContext = true
+
       // give the browser some time to breath, there could be a page reload, but because a bunch of
       // tests could run in the same event loop, we wouldn't notice.
       setTimeout(function () {
@@ -259,24 +278,25 @@ function Karma (socket, iframe, opener, navigator, location, document) {
   }
 
   socket.on('execute', function (cfg) {
-    // reset startEmitted and reload the iframe
-    startEmitted = false
-    self.config = cfg
-    // if not clearing context, reloadingContext always true to prevent beforeUnload error
-    reloadingContext = !self.config.clearContext
-    navigateContextTo(constant.CONTEXT_URL)
-
-    if (self.config.clientDisplayNone) {
-      [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
-        el.style.display = 'none'
-      })
-    }
+    self.scheduleExecution(() => {
+      // reset startEmitted and reload the iframe
+      startEmitted = false
+      self.config = cfg
 
-    // clear the console before run
-    // works only on FF (Safari, Chrome do not allow to clear console from js source)
-    if (window.console && window.console.clear) {
-      window.console.clear()
-    }
+      navigateContextTo(constant.CONTEXT_URL)
+
+      if (self.config.clientDisplayNone) {
+        [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
+          el.style.display = 'none'
+        })
+      }
+
+      // clear the console before run
+      // works only on FF (Safari, Chrome do not allow to clear console from js source)
+      if (window.console && window.console.clear) {
+        window.console.clear()
+      }
+    })
   })
   socket.on('stop', function () {
     this.complete()

test/client/karma.spec.js

@@ -85,7 +85,7 @@ describe('Karma', function () {
   })
 
   it('should remove reference to start even after syntax error', function () {
-    function ADAPTER_START_FN () {}
+    function ADAPTER_START_FN () { }
 
     ck.start = ADAPTER_START_FN
     ck.error('syntax error', '/some/file.js', 11)
@@ -129,23 +129,68 @@ describe('Karma', function () {
     assert(mockWindow.onerror != null)
   })
 
-  it('should error out if a script attempted to reload the browser after setup', function () {
-    // Perform setup
-    var config = ck.config = {
+  it('should schedule execution if clearContext is busy', function () {
+    // Arrange
+    ck.config = k.config = {
+      useIframe: true,
       clearContext: true
     }
-    socket.emit('execute', config)
-    var mockWindow = {}
+    const runConfig = {
+      useIFrame: true
+    }
+    const mockWindow = {}
     ck.setupContext(mockWindow)
+    k.complete() // set reloading contexts
+
+    // Act
+    socket.emit('execute', runConfig)
+    ck.loaded()
+
+    // Assert
+    assert(!startSpy.calledWith(runConfig))
+    assert(!!k.scheduledExecution)
+  })
+
+  describe('onbeforeunload', function () {
+    it('should error out if a script attempted to reload the browser after setup', function () {
+      // Perform setup
+      var config = ck.config = {
+        clearContext: true
+      }
+
+      socket.emit('execute', config)
+      var mockWindow = {}
+      ck.setupContext(mockWindow)
 
-    // Spy on our error handler
-    sinon.spy(k, 'error')
+      // Spy on our error handler
+      sinon.spy(k, 'error')
 
-    // Emulate an unload event
-    mockWindow.onbeforeunload()
+      // Emulate an unload event
+      mockWindow.onbeforeunload()
+
+      // Assert our spy was called
+      assert(k.error.calledWith('Some of your tests did a full page reload!'))
+    })
 
-    // Assert our spy was called
-    assert(k.error.calledWith('Some of your tests did a full page reload!'))
+    it('should execute if an earlier execution is scheduled', function () {
+      // Arrange
+      const config = ck.config = k.config = {
+        useIframe: true,
+        clearContext: true
+      }
+      const mockWindow = {}
+      ck.setupContext(mockWindow)
+      k.complete() // set reloading contexts
+      socket.emit('execute', config)
+
+      // Act
+      mockWindow.onbeforeunload()
+      ck.loaded()
+
+      // Assert
+      assert(startSpy.calledWith(config))
+      assert(!k.scheduledExecution)
+    })
   })
 
   it('should report navigator name', function () {
@@ -318,7 +363,7 @@ describe('Karma', function () {
 
       var mockWindow = {
         console: {
-          log: function () {}
+          log: function () { }
         }
       }
 
@@ -334,7 +379,7 @@ describe('Karma', function () {
 
       var mockWindow = {
         console: {
-          log: function () {}
+          log: function () { }
         }
       }