Revert "fix(client): avoid race between execute and clearContext (kar…

…ma-runner#3452)"

This reverts commit 8bc5b46.

client/karma.js

@@ -131,13 +131,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
       // TODO(vojta): show what test (with explanation about jasmine.UPDATE_INTERVAL)
       self.error('Some of your tests did a full page reload!')
     }
-    reloadingContext = false
-  }
-
-  function clearContext () {
-    reloadingContext = true
-
-    navigateContextTo('about:blank')
   }
 
   this.log = function (type, args) {
@@ -152,6 +145,12 @@ function Karma (socket, iframe, opener, navigator, location, document) {
 
   this.stringify = stringify
 
+  function clearContext () {
+    reloadingContext = true
+
+    navigateContextTo('about:blank')
+  }
+
   function getLocation (url, lineno, colno) {
     var location = ''
 
@@ -235,10 +234,11 @@ function Karma (socket, iframe, opener, navigator, location, document) {
     }
 
     if (self.config.clearContext) {
-      // A test could have incorrectly issued a navigate. To clear the context
-      // we will navigate the iframe. Delay ours to ensure the error from an
-      // incorrect navigate is processed.
-      setTimeout(clearContext)
+      // 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 () {
+        clearContext()
+      }, 0)
     }
 
     socket.emit('complete', result || {}, function () {
@@ -259,26 +259,24 @@ function Karma (socket, iframe, opener, navigator, location, document) {
   }
 
   socket.on('execute', function (cfg) {
-    // Delay our navigation to the next event in case the clearContext has not completed.
-    setTimeout(function allowClearContextToComplete () {
-      // reset startEmitted and reload the iframe
-      startEmitted = false
-      self.config = cfg
-
-      navigateContextTo(constant.CONTEXT_URL)
-
-      if (self.config.clientDisplayNone) {
-        [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
-          el.style.display = 'none'
-        })
-      }
+    // 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'
+      })
+    }
 
-      // 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()
-      }
-    })
+    // 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()

static/karma.js

@@ -141,13 +141,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
       // TODO(vojta): show what test (with explanation about jasmine.UPDATE_INTERVAL)
       self.error('Some of your tests did a full page reload!')
     }
-    reloadingContext = false
-  }
-
-  function clearContext () {
-    reloadingContext = true
-
-    navigateContextTo('about:blank')
   }
 
   this.log = function (type, args) {
@@ -162,6 +155,12 @@ function Karma (socket, iframe, opener, navigator, location, document) {
 
   this.stringify = stringify
 
+  function clearContext () {
+    reloadingContext = true
+
+    navigateContextTo('about:blank')
+  }
+
   function getLocation (url, lineno, colno) {
     var location = ''
 
@@ -245,10 +244,11 @@ function Karma (socket, iframe, opener, navigator, location, document) {
     }
 
     if (self.config.clearContext) {
-      // A test could have incorrectly issued a navigate. To clear the context
-      // we will navigate the iframe. Delay ours to ensure the error from an
-      // incorrect navigate is processed.
-      setTimeout(clearContext)
+      // 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 () {
+        clearContext()
+      }, 0)
     }
 
     socket.emit('complete', result || {}, function () {
@@ -269,26 +269,24 @@ function Karma (socket, iframe, opener, navigator, location, document) {
   }
 
   socket.on('execute', function (cfg) {
-    // Delay our navigation to the next event in case the clearContext has not completed.
-    setTimeout(function allowClearContextToComplete () {
-      // reset startEmitted and reload the iframe
-      startEmitted = false
-      self.config = cfg
-
-      navigateContextTo(constant.CONTEXT_URL)
-
-      if (self.config.clientDisplayNone) {
-        [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
-          el.style.display = 'none'
-        })
-      }
+    // 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'
+      })
+    }
 
-      // 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()
-      }
-    })
+    // 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

@@ -44,40 +44,31 @@ describe('Karma', function () {
     assert(startSpy.calledWith(config))
   })
 
-  it('should open a new window when useIFrame is false', function (done) {
+  it('should open a new window when useIFrame is false', function () {
     var config = ck.config = {
       useIframe: false,
       runInParent: false
     }
 
     socket.emit('execute', config)
-    setTimeout(function nextEventLoop () {
-      assert(!ck.start.called)
+    assert(!ck.start.called)
 
-      ck.loaded()
-      assert(startSpy.calledWith(config))
-      assert(windowStub.calledWith('context.html'))
-      done()
-    })
+    ck.loaded()
+    assert(startSpy.calledWith(config))
+    assert(windowStub.calledWith('context.html'))
   })
 
-  it('should not set style on elements', function (done) {
+  it('should not set style on elements', function () {
     var config = {}
     socket.emit('execute', config)
-    setTimeout(function nextEventLoop () {
-      assert(Object.keys(elements[0].style).length === 0)
-      done()
-    })
+    assert(Object.keys(elements[0].style).length === 0)
   })
 
-  it('should set display none on elements if clientDisplayNone', function (done) {
+  it('should set display none on elements if clientDisplayNone', function () {
     var config = { clientDisplayNone: true }
     socket.emit('execute', config)
-    setTimeout(function nextEventLoop () {
-      assert(elements[0].style.display === 'none')
-      assert(elements[1].style.display === 'none')
-      done()
-    })
+    assert(elements[0].style.display === 'none')
+    assert(elements[1].style.display === 'none')
   })
 
   it('should stop execution', function () {
@@ -106,65 +97,55 @@ describe('Karma', function () {
     assert.notStrictEqual(k.start, ADAPTER_START_FN)
   })
 
-  it('should not set up context if there was an error', function (done) {
+  it('should not set up context if there was an error', function () {
     var config = ck.config = {
       clearContext: true
     }
 
     socket.emit('execute', config)
 
-    setTimeout(function nextEventLoop () {
-      var mockWindow = {}
+    var mockWindow = {}
 
-      ck.error('page reload')
-      ck.setupContext(mockWindow)
+    ck.error('page reload')
+    ck.setupContext(mockWindow)
 
-      assert(mockWindow.onbeforeunload == null)
-      assert(mockWindow.onerror == null)
-      done()
-    })
+    assert(mockWindow.onbeforeunload == null)
+    assert(mockWindow.onerror == null)
   })
 
-  it('should setup context if there was error but clearContext config is false', function (done) {
+  it('should setup context if there was error but clearContext config is false', function () {
     var config = ck.config = {
       clearContext: false
     }
 
     socket.emit('execute', config)
 
-    setTimeout(function nextEventLoop () {
-      var mockWindow = {}
+    var mockWindow = {}
 
-      ck.error('page reload')
-      ck.setupContext(mockWindow)
+    ck.error('page reload')
+    ck.setupContext(mockWindow)
 
-      assert(mockWindow.onbeforeunload != null)
-      assert(mockWindow.onerror != null)
-      done()
-    })
+    assert(mockWindow.onbeforeunload != null)
+    assert(mockWindow.onerror != null)
   })
 
-  it('should error out if a script attempted to reload the browser after setup', function (done) {
+  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)
 
-    setTimeout(function nextEventLoop () {
-      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!'))
-      done()
-    })
+    // Assert our spy was called
+    assert(k.error.calledWith('Some of your tests did a full page reload!'))
   })
 
   it('should report navigator name', function () {
@@ -458,10 +439,12 @@ describe('Karma', function () {
       }
 
       socket.emit('execute', config)
-      clock.tick(1)
       var CURRENT_URL = iframe.src
+
       ck.complete()
+
       clock.tick(1)
+
       assert.strictEqual(iframe.src, CURRENT_URL)
     })
 
@@ -472,7 +455,6 @@ describe('Karma', function () {
       }
 
       socket.emit('execute', config)
-      clock.tick(1)
       assert(!startSpy.called)
 
       ck.loaded()